Hello Igor, Ok to push. On Mon, Jul 19, 2010 at 10:43:18PM -0700, Igor Babaev wrote:

Sergey,

Please review this patch for the 5.2 tree.

Regards, Igor.

-------- Original Message -------- Subject: [Commits] bzr commit into Mariadb 5.2, with Maria 2.0:maria/5.2 branch (igor:2827) Bug#607566 Date: Mon, 19 Jul 2010 22:41:37 -0700 (PDT) From: Igor Babaev Reply-To: maria-developers@lists.launchpad.net To: commits@mariadb.org

#At lp:maria/5.2 based on revid:igor@askmonty.org-20100717195808-mvh782jvt6c32u2d

2827 Igor Babaev 2010-07-19 Fixed bug #607566. For queries with order by clauses that employed filesort usage of virtual column references in select lists could trigger assertion failures. It happened because a wrong vcol_set bitmap was used for filesort. It turned out that filesort required its own vcol_set bitmap.

Made management of the vcol_set bitmaps similar to the management of the read_set and write_set bitmaps. modified: mysql-test/suite/vcol/r/vcol_misc.result mysql-test/suite/vcol/t/vcol_misc.test sql/field.cc sql/filesort.cc sql/sql_insert.cc sql/sql_select.cc sql/table.cc sql/table.h

=== modified file 'mysql-test/suite/vcol/r/vcol_misc.result' --- a/mysql-test/suite/vcol/r/vcol_misc.result 2010-07-17 19:58:08 +0000 +++ b/mysql-test/suite/vcol/r/vcol_misc.result 2010-07-20 05:41:24 +0000 @@ -87,3 +87,13 @@ a v 2002-02-15 00:00:00 0 2000-10-15 00:00:00 1 DROP TABLE t1, t2; +CREATE TABLE t1 ( +a char(255), b char(255), c char(255), d char(255), +v char(255) AS (CONCAT(c,d) ) VIRTUAL +); +INSERT INTO t1(a,b,c,d) VALUES ('w','x','y','z'), ('W','X','Y','Z'); +SELECT v FROM t1 ORDER BY CONCAT(a,b); +v +yz +YZ +DROP TABLE t1;

=== modified file 'mysql-test/suite/vcol/t/vcol_misc.test' --- a/mysql-test/suite/vcol/t/vcol_misc.test 2010-07-17 19:58:08 +0000 +++ b/mysql-test/suite/vcol/t/vcol_misc.test 2010-07-20 05:41:24 +0000 @@ -87,3 +87,18 @@ INSERT INTO t2(a) VALUES ('2000-10-15'); SELECT * FROM t2;

DROP TABLE t1, t2; + +# +# Bug#607566: Virtual column in the select list of SELECT with ORDER BY +# + +CREATE TABLE t1 ( + a char(255), b char(255), c char(255), d char(255), + v char(255) AS (CONCAT(c,d) ) VIRTUAL +); + +INSERT INTO t1(a,b,c,d) VALUES ('w','x','y','z'), ('W','X','Y','Z'); + +SELECT v FROM t1 ORDER BY CONCAT(a,b); + +DROP TABLE t1;

=== modified file 'sql/field.cc' --- a/sql/field.cc 2010-06-01 19:52:20 +0000 +++ b/sql/field.cc 2010-07-20 05:41:24 +0000 @@ -57,7 +57,7 @@ const char field_separator=','; ((ulong) ((LL(1) << min(arg, 4) * 8) - LL(1)))

#define ASSERT_COLUMN_MARKED_FOR_READ DBUG_ASSERT(!table || (!table->read_set || bitmap_is_set(table->read_set, field_index))) -#define ASSERT_COLUMN_MARKED_FOR_WRITE_OR_COMPUTED DBUG_ASSERT(!table || (!table->write_set || bitmap_is_set(table->write_set, field_index) || bitmap_is_set(&table->vcol_set, field_index))) +#define ASSERT_COLUMN_MARKED_FOR_WRITE_OR_COMPUTED DBUG_ASSERT(!table || (!table->write_set || bitmap_is_set(table->write_set, field_index) || bitmap_is_set(table->vcol_set, field_index)))

/* Rules for merging different types of fields in UNION

=== modified file 'sql/filesort.cc' --- a/sql/filesort.cc 2010-07-17 19:58:08 +0000 +++ b/sql/filesort.cc 2010-07-20 05:41:24 +0000 @@ -515,7 +515,7 @@ static ha_rows find_all_keys(SORTPARAM * THD *thd= current_thd; volatile THD::killed_state *killed= &thd->killed; handler *file; - MY_BITMAP *save_read_set, *save_write_set; + MY_BITMAP *save_read_set, *save_write_set, *save_vcol_set; DBUG_ENTER("find_all_keys"); DBUG_PRINT("info",("using: %s", (select ? select->quick ? "ranges" : "where": @@ -552,6 +552,7 @@ static ha_rows find_all_keys(SORTPARAM * /* Remember original bitmaps */ save_read_set= sort_form->read_set; save_write_set= sort_form->write_set; + save_vcol_set= sort_form->vcol_set; /* Set up temporary column read map for columns used by sort */ bitmap_clear_all(&sort_form->tmp_set); /* Temporary set for register_used_fields and register_field_in_read_map */ @@ -560,7 +561,8 @@ static ha_rows find_all_keys(SORTPARAM * if (select && select->cond) select->cond->walk(&Item::register_field_in_read_map, 1, (uchar*) sort_form); - sort_form->column_bitmaps_set(&sort_form->tmp_set, &sort_form->tmp_set); + sort_form->column_bitmaps_set(&sort_form->tmp_set, &sort_form->tmp_set, + &sort_form->tmp_set);

for (;;) { @@ -643,7 +645,7 @@ static ha_rows find_all_keys(SORTPARAM * DBUG_RETURN(HA_POS_ERROR);

/* Signal we should use orignal column read and write maps */ - sort_form->column_bitmaps_set(save_read_set, save_write_set); + sort_form->column_bitmaps_set(save_read_set, save_write_set, save_vcol_set);

DBUG_PRINT("test",("error: %d indexpos: %d",error,indexpos)); if (error != HA_ERR_END_OF_FILE)

=== modified file 'sql/sql_insert.cc' --- a/sql/sql_insert.cc 2010-07-15 23:51:05 +0000 +++ b/sql/sql_insert.cc 2010-07-20 05:41:24 +0000 @@ -2109,7 +2109,7 @@ TABLE *Delayed_insert::get_local_table(T copy= (TABLE*) client_thd->alloc(sizeof(*copy)+ (share->fields+1)*sizeof(Field**)+ share->reclength + - share->column_bitmap_size*2); + share->column_bitmap_size*3); if (!copy) goto error;

@@ -2119,7 +2119,7 @@ TABLE *Delayed_insert::get_local_table(T /* Assign the pointers for the field pointers array and the record. */ field= copy->field= (Field**) (copy + 1); bitmap= (uchar*) (field + share->fields + 1); - copy->record[0]= (bitmap + share->column_bitmap_size * 2); + copy->record[0]= (bitmap + share->column_bitmap_size*3); memcpy((char*) copy->record[0], (char*) table->record[0], share->reclength); /* Make a copy of all fields. @@ -2161,10 +2161,13 @@ TABLE *Delayed_insert::get_local_table(T copy->def_read_set.bitmap= (my_bitmap_map*) bitmap; copy->def_write_set.bitmap= ((my_bitmap_map*) (bitmap + share->column_bitmap_size)); + copy->def_vcol_set.bitmap= ((my_bitmap_map*) + (bitmap + 2*share->column_bitmap_size)); copy->tmp_set.bitmap= 0; // To catch errors - bzero((char*) bitmap, share->column_bitmap_size*2); + bzero((char*) bitmap, share->column_bitmap_size*3); copy->read_set= ©->def_read_set; copy->write_set= ©->def_write_set; + copy->vcol_set= ©->def_vcol_set;

DBUG_RETURN(copy);

=== modified file 'sql/sql_select.cc' --- a/sql/sql_select.cc 2010-07-17 19:58:08 +0000 +++ b/sql/sql_select.cc 2010-07-20 05:41:24 +0000 @@ -5513,7 +5513,7 @@ static void calc_used_field_length(THD * { uint null_fields,blobs,fields,rec_length; Field **f_ptr,*field; - MY_BITMAP *read_set= join_tab->table->read_set;; + MY_BITMAP *read_set= join_tab->table->read_set;

null_fields= blobs= fields= rec_length=0; for (f_ptr=join_tab->table->field ; (field= *f_ptr) ; f_ptr++) @@ -9877,11 +9877,11 @@ void setup_tmp_table_column_bitmaps(TABL uint field_count= table->s->fields; bitmap_init(&table->def_read_set, (my_bitmap_map*) bitmaps, field_count, FALSE); - bitmap_init(&table->tmp_set, + bitmap_init(&table->def_vcol_set, (my_bitmap_map*) (bitmaps+ bitmap_buffer_size(field_count)), field_count, FALSE); - bitmap_init(&table->vcol_set, - (my_bitmap_map*) (bitmaps+ 2+bitmap_buffer_size(field_count)), + bitmap_init(&table->tmp_set, + (my_bitmap_map*) (bitmaps+ 2*bitmap_buffer_size(field_count)), field_count, FALSE);

/* write_set and all_set are copies of read_set */

=== modified file 'sql/table.cc' --- a/sql/table.cc 2010-07-17 19:58:08 +0000 +++ b/sql/table.cc 2010-07-20 05:41:24 +0000 @@ -2343,9 +2343,9 @@ partititon_err: (my_bitmap_map*) bitmaps, share->fields, FALSE); bitmap_init(&outparam->def_write_set, (my_bitmap_map*) (bitmaps+bitmap_size), share->fields, FALSE); - bitmap_init(&outparam->tmp_set, + bitmap_init(&outparam->def_vcol_set, (my_bitmap_map*) (bitmaps+bitmap_size*2), share->fields, FALSE); - bitmap_init(&outparam->vcol_set, + bitmap_init(&outparam->tmp_set, (my_bitmap_map*) (bitmaps+bitmap_size*3), share->fields, FALSE); outparam->default_column_bitmaps();

@@ -4809,10 +4809,10 @@ void st_table::clear_column_bitmaps() Reset column read/write usage. It's identical to: bitmap_clear_all(&table->def_read_set); bitmap_clear_all(&table->def_write_set); + bitmap_clear_all(&table->def_vcol_set); */ - bzero((char*) def_read_set.bitmap, s->column_bitmap_size*2); - bzero((char*) def_read_set.bitmap, s->column_bitmap_size*4); - column_bitmaps_set(&def_read_set, &def_write_set); + bzero((char*) def_read_set.bitmap, s->column_bitmap_size*3); + column_bitmaps_set(&def_read_set, &def_write_set, &def_vcol_set); }

@@ -5085,7 +5085,7 @@ bool st_table::mark_virtual_col(Field *f { bool res; DBUG_ASSERT(field->vcol_info); - if (!(res= bitmap_fast_test_and_set(&vcol_set, field->field_index))) + if (!(res= bitmap_fast_test_and_set(vcol_set, field->field_index))) { Item *vcol_item= field->vcol_info->expr_item; DBUG_ASSERT(vcol_item); @@ -5464,7 +5464,7 @@ int update_virtual_fields(THD *thd, TABL vfield= (*vfield_ptr); DBUG_ASSERT(vfield->vcol_info && vfield->vcol_info->expr_item); /* Only update those fields that are marked in the vcol_set bitmap */ - if (bitmap_is_set(&table->vcol_set, vfield->field_index) && + if (bitmap_is_set(table->vcol_set, vfield->field_index) && (for_write || !vfield->stored_in_db)) { /* Compute the actual value of the virtual fields */

=== modified file 'sql/table.h' --- a/sql/table.h 2010-07-17 19:58:08 +0000 +++ b/sql/table.h 2010-07-20 05:41:24 +0000 @@ -719,9 +719,8 @@ struct st_table { const char *alias; /* alias or table name */ uchar *null_flags; my_bitmap_map *bitmap_init_value; - MY_BITMAP def_read_set, def_write_set, tmp_set; /* containers */ - MY_BITMAP vcol_set; /* set of used virtual columns */ - MY_BITMAP *read_set, *write_set; /* Active column sets */ + MY_BITMAP def_read_set, def_write_set, def_vcol_set, tmp_set; + MY_BITMAP *read_set, *write_set, *vcol_set; /* Active column sets */ /* The ID of the query that opened and is using this table. Has different meanings depending on the table type. @@ -904,12 +903,30 @@ struct st_table { if (file) file->column_bitmaps_signal(); } + inline void column_bitmaps_set(MY_BITMAP *read_set_arg, + MY_BITMAP *write_set_arg, + MY_BITMAP *vcol_set_arg) + { + read_set= read_set_arg; + write_set= write_set_arg; + vcol_set= vcol_set_arg; + if (file) + file->column_bitmaps_signal(); + } inline void column_bitmaps_set_no_signal(MY_BITMAP *read_set_arg, MY_BITMAP *write_set_arg) { read_set= read_set_arg; write_set= write_set_arg; } + inline void column_bitmaps_set_no_signal(MY_BITMAP *read_set_arg, + MY_BITMAP *write_set_arg, + MY_BITMAP *vcol_set_arg) + { + read_set= read_set_arg; + write_set= write_set_arg; + vcol_set= vcol_set_arg; + } inline void use_all_columns() { column_bitmaps_set(&s->all_set, &s->all_set); @@ -918,6 +935,7 @@ struct st_table { { read_set= &def_read_set; write_set= &def_write_set; + vcol_set= &def_vcol_set; } /* Is table open or should be treated as such by name-locking? */ inline bool is_name_opened() { return db_stat || open_placeholder; }

_______________________________________________ commits mailing list commits@mariadb.org https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

-- BR Sergey -- Sergey Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog