Mats, hej.
The patch is okay.
I only would like you to rephrase comments and update the cset's
synopsis
Bug #19958 RBR idempotency issue for UPDATE and DELETE
> Below is the list of changes that have just been committed into a local
> 5.1 repository of mats. When mats does a push these changes will
> be propagated to the main repository and, within 24 hours after the
> push, to the public repository.
> For information on how to access the public repository
> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>
> ChangeSet@stripped, 2007-10-26 09:32:25+02:00, mats@stripped +3 -0
> BUG#19958 (rpl_trigger.test causes Error in Update_rows event when used with RBR):
>
> The rpl_trigger test case indicated a problem with idempotency support when run
> under row-based replication, which this patch fixes.
>
> However, despite this, the test is not designed for execution under row-based
> replication and hence rpl_trigger.test is not executed under row-based
> replication.
>
> The problem is that the test expects triggers to be executed when the slave
> updates rows on the slave, and this is (deliberately) not done with row-based
> replication.
>
> mysql-test/suite/rpl/r/rpl_idempotency.result@stripped, 2007-10-26 09:32:21+02:00,
> mats@stripped +71 -0
> New BitKeeper file ``mysql-test/suite/rpl/r/rpl_idempotency.result''
>
> mysql-test/suite/rpl/r/rpl_idempotency.result@stripped, 2007-10-26 09:32:21+02:00,
> mats@stripped +0 -0
>
> mysql-test/suite/rpl/t/rpl_idempotency.test@stripped, 2007-10-26 09:32:21+02:00,
> mats@stripped +76 -0
> New BitKeeper file ``mysql-test/suite/rpl/t/rpl_idempotency.test''
>
> mysql-test/suite/rpl/t/rpl_idempotency.test@stripped, 2007-10-26 09:32:21+02:00,
> mats@stripped +0 -0
>
> sql/log_event.cc@stripped, 2007-10-26 09:32:21+02:00, mats@stripped
> +67 -4
> Adding function to print symbolic name of handler errors for debug purposes.
> Ignoring some more error messages to provide full idempotency support for
> update and delete operations.
>
> diff -Nrup a/mysql-test/suite/rpl/r/rpl_idempotency.result
> b/mysql-test/suite/rpl/r/rpl_idempotency.result
> --- /dev/null Wed Dec 31 16:00:00 196900
> +++ b/mysql-test/suite/rpl/r/rpl_idempotency.result 2007-10-26 09:32:21 +02:00
> @@ -0,0 +1,71 @@
> +stop slave;
> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
> +reset master;
> +reset slave;
> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
> +start slave;
> +CREATE TABLE t1 (a INT PRIMARY KEY);
> +CREATE TABLE t2 (a INT);
> +INSERT INTO t1 VALUES (-1),(-2),(-3);
> +INSERT INTO t2 VALUES (-1),(-2),(-3);
> +DELETE FROM t1 WHERE a = -2;
> +DELETE FROM t2 WHERE a = -2;
> +DELETE FROM t1 WHERE a = -2;
> +DELETE FROM t2 WHERE a = -2;
> +SELECT * FROM t1 ORDER BY a;
> +a
> +-3
> +-1
> +SELECT * FROM t2 ORDER BY a;
> +a
> +-3
> +-1
> +SELECT * FROM t1 ORDER BY a;
> +a
> +-3
> +-1
> +SELECT * FROM t2 ORDER BY a;
> +a
> +-3
> +-1
> +Last_SQL_Error
> +0
> +INSERT IGNORE INTO t1 VALUES (-2);
> +INSERT IGNORE INTO t1 VALUES (-2);
> +SELECT * FROM t1 ORDER BY a;
> +a
> +-3
> +-2
> +-1
> +SELECT * FROM t1 ORDER BY a;
> +a
> +-3
> +-2
> +-1
> +Last_SQL_Error
> +0
> +UPDATE t1 SET a = 1 WHERE a = -1;
> +UPDATE t2 SET a = 1 WHERE a = -1;
> +UPDATE t1 SET a = 1 WHERE a = -1;
> +UPDATE t2 SET a = 1 WHERE a = -1;
> +SELECT * FROM t1 ORDER BY a;
> +a
> +-3
> +-2
> +1
> +SELECT * FROM t2 ORDER BY a;
> +a
> +-3
> +1
> +SELECT * FROM t1 ORDER BY a;
> +a
> +-3
> +-2
> +1
> +SELECT * FROM t2 ORDER BY a;
> +a
> +-3
> +1
> +Last_SQL_Error
> +0
> +DROP TABLE t1, t2;
> diff -Nrup a/mysql-test/suite/rpl/t/rpl_idempotency.test
> b/mysql-test/suite/rpl/t/rpl_idempotency.test
> --- /dev/null Wed Dec 31 16:00:00 196900
> +++ b/mysql-test/suite/rpl/t/rpl_idempotency.test 2007-10-26 09:32:21 +02:00
> @@ -0,0 +1,76 @@
> +# Testing various forms of idempotency for replication that should
> +# work the same way under statement based as under row based.
> +
> +source include/master-slave.inc;
> +
> +connection master;
> +CREATE TABLE t1 (a INT PRIMARY KEY);
> +CREATE TABLE t2 (a INT);
> +INSERT INTO t1 VALUES (-1),(-2),(-3);
> +INSERT INTO t2 VALUES (-1),(-2),(-3);
> +sync_slave_with_master;
> +
> +# A delete for a row that does not exist, the statement is
> +# deliberately written to be idempotent for statement-based
> +# replication as well. We test this towards both a table with a
> +# primary key and without a primary key.
> +
> +connection slave;
> +DELETE FROM t1 WHERE a = -2;
> +DELETE FROM t2 WHERE a = -2;
> +connection master;
> +DELETE FROM t1 WHERE a = -2;
> +DELETE FROM t2 WHERE a = -2;
> +SELECT * FROM t1 ORDER BY a;
> +SELECT * FROM t2 ORDER BY a;
> +sync_slave_with_master;
> +SELECT * FROM t1 ORDER BY a;
> +SELECT * FROM t2 ORDER BY a;
> +let $last_error = query_get_value("SHOW SLAVE STATUS", Last_SQL_Errno, 1);
> +disable_query_log;
> +eval SELECT "$last_error" AS Last_SQL_Error;
> +enable_query_log;
> +
> +# An insert of a row that already exists. Since we are replacing the
> +# row if it already exists, the most apropriate representation is
> +# INSERT IGNORE. We only test this towards a table with a primary key,
> +# since the other case does not make sense.
> +
> +INSERT IGNORE INTO t1 VALUES (-2);
> +connection master;
> +INSERT IGNORE INTO t1 VALUES (-2);
> +SELECT * FROM t1 ORDER BY a;
> +sync_slave_with_master;
> +SELECT * FROM t1 ORDER BY a;
> +let $last_error = query_get_value("SHOW SLAVE STATUS", Last_SQL_Errno, 1);
> +disable_query_log;
> +eval SELECT "$last_error" AS Last_SQL_Error;
> +enable_query_log;
> +
> +# BUG#19958: rpl_trigger.test causes Error in Update_rows event when
> +# used with RBR.
> +
> +# This test case caused different behaviour in the following
> +# situation, but statement-based and row-based should do the same
> +# thing in this case. We test this both for tables with and without
> +# primary keys.
Could you please change this. It's hard to read. "caused different
behaviour in the following situation" - where is the situation?
> +
> +connection slave;
> +UPDATE t1 SET a = 1 WHERE a = -1;
> +UPDATE t2 SET a = 1 WHERE a = -1;
> +connection master;
> +UPDATE t1 SET a = 1 WHERE a = -1;
> +UPDATE t2 SET a = 1 WHERE a = -1;
> +SELECT * FROM t1 ORDER BY a;
> +SELECT * FROM t2 ORDER BY a;
> +sync_slave_with_master;
> +SELECT * FROM t1 ORDER BY a;
> +SELECT * FROM t2 ORDER BY a;
> +let $last_error = query_get_value("SHOW SLAVE STATUS", Last_SQL_Errno, 1);
> +disable_query_log;
> +eval SELECT "$last_error" AS Last_SQL_Error;
> +enable_query_log;
> +
> +connection master;
> +DROP TABLE t1, t2;
> +sync_slave_with_master;
> diff -Nrup a/sql/log_event.cc b/sql/log_event.cc
> --- a/sql/log_event.cc 2007-10-24 16:54:11 +02:00
> +++ b/sql/log_event.cc 2007-10-26 09:32:21 +02:00
> @@ -36,6 +36,63 @@
>
> #define FLAGSTR(V,F) ((V)&(F)?#F" ":"")
>
> +#ifndef MYSQL_CLIENT
> +static const char *HA_ERR(int i)
> +{
> + switch (i) {
> + case HA_ERR_KEY_NOT_FOUND: return "HA_ERR_KEY_NOT_FOUND";
> + case HA_ERR_FOUND_DUPP_KEY: return "HA_ERR_FOUND_DUPP_KEY";
> + case HA_ERR_RECORD_CHANGED: return "HA_ERR_RECORD_CHANGED";
> + case HA_ERR_WRONG_INDEX: return "HA_ERR_WRONG_INDEX";
> + case HA_ERR_CRASHED: return "HA_ERR_CRASHED";
> + case HA_ERR_WRONG_IN_RECORD: return "HA_ERR_WRONG_IN_RECORD";
> + case HA_ERR_OUT_OF_MEM: return "HA_ERR_OUT_OF_MEM";
> + case HA_ERR_NOT_A_TABLE: return "HA_ERR_NOT_A_TABLE";
> + case HA_ERR_WRONG_COMMAND: return "HA_ERR_WRONG_COMMAND";
> + case HA_ERR_OLD_FILE: return "HA_ERR_OLD_FILE";
> + case HA_ERR_NO_ACTIVE_RECORD: return "HA_ERR_NO_ACTIVE_RECORD";
> + case HA_ERR_RECORD_DELETED: return "HA_ERR_RECORD_DELETED";
> + case HA_ERR_RECORD_FILE_FULL: return "HA_ERR_RECORD_FILE_FULL";
> + case HA_ERR_INDEX_FILE_FULL: return "HA_ERR_INDEX_FILE_FULL";
> + case HA_ERR_END_OF_FILE: return "HA_ERR_END_OF_FILE";
> + case HA_ERR_UNSUPPORTED: return "HA_ERR_UNSUPPORTED";
> + case HA_ERR_TO_BIG_ROW: return "HA_ERR_TO_BIG_ROW";
> + case HA_WRONG_CREATE_OPTION: return "HA_WRONG_CREATE_OPTION";
> + case HA_ERR_FOUND_DUPP_UNIQUE: return "HA_ERR_FOUND_DUPP_UNIQUE";
> + case HA_ERR_UNKNOWN_CHARSET: return "HA_ERR_UNKNOWN_CHARSET";
> + case HA_ERR_WRONG_MRG_TABLE_DEF: return "HA_ERR_WRONG_MRG_TABLE_DEF";
> + case HA_ERR_CRASHED_ON_REPAIR: return "HA_ERR_CRASHED_ON_REPAIR";
> + case HA_ERR_CRASHED_ON_USAGE: return "HA_ERR_CRASHED_ON_USAGE";
> + case HA_ERR_LOCK_WAIT_TIMEOUT: return "HA_ERR_LOCK_WAIT_TIMEOUT";
> + case HA_ERR_LOCK_TABLE_FULL: return "HA_ERR_LOCK_TABLE_FULL";
> + case HA_ERR_READ_ONLY_TRANSACTION: return "HA_ERR_READ_ONLY_TRANSACTION";
> + case HA_ERR_LOCK_DEADLOCK: return "HA_ERR_LOCK_DEADLOCK";
> + case HA_ERR_CANNOT_ADD_FOREIGN: return "HA_ERR_CANNOT_ADD_FOREIGN";
> + case HA_ERR_NO_REFERENCED_ROW: return "HA_ERR_NO_REFERENCED_ROW";
> + case HA_ERR_ROW_IS_REFERENCED: return "HA_ERR_ROW_IS_REFERENCED";
> + case HA_ERR_NO_SAVEPOINT: return "HA_ERR_NO_SAVEPOINT";
> + case HA_ERR_NON_UNIQUE_BLOCK_SIZE: return "HA_ERR_NON_UNIQUE_BLOCK_SIZE";
> + case HA_ERR_NO_SUCH_TABLE: return "HA_ERR_NO_SUCH_TABLE";
> + case HA_ERR_TABLE_EXIST: return "HA_ERR_TABLE_EXIST";
> + case HA_ERR_NO_CONNECTION: return "HA_ERR_NO_CONNECTION";
> + case HA_ERR_NULL_IN_SPATIAL: return "HA_ERR_NULL_IN_SPATIAL";
> + case HA_ERR_TABLE_DEF_CHANGED: return "HA_ERR_TABLE_DEF_CHANGED";
> + case HA_ERR_NO_PARTITION_FOUND: return "HA_ERR_NO_PARTITION_FOUND";
> + case HA_ERR_RBR_LOGGING_FAILED: return "HA_ERR_RBR_LOGGING_FAILED";
> + case HA_ERR_DROP_INDEX_FK: return "HA_ERR_DROP_INDEX_FK";
> + case HA_ERR_FOREIGN_DUPLICATE_KEY: return "HA_ERR_FOREIGN_DUPLICATE_KEY";
> + case HA_ERR_TABLE_NEEDS_UPGRADE: return "HA_ERR_TABLE_NEEDS_UPGRADE";
> + case HA_ERR_TABLE_READONLY: return "HA_ERR_TABLE_READONLY";
> + case HA_ERR_AUTOINC_READ_FAILED: return "HA_ERR_AUTOINC_READ_FAILED";
> + case HA_ERR_AUTOINC_ERANGE: return "HA_ERR_AUTOINC_ERANGE";
> + case HA_ERR_GENERIC: return "HA_ERR_GENERIC";
> + case HA_ERR_RECORD_IS_THE_SAME: return "HA_ERR_RECORD_IS_THE_SAME";
> + case HA_ERR_LOGGING_IMPOSSIBLE: return "HA_ERR_LOGGING_IMPOSSIBLE";
> + case HA_ERR_CORRUPT_EVENT: return "HA_ERR_CORRUPT_EVENT";
> + }
> +}
> +#endif
> +
> /*
> Cache that will automatically be written to a dedicated file on
> destruction.
> @@ -6228,8 +6285,11 @@ int Rows_log_event::do_apply_event(Relay
>
> /* Some recoverable errors */
> case HA_ERR_RECORD_CHANGED:
> - case HA_ERR_KEY_NOT_FOUND: /* Idempotency support: OK if
> - tuple does not exist */
> + case HA_ERR_RECORD_DELETED:
> + case HA_ERR_KEY_NOT_FOUND:
> + case HA_ERR_END_OF_FILE:
> + /* Idempotency support: OK if tuple does not exist */
> + DBUG_PRINT("info", ("error: %s", HA_ERR(error)));
> error= 0;
> break;
>
> @@ -7467,6 +7527,9 @@ static bool record_compare(TABLE *table)
> records. Check that the other engines also return correct records.
> */
>
> + DBUG_DUMP("record[0]", table->record[0], table->s->reclength);
> + DBUG_DUMP("record[1]", table->record[1], table->s->reclength);
> +
> bool result= FALSE;
> uchar saved_x[2], saved_filler[2];
>
> @@ -7555,7 +7618,7 @@ record_compare_exit:
>
> int Rows_log_event::find_row(const Relay_log_info *rli)
> {
> - DBUG_ENTER("find_row");
> + DBUG_ENTER("Rows_log_event::find_row");
>
> DBUG_ASSERT(m_table && m_table->in_use != NULL);
>
> @@ -7784,7 +7847,7 @@ int Rows_log_event::find_row(const Relay
> DBUG_DUMP("record found", table->record[0], table->s->reclength);
> table->file->ha_rnd_end();
>
> - DBUG_ASSERT(error == HA_ERR_END_OF_FILE || error == 0);
> + DBUG_ASSERT(error == HA_ERR_END_OF_FILE || error == HA_ERR_RECORD_DELETED ||
> error == 0);
> DBUG_RETURN(error);
> }
>
>
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe: http://lists.mysql.com/commits?unsub=1
>
regards,
Andrei
