bk commit into 5.0 tree (mhansson:1.2443) BUG#23856

[mhansson]

Below is the list of changes that have just been committed into a local
5.0 repository of martin. When martin does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-04-25 15:56:26+03:00, mhansson@stripped +6 -0
  Bug #23856:GROUP_CONCAT and ORDER BY: junk from previous rows for query on I_S
  
  Item_func_group_concat::setup, responsible for initalizing placeholder fields to
represent
  the group column for GROUP_CONCAT, had misunderstood the save_sum_fields argument to 
  create_tmp_table:
  
      "We need to set set_sum_field in true for storing value of blob in buffer
       of a record instead of a pointer of one."
  
  This is not true, it's currently not possible to get this behavior out of blobs.
  It would be desirable, though, since if we could do this we could keep 
  using the TREE data structure that does the ORDER BY sorting. At the present state of
MySQL,
  blob fields can't be stored in this way since always keep a pointer to the actual data.
So
  the fix is to use VARCHAR for the placeholder field instead. This 
  will lead to results being cut if they exceed the maximum VARCHAR length (65535 bytes),
but 
  doing more is beyond a simple bug fix.
  
  There is still one problem, there is no warning about this cutting. Consider:
  
  create table t20b(a text character set utf8, b int);
  insert into t20b values (repeat('a', 0xffff+1), 0); 
  
  mysql> select length(group_concat(a)) from t20b;
  +-------------------------+
  | length(group_concat(a)) |
  +-------------------------+
  |                   32768 |
  +-------------------------+
  1 row in set, 1 warning (0.00 sec)
  
  mysql> show warnings;
  +---------+------+--------------------------------------+
  | Level   | Code | Message                              |
  +---------+------+--------------------------------------+
  | Warning | 1260 | 1 line(s) were cut by GROUP_CONCAT() |
  +---------+------+--------------------------------------+
  1 row in set (0.00 sec)
  
  mysql> select length(group_concat(a order by b)) from t20b;
  +------------------------------------+
  | length(group_concat(a order by b)) |
  +------------------------------------+
  |                              32768 |
  +------------------------------------+
  1 row in set (0.00 sec)
  
  Notice there's no warnings. This has nothing to do with our conversion to VARCHAR,
though:
  
  create table t20v(a varchar(65528), b int);
  insert into t20v values (repeat('a', 0xffff), 0); 
  
  mysql> select length(group_concat(a)) from t20v;
  +-------------------------+
  | length(group_concat(a)) |
  +-------------------------+
  |                   32768 |
  +-------------------------+
  1 row in set, 1 warning (0.00 sec)
  
  mysql> show warnings;
  +---------+------+--------------------------------------+
  | Level   | Code | Message                              |
  +---------+------+--------------------------------------+
  | Warning | 1260 | 1 line(s) were cut by GROUP_CONCAT() |
  +---------+------+--------------------------------------+
  1 row in set (0.00 sec)
  
  mysql> select length(group_concat(a order by b)) from t20v;
  +------------------------------------+
  | length(group_concat(a order by b)) |
  +------------------------------------+
  |                              32768 |
  +------------------------------------+
  1 row in set (0.00 sec)
  
  This seems to be a bug in its own respect.

  mysql-test/r/func_gconcat.result@stripped, 2007-04-25 15:56:23+03:00,
mhansson@stripped +7 -0
    Bug #23856:GROUP_CONCAT and ORDER BY: junk from previous rows for query on I_S
    
    correct result.

  mysql-test/t/func_gconcat.test@stripped, 2007-04-25 15:56:23+03:00, mhansson@stripped
+9 -0
    Bug #23856:GROUP_CONCAT and ORDER BY: junk from previous rows for query on I_S
    
    test case

  sql/field.h@stripped, 2007-04-25 15:56:23+03:00, mhansson@stripped +1 -0
    Bug #23856:GROUP_CONCAT and ORDER BY: junk from previous rows for query on I_S
    
    Intruduced new field in Field_varstring. Basically just putting a name for the concept
of
    maximum length for VARCHARs.

  sql/item_sum.cc@stripped, 2007-04-25 15:56:24+03:00, mhansson@stripped +9 -3
    Bug #23856:GROUP_CONCAT and ORDER BY: junk from previous rows for query on I_S
    
    The fix for the bug and motivation.
    Removed the erroneous conclusion that caused the bug.

  sql/sql_select.cc@stripped, 2007-04-25 15:56:24+03:00, mhansson@stripped +5 -3
    Bug #23856:GROUP_CONCAT and ORDER BY: junk from previous rows for query on I_S
    
    Fixed a comment that is simply not true. 
    Using the new field MAX_VALUE. Using UINT_MAX16 here is unclear.

  sql/sql_string.cc@stripped, 2007-04-25 15:56:24+03:00, mhansson@stripped +2 -2
    Bug #23856:GROUP_CONCAT and ORDER BY: junk from previous rows for query on I_S
    
    Fixed typo.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	mhansson
# Host:	linux-st28.site
# Root:	/home/martin/mysql/src/5.0o-bug23856

--- 1.200/sql/field.h	2007-04-02 11:50:16 +03:00
+++ 1.201/sql/field.h	2007-04-25 15:56:23 +03:00
@@ -1076,6 +1076,7 @@ public:
 
 class Field_varstring :public Field_longstr {
 public:
+  static const int MAX_SIZE= UINT_MAX16;
   /* Store number of bytes used to store length (1 or 2) */
   uint32 length_bytes;
   Field_varstring(char *ptr_arg,

--- 1.205/sql/item_sum.cc	2007-04-02 11:50:17 +03:00
+++ 1.206/sql/item_sum.cc	2007-04-25 15:56:24 +03:00
@@ -3257,14 +3257,20 @@ bool Item_func_group_concat::setup(THD *
   tmp_table_param->force_copy_fields= force_copy_fields;
   DBUG_ASSERT(table == 0);
   /*
+    Currently we have to force conversion of BLOB values to VARCHAR's
+    for GROUP_CONCAT if we have a GROUP CONCAT( x ORDER BY y ), since
+    BLOB's cannot be stored inside the TREE objects used for
+    sorting. This leads to truncation if the BLOB's size exceeds
+    Field_varstring::MAX_SIZE.
+  */
+  if (arg_count_order > 0)
+    tmp_table_param->convert_blob_length= Field_varstring::MAX_SIZE - 1;
+  /*
     We have to create a temporary table to get descriptions of fields
     (types, sizes and so on).
 
     Note that in the table, we first have the ORDER BY fields, then the
     field list.
-
-    We need to set set_sum_field in true for storing value of blob in buffer
-    of a record instead of a pointer of one.
   */
   if (!(table= create_tmp_table(thd, tmp_table_param, all_fields,
                                 (ORDER*) 0, 0, TRUE,

--- 1.507/sql/sql_select.cc	2007-04-02 04:56:34 +03:00
+++ 1.508/sql/sql_select.cc	2007-04-25 15:56:24 +03:00
@@ -8711,7 +8711,8 @@ const_expression_in_where(COND *cond, It
 			the record in the original table.
 			If item == NULL then fill_record() will update
 			the temporary table
-    convert_blob_length If >0 create a varstring(convert_blob_length) field 
+    convert_blob_length If inside the interval (0, Field_varstring::MAX_SIZE)
+                        create a varstring(convert_blob_length) field 
                         instead of blob.
 
   RETURN
@@ -8729,7 +8730,7 @@ Field* create_tmp_field_from_field(THD *
     Make sure that the blob fits into a Field_varstring which has 
     2-byte lenght. 
   */
-  if (convert_blob_length && convert_blob_length < UINT_MAX16 &&
+  if (convert_blob_length && convert_blob_length < Field_varstring::MAX_SIZE
&&
       (org_field->flags & BLOB_FLAG))
     new_field= new Field_varstring(convert_blob_length,
                                    org_field->maybe_null(),
@@ -8898,7 +8899,8 @@ Field *create_tmp_field_for_schema(THD *
 			the record in the original table.
 			If modify_item is 0 then fill_record() will update
 			the temporary table
-    convert_blob_length If >0 create a varstring(convert_blob_length) field 
+    convert_blob_length If inside the interval (0, Field_varstring::MAX_SIZE)
+                        create a varstring(convert_blob_length) field 
                         instead of blob.
 
   RETURN

--- 1.97/sql/sql_string.cc	2007-01-22 14:10:41 +02:00
+++ 1.98/sql/sql_string.cc	2007-04-25 15:56:24 +03:00
@@ -858,9 +858,9 @@ outp:
   with optional left padding (for binary -> UCS2 conversion)
   
   SYNOPSIS
-    well_formed_copy_nhars()
+    well_formed_copy_nchars()
     to			     Store result here
-    to_length                Maxinum length of "to" string
+    to_length                Maximum length of "to" string
     to_cs		     Character set of "to" string
     from		     Copy from here
     from_length		     Length of from string

--- 1.68/mysql-test/r/func_gconcat.result	2007-03-29 19:20:02 +03:00
+++ 1.69/mysql-test/r/func_gconcat.result	2007-04-25 15:56:23 +03:00
@@ -737,4 +737,11 @@ SELECT GROUP_CONCAT(DISTINCT UCASE(b)) F
 GROUP_CONCAT(DISTINCT UCASE(b))
 ONE.1,TWO.2,ONE.3
 DROP TABLE t1;
+set group_concat_max_len= 32768;
+set names utf8;
+create table t1 (a text character set utf8, b integer);
+insert into t1 values (repeat('a', 4), 0), (repeat(_utf8 0xc3b7, 4), 1);
+select group_concat(a order by b) from t1;
+group_concat(a order by b)
+aaaa,÷÷÷÷
 End of 5.0 tests

--- 1.55/mysql-test/t/func_gconcat.test	2007-03-29 19:20:02 +03:00
+++ 1.56/mysql-test/t/func_gconcat.test	2007-04-25 15:56:23 +03:00
@@ -507,4 +507,13 @@ SELECT GROUP_CONCAT(DISTINCT UCASE(a)) F
 SELECT GROUP_CONCAT(DISTINCT UCASE(b)) FROM t1;
 DROP TABLE t1;
 
+#
+# Bug #23856:GROUP_CONCAT and ORDER BY: junk from previous rows for query on I_S
+#
+set group_concat_max_len= 32768;
+set names utf8;
+create table t1 (a text character set utf8, b integer);
+insert into t1 values (repeat('a', 4), 0), (repeat(_utf8 0xc3b7, 4), 1);
+select group_concat(a order by b) from t1;
+
 --echo End of 5.0 tests