Skip to content

M
mariadb
Commit bb56c30a authored by Venkatesh Duggirala's avatar Venkatesh Duggirala
Browse files
Options

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.
parent 63dc9c3f
Hide whitespace changes
Inline Side-by-side
Showing with 365 additions and 155 deletions
mysql-test/suite/rpl/r/rpl_unsafe_statements.result 0 → 100644
View file @ bb56c30a
include/master-slave.inc
[connection master]
CREATE TABLE t1(id INT AUTO_INCREMENT, i INT, PRIMARY KEY (id)) ENGINE=INNODB;
CREATE TABLE t2(id INT AUTO_INCREMENT, i INT, PRIMARY KEY (id)) ENGINE=INNODB;
CREATE TRIGGER trig1 AFTER INSERT ON t1
FOR EACH ROW
INSERT INTO t2(i) VALUES(new.i);
START TRANSACTION;
INSERT INTO t2(i) VALUES (1);
ROLLBACK;
INSERT INTO t1(i) VALUES(2);
START TRANSACTION;
LOCK TABLES t1 WRITE, t2 WRITE;
INSERT INTO t1(i) VALUES(3);
UNLOCK TABLES;
COMMIT;
include/diff_tables.inc [master:t1, slave:t1]
include/diff_tables.inc [master:t2, slave:t2]
DROP TABLE t1,t2;
CREATE TABLE t1(i INT) ENGINE=INNODB;
CREATE TABLE t2(id INT AUTO_INCREMENT, i INT, PRIMARY KEY (id)) ENGINE=INNODB;
INSERT INTO t1 values (1), (2), (3);
START TRANSACTION;
INSERT INTO t2(i) VALUES (1);
ROLLBACK;
INSERT INTO t2(i) SELECT i FROM t1;
START TRANSACTION;
LOCK TABLES t2 WRITE, t1 READ;
INSERT INTO t2(i) SELECT i FROM t1;
UNLOCK TABLES;
COMMIT;
include/diff_tables.inc [master:t1, slave:t1]
include/diff_tables.inc [master:t2, slave:t2]
DROP TABLE t1,t2;
CREATE TABLE t1(i int, id INT AUTO_INCREMENT, PRIMARY KEY (i, id)) ENGINE=MYISAM;
INSERT INTO t1 (i) values (1);
START TRANSACTION;
LOCK TABLES t1 WRITE;
INSERT INTO t1 (i) values (2);
UNLOCK TABLES;
COMMIT;
include/diff_tables.inc [master:t1, slave:t1]
DROP TABLE t1;
CREATE TABLE t1(i INT, j INT, UNIQUE KEY(i), UNIQUE KEY(j)) ENGINE=INNODB;
INSERT INTO t1 (i,j) VALUES (1,2) ON DUPLICATE KEY UPDATE j=j+1;
START TRANSACTION;
LOCK TABLES t1 WRITE;
INSERT INTO t1 (i,j) VALUES (1,2) ON DUPLICATE KEY UPDATE j=j+1;
UNLOCK TABLES;
COMMIT;
include/diff_tables.inc [master:t1, slave:t1]
DROP TABLE t1;
include/rpl_end.inc
mysql-test/suite/rpl/t/rpl_unsafe_statements.test 0 → 100644
View file @ bb56c30a
################################################################################
# Bug#17047208 REPLICATION DIFFERENCE FOR MULTIPLE TRIGGERS
# Problem: 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 DML statement is not marked as 'unsafe' statement.
# Steps to reproduce the reported test case (BINLOG_STMT_UNSAFE_AUTOINC_COLUMNS)
# Case-1:
# > Create a trigger on a table and do a insert in the trigger that updates
# auto increment column
# > A DML that executes the trigger in step.1 and check that DML is marked
# as unsafe and DML is written into binlog using row format (in MBR)
# > Execute the step 2 by locking the required tables prior to DML and check
# that DML is marked as unsafe and DML is written into binlog using row
# format (in MBR)
#
# This test script also adds test cases to cover few other unsafe statements.
# Case-2: BINLOG_STMT_UNSAFE_WRITE_AUTOINC_SELECT
# Case-3: BINLOG_STMT_UNSAFE_AUTOINC_NOT_FIRST
# Case-4: BINLOG_STMT_UNSAFE_INSERT_TWO_KEYS
################################################################################
--source include/have_binlog_format_mixed.inc
--source include/master-slave.inc
# Case-1: BINLOG_STMT_UNSAFE_AUTOINC_COLUMNS
# Statement is unsafe because it invokes a trigger or a
# stored function that inserts into an AUTO_INCREMENT column.
# Step-1.1: Create two tables, one with AUTO_INCREMENT column.
CREATE TABLE t1(id INT AUTO_INCREMENT, i INT, PRIMARY KEY (id)) ENGINE=INNODB;
CREATE TABLE t2(id INT AUTO_INCREMENT, i INT, PRIMARY KEY (id)) ENGINE=INNODB;
# Step-1.2: Create a trigger that inserts into an AUTO_INCREMENT column.
CREATE TRIGGER trig1 AFTER INSERT ON t1
FOR EACH ROW
INSERT INTO t2(i) VALUES(new.i);
# Step-1.3: Create some gap in auto increment value on master's t2 table
# but not on slave (by doing rollback). Just in case if the unsafe statements
# are written in statement format, diff tables will fail.
START TRANSACTION;
INSERT INTO t2(i) VALUES (1);
ROLLBACK;
# Step-1.4: Insert a tuple into table t1 that triggers trig1 which inserts
# into an AUTO_INCREMENT column.
INSERT INTO t1(i) VALUES(2);
# Step-1.5: Repeat step 1.4 but using 'LOCK TABLES' logic.
START TRANSACTION;
LOCK TABLES t1 WRITE, t2 WRITE;
INSERT INTO t1(i) VALUES(3);
UNLOCK TABLES;
COMMIT;
# Step-1.6: Sync slave with master
--sync_slave_with_master
# Step-1.7: Diff master-slave tables to make sure everything is in sync.
--let $diff_tables=master:t1, slave:t1
--source include/diff_tables.inc
--let $diff_tables=master:t2, slave:t2
--source include/diff_tables.inc
# Step-1.8: Cleanup
--connection master
DROP TABLE t1,t2;
# Case-2: BINLOG_STMT_UNSAFE_WRITE_AUTOINC_SELECT
# Statements writing to a table with an auto-increment column after selecting
# from another table are unsafe because the order in which rows are retrieved
# determines what (if any) rows will be written. This order cannot be
# predicted and may differ on master and the slave.
# Step-2.1: Create two tables, one with AUTO_INCREMENT column.
CREATE TABLE t1(i INT) ENGINE=INNODB;
CREATE TABLE t2(id INT AUTO_INCREMENT, i INT, PRIMARY KEY (id)) ENGINE=INNODB;
# Step-2.2: Create some tuples in table t1.
INSERT INTO t1 values (1), (2), (3);
# Step-2.3: Create some gap in auto increment value on master's t2 table
# but not on slave (by doing rollback). Just in case if the unsafe statements
# are written in statement format, diff tables will fail.
START TRANSACTION;
INSERT INTO t2(i) VALUES (1);
ROLLBACK;
# Step-2.4: Insert into t2 (table with an auto-increment) by selecting tuples
# from table t1.
INSERT INTO t2(i) SELECT i FROM t1;
# Step-2.5: Repeat step 2.4 but now with 'LOCK TABLES' logic.
START TRANSACTION;
LOCK TABLES t2 WRITE, t1 READ;
INSERT INTO t2(i) SELECT i FROM t1;
UNLOCK TABLES;
COMMIT;
# Step-2.6: Sync slave with master
--sync_slave_with_master
# Step-2.7: Diff master-slave tables to make sure everything is in sync.
--let $diff_tables=master:t1, slave:t1
--source include/diff_tables.inc
--let $diff_tables=master:t2, slave:t2
--source include/diff_tables.inc
# Step-2.8: Cleanup
--connection master
DROP TABLE t1,t2;
# Case-3: BINLOG_STMT_UNSAFE_AUTOINC_NOT_FIRST
# INSERT into autoincrement field which is not the first part in the
# composed primary key is unsafe
#
# Step-3.1: Create a table with auto increment column and a composed primary key
# (second column is auto increment column). Such a definition is allowed only
# with 'myisam' engine.
CREATE TABLE t1(i int, id INT AUTO_INCREMENT, PRIMARY KEY (i, id)) ENGINE=MYISAM;
# Step-3.2: Inserting into such a table is unsafe.
INSERT INTO t1 (i) values (1);
# Step-3.3: Repeat step 3.2, now with 'LOCK TABLES' logic.
START TRANSACTION;
LOCK TABLES t1 WRITE;
INSERT INTO t1 (i) values (2);
UNLOCK TABLES;
COMMIT;
# Step-3.4: Sync slave with master
--sync_slave_with_master
# Step-3.5: Diff master-slave tables to make sure everything is in sync.
--let $diff_tables=master:t1, slave:t1
--source include/diff_tables.inc
# Step-3.6: Cleanup
--connection master
DROP TABLE t1;
# Case-4: BINLOG_STMT_UNSAFE_INSERT_TWO_KEYS
# INSERT... ON DUPLICATE KEY UPDATE on a table with more than one UNIQUE KEY
# is unsafe Statement
# Step-4.1: Create a table with two unique keys
CREATE TABLE t1(i INT, j INT, UNIQUE KEY(i), UNIQUE KEY(j)) ENGINE=INNODB;
# Step-4.2: Inserting into such a table is unsafe.
INSERT INTO t1 (i,j) VALUES (1,2) ON DUPLICATE KEY UPDATE j=j+1;
# Step-4.3: Repeat step 3.2, now with 'LOCK TABLES' logic.
START TRANSACTION;
LOCK TABLES t1 WRITE;
INSERT INTO t1 (i,j) VALUES (1,2) ON DUPLICATE KEY UPDATE j=j+1;
UNLOCK TABLES;
COMMIT;
# Step-4.4: Sync slave with master
--sync_slave_with_master
# Step-4.5: Diff master-slave tables to make sure everything is in sync.
--let $diff_tables=master:t1, slave:t1
--source include/diff_tables.inc
# Step-4.6: Cleanup
--connection master
DROP TABLE t1;
--source include/rpl_end.inc
sql/sql_base.cc
View file @ bb56c30a
...@@ -212,11 +212,6 @@ static bool check_and_update_table_version(THD *thd, TABLE_LIST *tables, ...@@ -212,11 +212,6 @@ static bool check_and_update_table_version(THD *thd, TABLE_LIST *tables,
static bool open_table_entry_fini(THD *thd, TABLE_SHARE *share, TABLE *entry); static bool open_table_entry_fini(THD *thd, TABLE_SHARE *share, TABLE *entry);
static bool auto_repair_table(THD *thd, TABLE_LIST *table_list); static bool auto_repair_table(THD *thd, TABLE_LIST *table_list);
static void free_cache_entry(TABLE *entry); static void free_cache_entry(TABLE *entry);
static bool
has_write_table_with_auto_increment(TABLE_LIST *tables);
static bool
has_write_table_with_auto_increment_and_select(TABLE_LIST *tables);
static bool has_write_table_auto_increment_not_first_in_pk(TABLE_LIST *tables);
uint cached_open_tables(void) uint cached_open_tables(void)
{ {
...@@ -5744,63 +5739,6 @@ bool lock_tables(THD *thd, TABLE_LIST *tables, uint count, ...@@ -5744,63 +5739,6 @@ bool lock_tables(THD *thd, TABLE_LIST *tables, uint count,
*(ptr++)= table->table; *(ptr++)= table->table;
} }
/*
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 (thd->variables.binlog_format != BINLOG_FORMAT_ROW && tables &&
has_write_table_with_auto_increment_and_select(tables))
thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_WRITE_AUTOINC_SELECT);
/* Todo: merge all has_write_table_auto_inc with decide_logging_format */
if (thd->variables.binlog_format != BINLOG_FORMAT_ROW && tables)
{
if (has_write_table_auto_increment_not_first_in_pk(tables))
thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_AUTOINC_NOT_FIRST);
}
/*
INSERT...ON DUPLICATE KEY UPDATE on a table with more than one unique keys
can be unsafe.
*/
uint unique_keys= 0;
for (TABLE_LIST *query_table= tables; query_table && unique_keys <= 1;
query_table= query_table->next_global)
if(query_table->table)
{
uint keys= query_table->table->s->keys, i= 0;
unique_keys= 0;
for (KEY* keyinfo= query_table->table->s->key_info;
i < keys && unique_keys <= 1; i++, keyinfo++)
{
if (keyinfo->flags & HA_NOSAME)
unique_keys++;
}
if (!query_table->placeholder() &&
query_table->lock_type >= TL_WRITE_ALLOW_WRITE &&
unique_keys > 1 && thd->lex->sql_command == SQLCOM_INSERT &&
/* Duplicate key update is not supported by INSERT DELAYED */
thd->command != COM_DELAYED_INSERT &&
thd->lex->duplicates == DUP_UPDATE)
thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_INSERT_TWO_KEYS);
}
/* We have to emulate LOCK TABLES if we are statement needs prelocking. */
if (thd->lex->requires_prelocking())
{
/*
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 (thd->variables.binlog_format != BINLOG_FORMAT_ROW && tables &&
has_write_table_with_auto_increment(thd->lex->first_not_own_table()))
thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_AUTOINC_COLUMNS);
}
DEBUG_SYNC(thd, "before_lock_tables_takes_lock"); DEBUG_SYNC(thd, "before_lock_tables_takes_lock");
if (! (thd->lock= mysql_lock_tables(thd, start, (uint) (ptr - start), if (! (thd->lock= mysql_lock_tables(thd, start, (uint) (ptr - start),
...@@ -9170,98 +9108,6 @@ bool is_equal(const LEX_STRING *a, const LEX_STRING *b) ...@@ -9170,98 +9108,6 @@ bool is_equal(const LEX_STRING *a, const LEX_STRING *b)
return a->length == b->length && !strncmp(a->str, b->str, a->length); return a->length == b->length && !strncmp(a->str, b->str, a->length);
} }
/*
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;
}
/* /*
Open and lock system tables for read. Open and lock system tables for read.
......
sql/sql_class.cc
View file @ bb56c30a
...@@ -4142,6 +4142,94 @@ void xid_cache_delete(XID_STATE *xid_state) ...@@ -4142,6 +4142,94 @@ void xid_cache_delete(XID_STATE *xid_state)
mysql_mutex_unlock(&LOCK_xid_cache); 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 Decide on logging format to use for the statement and issue errors
...@@ -4305,6 +4393,31 @@ int THD::decide_logging_format(TABLE_LIST *tables) ...@@ -4305,6 +4393,31 @@ int THD::decide_logging_format(TABLE_LIST *tables)
} }
#endif #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 Get the capabilities vector for all involved storage engines and
mask out the flags for the binary log. mask out the flags for the binary log.
...@@ -4343,6 +4456,24 @@ int THD::decide_logging_format(TABLE_LIST *tables) ...@@ -4343,6 +4456,24 @@ int THD::decide_logging_format(TABLE_LIST *tables)
prev_write_table= table->table; 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; flags_access_some_set |= flags;
......
sql/sql_lex.h
View file @ bb56c30a
/* Copyright (c) 2000, 2014, Oracle and/or its affiliates. All rights reserved. /* Copyright (c) 2000, 2015, Oracle and/or its affiliates. All rights reserved.
This program is free software; you can redistribute it and/or modify This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by it under the terms of the GNU General Public License as published by
...@@ -1362,6 +1362,11 @@ public: ...@@ -1362,6 +1362,11 @@ public:
return get_stmt_unsafe_flags() != 0; return get_stmt_unsafe_flags() != 0;
} }
inline bool is_stmt_unsafe(enum_binlog_stmt_unsafe unsafe)
{
return binlog_stmt_flags & (1 << unsafe);
}
/** /**
Flag the current (top-level) statement as unsafe. Flag the current (top-level) statement as unsafe.
The flag will be reset after the statement has finished. The flag will be reset after the statement has finished.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7