From f78b870c9bd11d5e25f6d423718b8f623f712377 Mon Sep 17 00:00:00 2001 From: Igor Babaev Date: Mon, 19 Jul 2010 22:41:24 -0700 Subject: [PATCH] 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. --- mysql-test/suite/vcol/r/vcol_misc.result | 10 ++++++++++ mysql-test/suite/vcol/t/vcol_misc.test | 15 +++++++++++++++ sql/field.cc | 2 +- sql/filesort.cc | 8 +++++--- sql/sql_insert.cc | 9 ++++++--- sql/sql_select.cc | 8 ++++---- sql/table.cc | 14 +++++++------- sql/table.h | 24 +++++++++++++++++++++--- 8 files changed, 69 insertions(+), 21 deletions(-) diff --git a/mysql-test/suite/vcol/r/vcol_misc.result b/mysql-test/suite/vcol/r/vcol_misc.result index 8ee9f5a9f67..a0a036f1900 100644 --- a/mysql-test/suite/vcol/r/vcol_misc.result +++ b/mysql-test/suite/vcol/r/vcol_misc.result @@ -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; diff --git a/mysql-test/suite/vcol/t/vcol_misc.test b/mysql-test/suite/vcol/t/vcol_misc.test index 87a870181b0..214b3dce612 100644 --- a/mysql-test/suite/vcol/t/vcol_misc.test +++ b/mysql-test/suite/vcol/t/vcol_misc.test @@ -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; diff --git a/sql/field.cc b/sql/field.cc index 671e05f6647..f621dab7539 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -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 diff --git a/sql/filesort.cc b/sql/filesort.cc index 73cc0e4d593..3f2e2e55046 100644 --- a/sql/filesort.cc +++ b/sql/filesort.cc @@ -515,7 +515,7 @@ static ha_rows find_all_keys(SORTPARAM *param, SQL_SELECT *select, 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 *param, SQL_SELECT *select, /* 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 *param, SQL_SELECT *select, 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 *param, SQL_SELECT *select, 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) diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 4ce13374d03..44dc7308292 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -2109,7 +2109,7 @@ TABLE *Delayed_insert::get_local_table(THD* client_thd) 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(THD* client_thd) /* 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(THD* client_thd) 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); diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 2a67259f91d..ef2efc62a3b 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -5513,7 +5513,7 @@ static void calc_used_field_length(THD *thd, JOIN_TAB *join_tab) { 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(TABLE *table, uchar *bitmaps) 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 */ diff --git a/sql/table.cc b/sql/table.cc index af515090ee4..7db7e510ea0 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -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 *field) { 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, TABLE *table, bool for_write) 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 */ diff --git a/sql/table.h b/sql/table.h index fa791fa6939..d2fe7ba3261 100644 --- a/sql/table.h +++ b/sql/table.h @@ -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; }