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-05-16 10:53:51+03:00, mhansson@stripped +6 -0
  Bug #23856:GROUP_CONCAT and ORDER BY: junk from previous rows for query on I_S
  
  Problem:
  When group_concat() is given an ORDER BY or DISTINCT option, it uses a
  TREE structure to sort/remove duplicates. The current implementation
  incorrectly tries to store BLOB's inside the TREE structure for sorting,
  because at present BLOB Fields always keep a pointer to the actual data
  and not the data itself.
  
  In particular Item_func_group_concat::setup calls create_tmp_table with
  save_sum_fields == TRUE. According to a (removed) comment this should 
  lead to blobs being stored inside the TREE, which is wrong. This flag's 
  only effect wrt GROUP_BY is that the blob is stored using do_save_blob 
  rather than do_conv_blob. There seems to be no difference between them.
  A to do comment is left above Field_copy::set for future work. 
  ( version >= 5.1 )
  
  Solution:
  Always use VARCHAR for the placeholder field. This leads to results being
  truncated when they exceed the maximum VARCHAR length (65535 bytes).
  In such cases we print a warning. See also Bug#28273: GROUP_CONCAT and 
  ORDER BY: No warning when result gets truncated.

  mysql-test/r/func_gconcat.result@stripped, 2007-05-16 10:53:47+03:00,
mhansson@stripped +47 -0
    Bug #23856: correct result.

  mysql-test/t/func_gconcat.test@stripped, 2007-05-16 10:53:47+03:00, mhansson@stripped
+32 -0
    Bug #23856: test case.

  sql/field.h@stripped, 2007-05-16 10:53:47+03:00, mhansson@stripped +5 -0
    Bug #23856: Introduced new constant field in Field_varstring for the maximum
    VARCHAR size.

  sql/field_conv.cc@stripped, 2007-05-16 10:53:47+03:00, mhansson@stripped +14 -0
    Bug #23856: Added to do comment about

  sql/item_sum.cc@stripped, 2007-05-16 10:53:47+03:00, mhansson@stripped +10 -4
    Bug #23856: 
    - changed UINT_MAX16 to Field_varstring::MAX_SIZE
    - The fix for the bug and motivation. 
    - Removed comment containing the erroneous assumption that caused the bug.

  sql/sql_select.cc@stripped, 2007-05-16 10:53:47+03:00, mhansson@stripped +3 -2
    Bug#23856:
    - Clarified and doxygenated comment regarding convert_blob_length (2 places)
    - Used Field_varstring::MAX_SIZE rather that UINT_MAX16. (2 places)

# 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-05-16 10:53:47 +03:00
@@ -1076,6 +1076,11 @@ public:
 
 class Field_varstring :public Field_longstr {
 public:
+  /*
+    The maximum space available in a Field_varstring, in bytes. See
+    length_bytes.
+  */
+  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.63/sql/field_conv.cc	2007-03-30 17:08:54 +03:00
+++ 1.64/sql/field_conv.cc	2007-05-16 10:53:47 +03:00
@@ -504,7 +504,21 @@ void Copy_field::set(char *to,Field *fro
 }
 
 
+/*
+  To do: 
 
+  If 'save\ is set to true and the 'from' is a blob field, do_copy is set to
+  do_save_blob rather than do_conv_blob.  The only differences between them
+  appears to be:
+
+  - do_save_blob allocates and uses an intermediate buffer before calling 
+    Field_blob::store. Is this in order to trigger the call to 
+    well_formed_copy_nchars, by changing the pointer copy->tmp.ptr()?
+    That call will take place anyway in all known cases.
+
+  - The above causes a truncation to MAX_FIELD_WIDTH. Is this the intended 
+    effect? Truncation is handled by well_formed_copy_nchars anyway.
+ */
 void Copy_field::set(Field *to,Field *from,bool save)
 {
   if (to->type() == FIELD_TYPE_NULL)

--- 1.205/sql/item_sum.cc	2007-04-02 11:50:17 +03:00
+++ 1.206/sql/item_sum.cc	2007-05-16 10:53:47 +03:00
@@ -420,7 +420,7 @@ Field *Item_sum::create_tmp_field(bool g
       2-byte lenght. 
     */
     if (max_length/collation.collation->mbmaxlen > 255 && 
-        convert_blob_length < UINT_MAX16 && convert_blob_length)
+        convert_blob_length <= Field_varstring::MAX_SIZE &&
convert_blob_length)
       return new Field_varstring(convert_blob_length, maybe_null,
                                  name, table,
                                  collation.collation);
@@ -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
+    if we are to store them in TREE objects used for ORDER BY and
+    DISTINCT. This leads to truncation if the BLOB's size exceeds
+    Field_varstring::MAX_SIZE.
+  */
+  if (arg_count_order > 0 || distinct)
+    set_if_smaller(tmp_table_param->convert_blob_length, 
+                   Field_varstring::MAX_SIZE);
+  /*
     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-05-16 10:53:47 +03:00
@@ -8729,7 +8729,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(),
@@ -8820,7 +8820,8 @@ static Field *create_tmp_field_from_item
       2-byte lenght. 
     */
     else if (item->max_length/item->collation.collation->mbmaxlen > 255
&&
-             convert_blob_length < UINT_MAX16 && convert_blob_length)
+             convert_blob_length <= Field_varstring::MAX_SIZE && 
+             convert_blob_length)
       new_field= new Field_varstring(convert_blob_length, maybe_null,
                                      item->name, table,
                                      item->collation.collation);

--- 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-05-16 10:53:47 +03:00
@@ -737,4 +737,51 @@ 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= 65535;
+CREATE TABLE t1( a TEXT, b INTEGER );
+INSERT INTO t1 VALUES ( 'a', 0 ), ( 'b', 1 );
+SELECT GROUP_CONCAT( a ORDER BY b ) FROM t1;
+GROUP_CONCAT( a ORDER BY b )
+a,b
+SELECT GROUP_CONCAT(DISTINCT a ORDER BY b) FROM t1;
+GROUP_CONCAT(DISTINCT a ORDER BY b)
+a,b
+SELECT GROUP_CONCAT(DISTINCT a) FROM t1;
+GROUP_CONCAT(DISTINCT a)
+a,b
+SET group_concat_max_len= 10;
+SELECT GROUP_CONCAT(a ORDER BY b) FROM t1;
+GROUP_CONCAT(a ORDER BY b)
+a,b
+SELECT GROUP_CONCAT(DISTINCT a ORDER BY b) FROM t1;
+GROUP_CONCAT(DISTINCT a ORDER BY b)
+a,b
+SELECT GROUP_CONCAT(DISTINCT a) FROM t1;
+GROUP_CONCAT(DISTINCT a)
+a,b
+SET group_concat_max_len= 65535;
+CREATE TABLE t2( a TEXT );
+INSERT INTO t2 VALUES( REPEAT( 'a', 5000 ) );
+INSERT INTO t2 VALUES( REPEAT( 'b', 5000 ) );
+INSERT INTO t2 VALUES( REPEAT( 'a', 5000 ) );
+SELECT LENGTH( GROUP_CONCAT( DISTINCT a ) ) FROM t2;
+LENGTH( GROUP_CONCAT( DISTINCT a ) )
+10001
+CREATE TABLE t3( a TEXT, b INT  );
+INSERT INTO t3 VALUES( REPEAT( 'a', 65534 ), 1 );
+INSERT INTO t3 VALUES( REPEAT( 'a', 65535 ), 2 );
+INSERT INTO t3 VALUES( REPEAT( 'a', 65536 ), 3 );
+Warnings:
+Warning	1265	Data truncated for column 'a' at row 1
+SELECT LENGTH( GROUP_CONCAT( a ) ) FROM t3 WHERE b = 1;
+LENGTH( GROUP_CONCAT( a ) )
+65534
+SELECT LENGTH( GROUP_CONCAT( a ) ) FROM t3 WHERE b = 2;
+LENGTH( GROUP_CONCAT( a ) )
+65535
+SELECT LENGTH( GROUP_CONCAT( a ) ) FROM t3 WHERE b = 3;
+LENGTH( GROUP_CONCAT( a ) )
+65535
+SET group_concat_max_len= DEFAULT;
+DROP TABLE t1, t2, t3;
 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-05-16 10:53:47 +03:00
@@ -507,4 +507,36 @@ 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= 65535;
+CREATE TABLE t1( a TEXT, b INTEGER );
+INSERT INTO t1 VALUES ( 'a', 0 ), ( 'b', 1 );
+SELECT GROUP_CONCAT( a ORDER BY b ) FROM t1;
+SELECT GROUP_CONCAT(DISTINCT a ORDER BY b) FROM t1;
+SELECT GROUP_CONCAT(DISTINCT a) FROM t1;
+SET group_concat_max_len= 10;
+SELECT GROUP_CONCAT(a ORDER BY b) FROM t1;
+SELECT GROUP_CONCAT(DISTINCT a ORDER BY b) FROM t1;
+SELECT GROUP_CONCAT(DISTINCT a) FROM t1;
+
+SET group_concat_max_len= 65535;
+CREATE TABLE t2( a TEXT );
+INSERT INTO t2 VALUES( REPEAT( 'a', 5000 ) );
+INSERT INTO t2 VALUES( REPEAT( 'b', 5000 ) );
+INSERT INTO t2 VALUES( REPEAT( 'a', 5000 ) );
+SELECT LENGTH( GROUP_CONCAT( DISTINCT a ) ) FROM t2;
+
+CREATE TABLE t3( a TEXT, b INT  );
+INSERT INTO t3 VALUES( REPEAT( 'a', 65534 ), 1 );
+INSERT INTO t3 VALUES( REPEAT( 'a', 65535 ), 2 );
+INSERT INTO t3 VALUES( REPEAT( 'a', 65536 ), 3 );
+SELECT LENGTH( GROUP_CONCAT( a ) ) FROM t3 WHERE b = 1;
+SELECT LENGTH( GROUP_CONCAT( a ) ) FROM t3 WHERE b = 2;
+SELECT LENGTH( GROUP_CONCAT( a ) ) FROM t3 WHERE b = 3;
+
+SET group_concat_max_len= DEFAULT;
+DROP TABLE t1, t2, t3;
+
 --echo End of 5.0 tests