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-26 17:35:35+03:00, mhansson@stripped +5 -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."
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 they always keep
a pointer to the actual data.
So the fix is to always 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.
mysql-test/r/func_gconcat.result@stripped, 2007-04-26 17:35:32+03:00, mhansson@stripped +32 -0
Bug #23856: correct result.
mysql-test/t/func_gconcat.test@stripped, 2007-04-26 17:35:33+03:00, mhansson@stripped +24 -0
Bug #23856: test case.
sql/field.h@stripped, 2007-04-26 17:35:33+03:00, mhansson@stripped +5 -0
Bug #23856: Introduced new field in Field_varstring. Basically just
giving the maximum VARCHAR size a name.
sql/item_sum.cc@stripped, 2007-04-26 17:35:33+03:00, mhansson@stripped +11 -4
Bug #23856:
- changed UINT_MAX16 to Field_varstring::MAX_SIZE
- The fix for the bug and motivation.
- Removed the erroneous conclusion that caused the bug.
sql/sql_select.cc@stripped, 2007-04-26 17:35:33+03:00, mhansson@stripped +37 -35
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-04-26 17:35:33 +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.205/sql/item_sum.cc 2007-04-02 11:50:17 +03:00
+++ 1.206/sql/item_sum.cc 2007-04-26 17:35:33 +03:00
@@ -420,7 +420,8 @@ 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 +3258,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 - 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-26 17:35:33 +03:00
@@ -8699,22 +8699,22 @@ const_expression_in_where(COND *cond, It
/*
Create field for temporary table from given field
- SYNOPSIS
- create_tmp_field_from_field()
- thd Thread handler
- org_field field from which new field will be created
- name New field name
- table Temporary table
- item !=NULL if item->result_field should point to new field.
+ @param thd Thread handler
+ @param org_field field from which new field will be created
+ @param name New field name
+ @param table Temporary table
+ @param item !=NULL if item->result_field should point to new field.
This is relevant for how fill_record() is going to work:
If item != NULL then fill_record() will update
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
- instead of blob.
+ @param convert_blob_length If inside the interval
+ (0, Field_varstring::MAX_SIZE) create a
+ varstring(convert_blob_length) field instead of
+ blob.
- RETURN
+ @return
0 on error
new_created field
*/
@@ -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);
@@ -8879,31 +8880,32 @@ Field *create_tmp_field_for_schema(THD *
/*
Create field for temporary table
- SYNOPSIS
- create_tmp_field()
- thd Thread handler
- table Temporary table
- item Item to create a field for
- type Type of item (normally item->type)
- copy_func If set and item is a function, store copy of item
- in this array
- from_field if field will be created using other field as example,
+ @param thd Thread handler
+ @param table Temporary table
+ @param item Item to create a field for
+ @param type Type of item (normally item->type)
+ @param copy_func If set and item is a function, store copy of item
+ in this array
+
+ @param from_field if field will be created using other field as example,
pointer example field will be written here
- default_field If field has a default value field, store it here
- group 1 if we are going to do a relative group by on result
- modify_item 1 if item->result_field should point to new item.
- This is relevent for how fill_record() is going to
- work:
- If modify_item is 1 then fill_record() will update
- 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
- instead of blob.
- RETURN
- 0 on error
- new_created field
+ @param default_field If field has a default value field, store it here
+ @param group 1 if we are going to do a relative group by on result
+ @param modify_item 1 if item->result_field should point to new item.
+ This is relevent for how fill_record() is going to work:
+ If modify_item is 1 then fill_record() will update
+ the record in the original table.
+ If modify_item is 0 then fill_record() will update
+ the temporary table
+
+ @param convert_blob_length If inside the interval (0, Field_varstring::MAX_SIZE)
+ create a varstring(convert_blob_length) field
+ instead of blob.
+
+ @return
+ 0 on error
+ new_created field
*/
Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type,
--- 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-26 17:35:32 +03:00
@@ -737,4 +737,36 @@ 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( x 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 x ) ) FROM t2;
+LENGTH( GROUP_CONCAT( DISTINCT x ) )
+10001
+SET group_concat_max_len= DEFAULT;
+DROP TABLE t1, t2;
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-26 17:35:33 +03:00
@@ -507,4 +507,28 @@ 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( x 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 x ) ) FROM t2;
+
+SET group_concat_max_len= DEFAULT;
+DROP TABLE t1, t2;
+
--echo End of 5.0 tests
| Thread |
|---|
| • bk commit into 5.0 tree (mhansson:1.2443) BUG#23856 | mhansson | 26 Apr |