From 013ba71dfdd6f9d49730c1a918d1d089976faefd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 25 Oct 2011 17:33:38 +0300 Subject: [PATCH] Bug#13002783 PARTIALLY UNINITIALIZED CASCADE UPDATE VECTOR In the ON UPDATE CASCADE clause of FOREIGN KEY constraints, the calculated update vector was not fully initialized. This bug was introduced in the InnoDB Plugin when implementing support for ROW_FORMAT=DYNAMIC. Additionally, the data type information was not initialized, but apparently it has never been needed in this case. Nevertheless, it is not good programming practice to pass uninitialized values around. calc_row_difference(): Declare the update field uninitialized in Valgrind. Copy the data type information as well, except when the field is SQL NULL. In the built-in InnoDB, initialize ufield->extern_storage = FALSE (an initialization bug that had gone unnoticed this far). The InnoDB Plugin and later have this flag to dfield_t and have always initialized it properly. row_ins_cascade_calc_update_vec(): Reduce the scope of some pointers. Initialize orig_len. (This caused the bug in InnoDB Plugin and later.) row_ins_foreign_check_on_constraint(): Simplify a condition. Declare the update vector uninitialized. rb:771 approved by Jimmy Yang --- mysql-test/suite/innodb/r/innodb.result | 16 +++++++++++++++- mysql-test/suite/innodb/t/innodb.test | 18 ++++++++++++++++++ mysql-test/suite/innodb_plugin/r/innodb.result | 16 +++++++++++++++- mysql-test/suite/innodb_plugin/t/innodb.test | 18 ++++++++++++++++++ storage/innobase/handler/ha_innodb.cc | 10 +++++++--- storage/innobase/row/row0ins.c | 15 ++++++++------- storage/innodb_plugin/ChangeLog | 7 ++++++- storage/innodb_plugin/handler/ha_innodb.cc | 9 +++++---- storage/innodb_plugin/row/row0ins.c | 17 ++++++++++------- 9 files changed, 102 insertions(+), 24 deletions(-) diff --git a/mysql-test/suite/innodb/r/innodb.result b/mysql-test/suite/innodb/r/innodb.result index c6f6f352743..edf3c7d2753 100644 --- a/mysql-test/suite/innodb/r/innodb.result +++ b/mysql-test/suite/innodb/r/innodb.result @@ -1293,6 +1293,20 @@ ERROR 23000: Cannot delete or update a parent row: a foreign key constraint fail update t3 set t3.id=7 where t1.id =1 and t2.id = t1.id and t3.id = t2.id; ERROR 42S22: Unknown column 't1.id' in 'where clause' drop table t3,t2,t1; +CREATE TABLE t1 ( +c1 VARCHAR(8), c2 VARCHAR(8), +PRIMARY KEY (c1, c2) +) ENGINE=InnoDB; +CREATE TABLE t2 ( +c0 INT PRIMARY KEY, +c1 VARCHAR(8) UNIQUE, +FOREIGN KEY (c1) REFERENCES t1 (c1) ON UPDATE CASCADE +) ENGINE=InnoDB; +INSERT INTO t1 VALUES ('old', 'somevalu'), ('other', 'anyvalue'); +INSERT INTO t2 VALUES (10, 'old'), (20, 'other'); +UPDATE t1 SET c1 = 'other' WHERE c1 = 'old'; +ERROR 23000: Upholding foreign key constraints for table 't1', entry '', key 2 would lead to a duplicate entry +DROP TABLE t2,t1; create table t1( id int primary key, pid int, @@ -1671,7 +1685,7 @@ variable_value - @innodb_rows_deleted_orig 71 SELECT variable_value - @innodb_rows_inserted_orig FROM information_schema.global_status WHERE LOWER(variable_name) = 'innodb_rows_inserted'; variable_value - @innodb_rows_inserted_orig -1063 +1067 SELECT variable_value - @innodb_rows_updated_orig FROM information_schema.global_status WHERE LOWER(variable_name) = 'innodb_rows_updated'; variable_value - @innodb_rows_updated_orig 865 diff --git a/mysql-test/suite/innodb/t/innodb.test b/mysql-test/suite/innodb/t/innodb.test index 480183b9e2d..ef050390b48 100644 --- a/mysql-test/suite/innodb/t/innodb.test +++ b/mysql-test/suite/innodb/t/innodb.test @@ -1021,6 +1021,24 @@ update t1,t2,t3 set t3.id=5, t2.id=6, t1.id=7 where t1.id =1 and t2.id = t1.id update t3 set t3.id=7 where t1.id =1 and t2.id = t1.id and t3.id = t2.id; drop table t3,t2,t1; +# test ON UPDATE CASCADE +CREATE TABLE t1 ( + c1 VARCHAR(8), c2 VARCHAR(8), + PRIMARY KEY (c1, c2) +) ENGINE=InnoDB; + +CREATE TABLE t2 ( + c0 INT PRIMARY KEY, + c1 VARCHAR(8) UNIQUE, + FOREIGN KEY (c1) REFERENCES t1 (c1) ON UPDATE CASCADE +) ENGINE=InnoDB; + +INSERT INTO t1 VALUES ('old', 'somevalu'), ('other', 'anyvalue'); +INSERT INTO t2 VALUES (10, 'old'), (20, 'other'); +-- error ER_FOREIGN_DUPLICATE_KEY +UPDATE t1 SET c1 = 'other' WHERE c1 = 'old'; +DROP TABLE t2,t1; + # # test for recursion depth limit # diff --git a/mysql-test/suite/innodb_plugin/r/innodb.result b/mysql-test/suite/innodb_plugin/r/innodb.result index d3f7a0b9fff..a550b9702bb 100644 --- a/mysql-test/suite/innodb_plugin/r/innodb.result +++ b/mysql-test/suite/innodb_plugin/r/innodb.result @@ -1301,6 +1301,20 @@ ERROR 23000: Cannot delete or update a parent row: a foreign key constraint fail update t3 set t3.id=7 where t1.id =1 and t2.id = t1.id and t3.id = t2.id; ERROR 42S22: Unknown column 't1.id' in 'where clause' drop table t3,t2,t1; +CREATE TABLE t1 ( +c1 VARCHAR(8), c2 VARCHAR(8), +PRIMARY KEY (c1, c2) +) ENGINE=InnoDB; +CREATE TABLE t2 ( +c0 INT PRIMARY KEY, +c1 VARCHAR(8) UNIQUE, +FOREIGN KEY (c1) REFERENCES t1 (c1) ON UPDATE CASCADE +) ENGINE=InnoDB; +INSERT INTO t1 VALUES ('old', 'somevalu'), ('other', 'anyvalue'); +INSERT INTO t2 VALUES (10, 'old'), (20, 'other'); +UPDATE t1 SET c1 = 'other' WHERE c1 = 'old'; +ERROR 23000: Upholding foreign key constraints for table 't1', entry '', key 2 would lead to a duplicate entry +DROP TABLE t2,t1; create table t1( id int primary key, pid int, @@ -1679,7 +1693,7 @@ variable_value - @innodb_rows_deleted_orig 71 SELECT variable_value - @innodb_rows_inserted_orig FROM information_schema.global_status WHERE LOWER(variable_name) = 'innodb_rows_inserted'; variable_value - @innodb_rows_inserted_orig -1067 +1071 SELECT variable_value - @innodb_rows_updated_orig FROM information_schema.global_status WHERE LOWER(variable_name) = 'innodb_rows_updated'; variable_value - @innodb_rows_updated_orig 866 diff --git a/mysql-test/suite/innodb_plugin/t/innodb.test b/mysql-test/suite/innodb_plugin/t/innodb.test index cfc2277d501..1480492cf2a 100644 --- a/mysql-test/suite/innodb_plugin/t/innodb.test +++ b/mysql-test/suite/innodb_plugin/t/innodb.test @@ -1040,6 +1040,24 @@ update t1,t2,t3 set t3.id=5, t2.id=6, t1.id=7 where t1.id =1 and t2.id = t1.id update t3 set t3.id=7 where t1.id =1 and t2.id = t1.id and t3.id = t2.id; drop table t3,t2,t1; +# test ON UPDATE CASCADE +CREATE TABLE t1 ( + c1 VARCHAR(8), c2 VARCHAR(8), + PRIMARY KEY (c1, c2) +) ENGINE=InnoDB; + +CREATE TABLE t2 ( + c0 INT PRIMARY KEY, + c1 VARCHAR(8) UNIQUE, + FOREIGN KEY (c1) REFERENCES t1 (c1) ON UPDATE CASCADE +) ENGINE=InnoDB; + +INSERT INTO t1 VALUES ('old', 'somevalu'), ('other', 'anyvalue'); +INSERT INTO t2 VALUES (10, 'old'), (20, 'other'); +-- error ER_FOREIGN_DUPLICATE_KEY +UPDATE t1 SET c1 = 'other' WHERE c1 = 'old'; +DROP TABLE t2,t1; + # # test for recursion depth limit # diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 2d230e1c297..44b9a7fba36 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -4264,14 +4264,16 @@ calc_row_difference( /* The field has changed */ ufield = uvect->fields + n_changed; + UNIV_MEM_INVALID(ufield, sizeof *ufield); /* Let us use a dummy dfield to make the conversion from the MySQL column format to the InnoDB format */ - dict_col_copy_type_noninline(prebuilt->table->cols + i, - &dfield.type); - if (n_len != UNIV_SQL_NULL) { + dict_col_copy_type_noninline( + prebuilt->table->cols + i, + &dfield.type); + buf = row_mysql_store_col_in_innobase_format( &dfield, (byte*)buf, @@ -4282,11 +4284,13 @@ calc_row_difference( prebuilt->table)); ufield->new_val.data = dfield.data; ufield->new_val.len = dfield.len; + ufield->new_val.type = dfield.type; } else { ufield->new_val.data = NULL; ufield->new_val.len = UNIV_SQL_NULL; } + ufield->extern_storage = FALSE; ufield->exp = NULL; ufield->field_no = dict_col_get_clust_pos_noninline( &prebuilt->table->cols[i], clust_index); diff --git a/storage/innobase/row/row0ins.c b/storage/innobase/row/row0ins.c index 6366beb6b47..f6e6c81534b 100644 --- a/storage/innobase/row/row0ins.c +++ b/storage/innobase/row/row0ins.c @@ -425,11 +425,9 @@ row_ins_cascade_calc_update_vec( dict_table_t* table = foreign->foreign_table; dict_index_t* index = foreign->foreign_index; upd_t* update; - upd_field_t* ufield; dict_table_t* parent_table; dict_index_t* parent_index; upd_t* parent_update; - upd_field_t* parent_ufield; ulint n_fields_updated; ulint parent_field_no; ulint i; @@ -465,12 +463,14 @@ row_ins_cascade_calc_update_vec( dict_index_get_nth_col_no(parent_index, i)); for (j = 0; j < parent_update->n_fields; j++) { - parent_ufield = parent_update->fields + j; + const upd_field_t* parent_ufield + = &parent_update->fields[j]; if (parent_ufield->field_no == parent_field_no) { ulint min_size; const dict_col_t* col; + upd_field_t* ufield; col = dict_index_get_nth_col(index, i); @@ -975,10 +975,9 @@ row_ins_foreign_check_on_constraint( goto nonstandard_exit_func; } - if ((node->is_delete - && (foreign->type & DICT_FOREIGN_ON_DELETE_SET_NULL)) - || (!node->is_delete - && (foreign->type & DICT_FOREIGN_ON_UPDATE_SET_NULL))) { + if (node->is_delete + ? (foreign->type & DICT_FOREIGN_ON_DELETE_SET_NULL) + : (foreign->type & DICT_FOREIGN_ON_UPDATE_SET_NULL)) { /* Build the appropriate update vector which sets foreign->n_fields first fields in rec to SQL NULL */ @@ -987,6 +986,8 @@ row_ins_foreign_check_on_constraint( update->info_bits = 0; update->n_fields = foreign->n_fields; + UNIV_MEM_INVALID(update->fields, + update->n_fields * sizeof *update->fields); for (i = 0; i < foreign->n_fields; i++) { (update->fields + i)->field_no diff --git a/storage/innodb_plugin/ChangeLog b/storage/innodb_plugin/ChangeLog index 68f60c37f1f..4e6e2be615a 100644 --- a/storage/innodb_plugin/ChangeLog +++ b/storage/innodb_plugin/ChangeLog @@ -1,6 +1,11 @@ +2011-10-25 The InnoDB Team + + * handler/ha_innodb.cc, row/row0ins.c: + Fix Bug#13002783 PARTIALLY UNINITIALIZED CASCADE UPDATE VECTOR + 2011-10-20 The InnoDB Team - * btr/brt0cur.c: + * btr/btr0cur.c: Fix Bug#13116045 Compilation failure using GCC 4.6.1 in btr/btr0cur.c 2011-10-12 The InnoDB Team diff --git a/storage/innodb_plugin/handler/ha_innodb.cc b/storage/innodb_plugin/handler/ha_innodb.cc index fc1193e55bb..566c54e0175 100644 --- a/storage/innodb_plugin/handler/ha_innodb.cc +++ b/storage/innodb_plugin/handler/ha_innodb.cc @@ -4971,14 +4971,15 @@ calc_row_difference( /* The field has changed */ ufield = uvect->fields + n_changed; + UNIV_MEM_INVALID(ufield, sizeof *ufield); /* Let us use a dummy dfield to make the conversion from the MySQL column format to the InnoDB format */ - dict_col_copy_type(prebuilt->table->cols + i, - dfield_get_type(&dfield)); - if (n_len != UNIV_SQL_NULL) { + dict_col_copy_type(prebuilt->table->cols + i, + dfield_get_type(&dfield)); + buf = row_mysql_store_col_in_innobase_format( &dfield, (byte*)buf, @@ -4986,7 +4987,7 @@ calc_row_difference( new_mysql_row_col, col_pack_len, dict_table_is_comp(prebuilt->table)); - dfield_copy_data(&ufield->new_val, &dfield); + dfield_copy(&ufield->new_val, &dfield); } else { dfield_set_null(&ufield->new_val); } diff --git a/storage/innodb_plugin/row/row0ins.c b/storage/innodb_plugin/row/row0ins.c index 0f158cdc706..cd135e8ba8f 100644 --- a/storage/innodb_plugin/row/row0ins.c +++ b/storage/innodb_plugin/row/row0ins.c @@ -434,11 +434,9 @@ row_ins_cascade_calc_update_vec( dict_table_t* table = foreign->foreign_table; dict_index_t* index = foreign->foreign_index; upd_t* update; - upd_field_t* ufield; dict_table_t* parent_table; dict_index_t* parent_index; upd_t* parent_update; - upd_field_t* parent_ufield; ulint n_fields_updated; ulint parent_field_no; ulint i; @@ -474,13 +472,15 @@ row_ins_cascade_calc_update_vec( dict_index_get_nth_col_no(parent_index, i)); for (j = 0; j < parent_update->n_fields; j++) { - parent_ufield = parent_update->fields + j; + const upd_field_t* parent_ufield + = &parent_update->fields[j]; if (parent_ufield->field_no == parent_field_no) { ulint min_size; const dict_col_t* col; ulint ufield_len; + upd_field_t* ufield; col = dict_index_get_nth_col(index, i); @@ -493,6 +493,8 @@ row_ins_cascade_calc_update_vec( ufield->field_no = dict_table_get_nth_col_pos( table, dict_col_get_no(col)); + + ufield->orig_len = 0; ufield->exp = NULL; ufield->new_val = parent_ufield->new_val; @@ -993,10 +995,9 @@ row_ins_foreign_check_on_constraint( goto nonstandard_exit_func; } - if ((node->is_delete - && (foreign->type & DICT_FOREIGN_ON_DELETE_SET_NULL)) - || (!node->is_delete - && (foreign->type & DICT_FOREIGN_ON_UPDATE_SET_NULL))) { + if (node->is_delete + ? (foreign->type & DICT_FOREIGN_ON_DELETE_SET_NULL) + : (foreign->type & DICT_FOREIGN_ON_UPDATE_SET_NULL)) { /* Build the appropriate update vector which sets foreign->n_fields first fields in rec to SQL NULL */ @@ -1005,6 +1006,8 @@ row_ins_foreign_check_on_constraint( update->info_bits = 0; update->n_fields = foreign->n_fields; + UNIV_MEM_INVALID(update->fields, + update->n_fields * sizeof *update->fields); for (i = 0; i < foreign->n_fields; i++) { upd_field_t* ufield = &update->fields[i];