From bb56c30ad750465ab140477a1042cb34e714dcc5 Mon Sep 17 00:00:00 2001 From: Venkatesh Duggirala Date: Thu, 19 Nov 2015 13:59:27 +0530 Subject: Bug#17047208 REPLICATION DIFFERENCE FOR MULTIPLE TRIGGERS Problem & Analysis: If DML invokes a trigger or a stored function that inserts into an AUTO_INCREMENT column, that DML has to be marked as 'unsafe' statement. If the tables are locked in the transaction prior to DML statement (using LOCK TABLES), then the same statement is not marked as 'unsafe' statement. The logic of checking whether unsafeness is protected with if (!thd->locked_tables_mode). Hence if we lock the tables prior to DML statement, it is *not* entering into this if condition. Hence the statement is not marked as unsafe statement. Fix: Irrespective of locked_tables_mode value, the unsafeness check should be done. Now with this patch, the code is moved out to 'decide_logging_format()' function where all these checks are happening and also with out 'if(!thd->locked_tables_mode)'. Along with the specified test case in the bug scenario (BINLOG_STMT_UNSAFE_AUTOINC_COLUMNS), we also identified that other cases BINLOG_STMT_UNSAFE_AUTOINC_NOT_FIRST, BINLOG_STMT_UNSAFE_WRITE_AUTOINC_SELECT, BINLOG_STMT_UNSAFE_INSERT_TWO_KEYS are also protected with thd->locked_tables_mode which is not right. All of those checks also moved to 'decide_logging_format()' function. --- sql/sql_class.cc | 131 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) (limited to 'sql/sql_class.cc') diff --git a/sql/sql_class.cc b/sql/sql_class.cc index c3d2a092fd6..05bb38d8358 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -4142,6 +4142,94 @@ void xid_cache_delete(XID_STATE *xid_state) mysql_mutex_unlock(&LOCK_xid_cache); } +/* + Tells if two (or more) tables have auto_increment columns and we want to + lock those tables with a write lock. + + SYNOPSIS + has_two_write_locked_tables_with_auto_increment + tables Table list + + NOTES: + Call this function only when you have established the list of all tables + which you'll want to update (including stored functions, triggers, views + inside your statement). +*/ + +static bool +has_write_table_with_auto_increment(TABLE_LIST *tables) +{ + for (TABLE_LIST *table= tables; table; table= table->next_global) + { + /* we must do preliminary checks as table->table may be NULL */ + if (!table->placeholder() && + table->table->found_next_number_field && + (table->lock_type >= TL_WRITE_ALLOW_WRITE)) + return 1; + } + + return 0; +} + +/* + checks if we have select tables in the table list and write tables + with auto-increment column. + + SYNOPSIS + has_two_write_locked_tables_with_auto_increment_and_select + tables Table list + + RETURN VALUES + + -true if the table list has atleast one table with auto-increment column + + + and atleast one table to select from. + -false otherwise +*/ + +static bool +has_write_table_with_auto_increment_and_select(TABLE_LIST *tables) +{ + bool has_select= false; + bool has_auto_increment_tables = has_write_table_with_auto_increment(tables); + for(TABLE_LIST *table= tables; table; table= table->next_global) + { + if (!table->placeholder() && + (table->lock_type <= TL_READ_NO_INSERT)) + { + has_select= true; + break; + } + } + return(has_select && has_auto_increment_tables); +} + +/* + Tells if there is a table whose auto_increment column is a part + of a compound primary key while is not the first column in + the table definition. + + @param tables Table list + + @return true if the table exists, fais if does not. +*/ + +static bool +has_write_table_auto_increment_not_first_in_pk(TABLE_LIST *tables) +{ + for (TABLE_LIST *table= tables; table; table= table->next_global) + { + /* we must do preliminary checks as table->table may be NULL */ + if (!table->placeholder() && + table->table->found_next_number_field && + (table->lock_type >= TL_WRITE_ALLOW_WRITE) + && table->table->s->next_number_keypart != 0) + return 1; + } + + return 0; +} /** Decide on logging format to use for the statement and issue errors @@ -4305,6 +4393,31 @@ int THD::decide_logging_format(TABLE_LIST *tables) } #endif + if (variables.binlog_format != BINLOG_FORMAT_ROW && tables) + { + /* + DML statements that modify a table with an auto_increment column based on + rows selected from a table are unsafe as the order in which the rows are + fetched fron the select tables cannot be determined and may differ on + master and slave. + */ + if (has_write_table_with_auto_increment_and_select(tables)) + lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_WRITE_AUTOINC_SELECT); + + if (has_write_table_auto_increment_not_first_in_pk(tables)) + lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_AUTOINC_NOT_FIRST); + + /* + A query that modifies autoinc column in sub-statement can make the + master and slave inconsistent. + We can solve these problems in mixed mode by switching to binlogging + if at least one updated table is used by sub-statement + */ + if (lex->requires_prelocking() && + has_write_table_with_auto_increment(lex->first_not_own_table())) + lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_AUTOINC_COLUMNS); + } + /* Get the capabilities vector for all involved storage engines and mask out the flags for the binary log. @@ -4343,6 +4456,24 @@ int THD::decide_logging_format(TABLE_LIST *tables) prev_write_table= table->table; + /* + INSERT...ON DUPLICATE KEY UPDATE on a table with more than one unique keys + can be unsafe. Check for it if the flag is already not marked for the + given statement. + */ + if (!lex->is_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_INSERT_TWO_KEYS) && + lex->sql_command == SQLCOM_INSERT && lex->duplicates == DUP_UPDATE) + { + uint keys= table->table->s->keys, i= 0, unique_keys= 0; + for (KEY* keyinfo= table->table->s->key_info; + i < keys && unique_keys <= 1; i++, keyinfo++) + { + if (keyinfo->flags & HA_NOSAME) + unique_keys++; + } + if (unique_keys > 1 ) + lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_INSERT_TWO_KEYS); + } } flags_access_some_set |= flags; -- cgit v1.2.1 From a7fb5aecfd527c6b9274db02dcec69daf06c97a3 Mon Sep 17 00:00:00 2001 From: Chaithra Gopalareddy Date: Fri, 20 Nov 2015 12:30:15 +0530 Subject: Bug#19941403: FATAL_SIGNAL(SIG 6) IN BUILD_EQUAL_ITEMS_FOR_COND | IN SQL/SQL_OPTIMIZER.CC:1657 Problem: At the end of first execution select_lex->prep_where is pointing to a runtime created object (temporary table field). As a result server exits trying to access a invalid pointer during second execution. Analysis: While optimizing the join conditions for the query, after the permanent transformation, optimizer makes a copy of the new where conditions in select_lex->prep_where. "prep_where" is what is used as the "where condition" for the query at the start of execution. W.r.t the query in question, "where" condition is actually pointing to a field in the temporary table. As a result, for the second execution the pointer is no more valid resulting in server exit. Fix: At the end of the first execution, select_lex->where will have the original item of the where condition. Make prep_where the new place where the original item of select->where has to be rolled back. Fixed in 5.7 with the wl#7082 - Move permanent transformations from JOIN::optimize to JOIN::prepare Patch for 5.5 includes the following backports from 5.6: Bugfix for Bug12603141 - This makes the first execute statement in the testcase pass in 5.5 However it was noted later in in Bug16163596 that the above bugfix needed to be modified. Although Bug16163596 is reproducible only with changes done for Bug12582849, we have decided include the fix. Considering that Bug12582849 is related to Bug12603141, the fix is also included here. However this results in Bug16317817, Bug16317685, Bug16739050. So fix for the above three bugs is also part of this patch. --- sql/sql_class.cc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'sql/sql_class.cc') diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 05bb38d8358..629088dd862 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -2064,6 +2064,21 @@ struct Item_change_record: public ilink static void operator delete(void *ptr, void *mem) { /* never called */ } }; +void THD::change_item_tree_place(Item **old_ref, Item **new_ref) +{ + I_List_iterator it(change_list); + Item_change_record *change; + while ((change= it++)) + { + if (change->place == old_ref) + { + DBUG_PRINT("info", ("change_item_tree_place old_ref %p new_ref %p", + old_ref, new_ref)); + change->place= new_ref; + break; + } + } +} /* Register an item tree tree transformation, performed by the query -- cgit v1.2.1 From 08e929388b2cea170f6a44e9c8669f9c0f636808 Mon Sep 17 00:00:00 2001 From: Venkatesh Duggirala Date: Sat, 21 Nov 2015 11:08:44 +0530 Subject: Bug #17047208 REPLICATION DIFFERENCE FOR MULTIPLE TRIGGERS Fixing pb2 valgrind failure Missed a 'if condition' check while moving the logic from one place to another place. --- sql/sql_class.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'sql/sql_class.cc') diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 629088dd862..4711009d7cd 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -4477,7 +4477,9 @@ int THD::decide_logging_format(TABLE_LIST *tables) given statement. */ if (!lex->is_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_INSERT_TWO_KEYS) && - lex->sql_command == SQLCOM_INSERT && lex->duplicates == DUP_UPDATE) + lex->sql_command == SQLCOM_INSERT && + /* Duplicate key update is not supported by INSERT DELAYED */ + command != COM_DELAYED_INSERT && lex->duplicates == DUP_UPDATE) { uint keys= table->table->s->keys, i= 0, unique_keys= 0; for (KEY* keyinfo= table->table->s->key_info; -- cgit v1.2.1