Hi Andrei,
Good work. Patch approved.
Regards,
Luís
On Wed, 2009-04-08 at 16:09 +0300, Andrei Elkin wrote:
> #At
> file:///home/andrei/MySQL/BZR/FIXES/6.0-rpl-bug38173-rbr_no_default_val_slave_err/ based
> on revid:aelkin@stripped
>
> 2836 Andrei Elkin 2009-04-08
> Bug #38173 Field doesn't have a default value with row-based replication
>
> The reason of the bug was incompatibile with the master side behaviour.
> INSERT query on the master is allowed to insert into a table without
> specifying
> values of DEFAULT-less fields if sql_mode is not strict.
>
> Fixed with checking sql_mode by the sql thread to decide how to react.
> Non-strict sql_mode should allow Write_rows event to complete.
>
> todo: warnings can be shown via show slave status, still this is a
> separate rather general issue how to show warnings for the slave
> threads.
> modified:
> mysql-test/extra/rpl_tests/rpl_extraSlave_Col.test
> mysql-test/extra/rpl_tests/rpl_row_tabledefs.test
> mysql-test/suite/rpl/r/rpl_extraCol_innodb.result
> mysql-test/suite/rpl/r/rpl_extraCol_myisam.result
> mysql-test/suite/rpl/r/rpl_row_tabledefs_2myisam.result
> mysql-test/suite/rpl/r/rpl_row_tabledefs_3innodb.result
> sql/log_event.cc
> sql/rpl_record.cc
> sql/rpl_record.h
>
> per-file messages:
> mysql-test/extra/rpl_tests/rpl_extraSlave_Col.test
> regression test for bug#38173 adds:
> - fields that type does not have DEFAULT, still having the implicit default to
> be set
> in if sql_mode permits;
> - explanation/todo for bug#43992: to stop by slave when sql_mode will be
> possible to
> set strict.
> mysql-test/extra/rpl_tests/rpl_row_tabledefs.test
> regression test for bug#38173 refines the test not to expect the slave stopped.
> mysql-test/suite/rpl/r/rpl_extraCol_innodb.result
> bug#38173 changed results.
> mysql-test/suite/rpl/r/rpl_extraCol_myisam.result
> bug#38173 changed results.
> mysql-test/suite/rpl/r/rpl_row_tabledefs_2myisam.result
> bug#38173 changed results.
> mysql-test/suite/rpl/r/rpl_row_tabledefs_3innodb.result
> bug#38173 changed results.
> sql/log_event.cc
> Rows_log_event::write_row() ships instruction how to react on no-DEFAULT by
> prepare_record().
> sql/rpl_record.cc
> prepare_record() arg lists is augmented with abort_on_warning bool. it's supposed
> to
> be false when sql_mode is not strict.
> The new parameter refines how exactly the error situation should be treated if
> checking the default is requested by the caller.
> sql/rpl_record.h
> prepare_record() gets similar to check_that_all_fields_are_given_values()
> paramenter to control field-does-not-have-default error reaction.
> The new argument is made as default since only Write_rows the inserting method
> needs it.
> === modified file 'mysql-test/extra/rpl_tests/rpl_extraSlave_Col.test'
> --- a/mysql-test/extra/rpl_tests/rpl_extraSlave_Col.test 2008-07-10 16:09:39 +0000
> +++ b/mysql-test/extra/rpl_tests/rpl_extraSlave_Col.test 2009-04-08 13:09:37 +0000
> @@ -405,13 +405,18 @@ sync_slave_with_master;
> ###########################################
> # Bug#22234, Bug#23907 Extra Slave Col is not
> # erroring on extra col with no default values.
> -########################################################
> +###############################################################
> +# Error reaction is up to sql_mode of the slave sql (bug#38173)
> #--echo *** Create t9 on slave ***
> STOP SLAVE;
> RESET SLAVE;
> eval CREATE TABLE t9 (a INT KEY, b BLOB, c CHAR(5),
> d TIMESTAMP,
> - e INT NOT NULL) ENGINE=$engine_type;
> + e INT NOT NULL,
> + f text not null,
> + g text,
> + h blob not null,
> + i blob) ENGINE=$engine_type;
>
> --echo *** Create t9 on Master ***
> connection master;
> @@ -429,13 +434,25 @@ set @b1 = 'b1b1b1b1';
> set @b1 = concat(@b1,@b1);
> INSERT INTO t9 () VALUES(1,@b1,'Kyle'),(2,@b1,'JOE'),(3,@b1,'QA');
>
> -connection slave;
> ---source include/wait_for_slave_sql_to_stop.inc
> ---replace_result $MASTER_MYPORT MASTER_PORT
> ---replace_column 1 # 4 # 7 # 8 # 9 # 16 # 22 # 23 # 33 # 35 # 36 #
> ---query_vertical SHOW SLAVE STATUS
> -SET GLOBAL SQL_SLAVE_SKIP_COUNTER=2;
> -START SLAVE;
> +# the test would stop slave if @@sql_mode for the sql thread
> +# was set to strict. Otherwise, as with this tests setup,
> +# the implicit defaults will be inserted into fields even though
> +# they are declared without DEFAULT clause.
> +
> +sync_slave_with_master;
> +select * from t9;
> +
> +# todo: fix Bug #43992 slave sql thread can't tune own sql_mode ...
> +# and add/restore waiting for stop test
> +
> +#--source include/wait_for_slave_sql_to_stop.inc
> +#--replace_result $MASTER_MYPORT MASTER_PORT
> +#--replace_column 1 # 4 # 7 # 8 # 9 # 16 # 22 # 23 # 33 # 35 # 36 #
> +#--query_vertical SHOW SLAVE STATUS
> +#SET GLOBAL SQL_SLAVE_SKIP_COUNTER=2;
> +#START SLAVE;
> +
> +
>
> #--echo *** Drop t9 ***
> #connection master;
>
> === modified file 'mysql-test/extra/rpl_tests/rpl_row_tabledefs.test'
> --- a/mysql-test/extra/rpl_tests/rpl_row_tabledefs.test 2009-02-02 15:58:48 +0000
> +++ b/mysql-test/extra/rpl_tests/rpl_row_tabledefs.test 2009-04-08 13:09:37 +0000
> @@ -111,21 +111,18 @@ SELECT a,b,x FROM t1_int ORDER BY a;
> SELECT a,b,HEX(x),HEX(y),HEX(z) FROM t1_bit ORDER BY a;
> SELECT a,b,x FROM t1_char ORDER BY a;
>
> -# Each of these inserts should generate an error and stop the slave
> -
> connection master;
> INSERT INTO t9 VALUES (2);
> sync_slave_with_master;
> # Now slave is guaranteed to be running
> connection master;
> INSERT INTO t1_nodef VALUES (1,2);
> -connection slave;
> ---source include/wait_for_slave_sql_to_stop.inc
> ---replace_result $MASTER_MYPORT MASTER_PORT
> ---replace_column 1 # 4 # 7 # 8 # 9 # 20 <Last_Error> 22 # 23 # 33 # 35
> <Last_IO_Errno> 36 <Last_IO_Error> 38 <Last_SQL_Error>
> ---query_vertical SHOW SLAVE STATUS
> -SET GLOBAL SQL_SLAVE_SKIP_COUNTER=2;
> -START SLAVE;
> +
> +# Last insert on wider slave table succeeds while slave sql sql_mode permits.
> +# The previous version of the above test expected slave sql to stop.
> +# bug#38173 relaxed conditions to stop only with the strict mode.
> +sync_slave_with_master;
> +select count(*) from t1_nodef;
>
> #
> # Replicating to tables with fewer columns at the end works as of WL#3228
>
> === modified file 'mysql-test/suite/rpl/r/rpl_extraCol_innodb.result'
> --- a/mysql-test/suite/rpl/r/rpl_extraCol_innodb.result 2008-11-12 15:23:22 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_extraCol_innodb.result 2009-04-08 13:09:37 +0000
> @@ -413,7 +413,11 @@ STOP SLAVE;
> RESET SLAVE;
> CREATE TABLE t9 (a INT KEY, b BLOB, c CHAR(5),
> d TIMESTAMP,
> -e INT NOT NULL) ENGINE='InnoDB';
> +e INT NOT NULL,
> +f text not null,
> +g text,
> +h blob not null,
> +i blob) ENGINE='InnoDB';
> *** Create t9 on Master ***
> CREATE TABLE t9 (a INT PRIMARY KEY, b BLOB, c CHAR(5)
> ) ENGINE='InnoDB';
> @@ -424,49 +428,11 @@ START SLAVE;
> set @b1 = 'b1b1b1b1';
> set @b1 = concat(@b1,@b1);
> INSERT INTO t9 () VALUES(1,@b1,'Kyle'),(2,@b1,'JOE'),(3,@b1,'QA');
> -SHOW SLAVE STATUS;
> -Slave_IO_State #
> -Master_Host 127.0.0.1
> -Master_User root
> -Master_Port #
> -Connect_Retry 1
> -Master_Log_File master-bin.000001
> -Read_Master_Log_Pos #
> -Relay_Log_File #
> -Relay_Log_Pos #
> -Relay_Master_Log_File master-bin.000001
> -Slave_IO_Running Yes
> -Slave_SQL_Running No
> -Replicate_Do_DB
> -Replicate_Ignore_DB
> -Replicate_Do_Table
> -Replicate_Ignore_Table #
> -Replicate_Wild_Do_Table
> -Replicate_Wild_Ignore_Table
> -Last_Errno 1364
> -Last_Error Could not execute Write_rows event on table test.t9; Field 'e' doesn't
> have a default value, Error_code: 1364; handler error HA_ERR_ROWS_EVENT_APPLY; the event's
> master log master-bin.000001, end_log_pos 331
> -Skip_Counter 0
> -Exec_Master_Log_Pos #
> -Relay_Log_Space #
> -Until_Condition None
> -Until_Log_File
> -Until_Log_Pos 0
> -Master_SSL_Allowed No
> -Master_SSL_CA_File
> -Master_SSL_CA_Path
> -Master_SSL_Cert
> -Master_SSL_Cipher
> -Master_SSL_Key
> -Seconds_Behind_Master #
> -Master_SSL_Verify_Server_Cert No
> -Last_IO_Errno #
> -Last_IO_Error #
> -Last_SQL_Errno 1364
> -Last_SQL_Error Could not execute Write_rows event on table test.t9; Field 'e'
> doesn't have a default value, Error_code: 1364; handler error HA_ERR_ROWS_EVENT_APPLY; the
> event's master log master-bin.000001, end_log_pos 331
> -Replicate_Ignore_Server_Ids
> -Master_Server_Id 1
> -SET GLOBAL SQL_SLAVE_SKIP_COUNTER=2;
> -START SLAVE;
> +select * from t9;
> +a b c d e f g h i
> +1 b1b1b1b1b1b1b1b1 Kyle 0000-00-00 00:00:00 0 NULL NULL
> +2 b1b1b1b1b1b1b1b1 JOE 0000-00-00 00:00:00 0 NULL NULL
> +3 b1b1b1b1b1b1b1b1 QA 0000-00-00 00:00:00 0 NULL NULL
> *** Create t10 on slave ***
> STOP SLAVE;
> RESET SLAVE;
>
> === modified file 'mysql-test/suite/rpl/r/rpl_extraCol_myisam.result'
> --- a/mysql-test/suite/rpl/r/rpl_extraCol_myisam.result 2008-11-12 15:23:22 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_extraCol_myisam.result 2009-04-08 13:09:37 +0000
> @@ -413,7 +413,11 @@ STOP SLAVE;
> RESET SLAVE;
> CREATE TABLE t9 (a INT KEY, b BLOB, c CHAR(5),
> d TIMESTAMP,
> -e INT NOT NULL) ENGINE='MyISAM';
> +e INT NOT NULL,
> +f text not null,
> +g text,
> +h blob not null,
> +i blob) ENGINE='MyISAM';
> *** Create t9 on Master ***
> CREATE TABLE t9 (a INT PRIMARY KEY, b BLOB, c CHAR(5)
> ) ENGINE='MyISAM';
> @@ -424,49 +428,11 @@ START SLAVE;
> set @b1 = 'b1b1b1b1';
> set @b1 = concat(@b1,@b1);
> INSERT INTO t9 () VALUES(1,@b1,'Kyle'),(2,@b1,'JOE'),(3,@b1,'QA');
> -SHOW SLAVE STATUS;
> -Slave_IO_State #
> -Master_Host 127.0.0.1
> -Master_User root
> -Master_Port #
> -Connect_Retry 1
> -Master_Log_File master-bin.000001
> -Read_Master_Log_Pos #
> -Relay_Log_File #
> -Relay_Log_Pos #
> -Relay_Master_Log_File master-bin.000001
> -Slave_IO_Running Yes
> -Slave_SQL_Running No
> -Replicate_Do_DB
> -Replicate_Ignore_DB
> -Replicate_Do_Table
> -Replicate_Ignore_Table #
> -Replicate_Wild_Do_Table
> -Replicate_Wild_Ignore_Table
> -Last_Errno 1364
> -Last_Error Could not execute Write_rows event on table test.t9; Field 'e' doesn't
> have a default value, Error_code: 1364; handler error HA_ERR_ROWS_EVENT_APPLY; the event's
> master log master-bin.000001, end_log_pos 331
> -Skip_Counter 0
> -Exec_Master_Log_Pos #
> -Relay_Log_Space #
> -Until_Condition None
> -Until_Log_File
> -Until_Log_Pos 0
> -Master_SSL_Allowed No
> -Master_SSL_CA_File
> -Master_SSL_CA_Path
> -Master_SSL_Cert
> -Master_SSL_Cipher
> -Master_SSL_Key
> -Seconds_Behind_Master #
> -Master_SSL_Verify_Server_Cert No
> -Last_IO_Errno #
> -Last_IO_Error #
> -Last_SQL_Errno 1364
> -Last_SQL_Error Could not execute Write_rows event on table test.t9; Field 'e'
> doesn't have a default value, Error_code: 1364; handler error HA_ERR_ROWS_EVENT_APPLY; the
> event's master log master-bin.000001, end_log_pos 331
> -Replicate_Ignore_Server_Ids
> -Master_Server_Id 1
> -SET GLOBAL SQL_SLAVE_SKIP_COUNTER=2;
> -START SLAVE;
> +select * from t9;
> +a b c d e f g h i
> +1 b1b1b1b1b1b1b1b1 Kyle 0000-00-00 00:00:00 0 NULL NULL
> +2 b1b1b1b1b1b1b1b1 JOE 0000-00-00 00:00:00 0 NULL NULL
> +3 b1b1b1b1b1b1b1b1 QA 0000-00-00 00:00:00 0 NULL NULL
> *** Create t10 on slave ***
> STOP SLAVE;
> RESET SLAVE;
>
> === modified file 'mysql-test/suite/rpl/r/rpl_row_tabledefs_2myisam.result'
> --- a/mysql-test/suite/rpl/r/rpl_row_tabledefs_2myisam.result 2009-02-02 15:58:48
> +0000
> +++ b/mysql-test/suite/rpl/r/rpl_row_tabledefs_2myisam.result 2009-04-08 13:09:37
> +0000
> @@ -105,49 +105,9 @@ a b x
> 2 10 Foo is a bar
> INSERT INTO t9 VALUES (2);
> INSERT INTO t1_nodef VALUES (1,2);
> -SHOW SLAVE STATUS;
> -Slave_IO_State #
> -Master_Host 127.0.0.1
> -Master_User root
> -Master_Port #
> -Connect_Retry 1
> -Master_Log_File master-bin.000001
> -Read_Master_Log_Pos #
> -Relay_Log_File #
> -Relay_Log_Pos #
> -Relay_Master_Log_File master-bin.000001
> -Slave_IO_Running Yes
> -Slave_SQL_Running No
> -Replicate_Do_DB
> -Replicate_Ignore_DB
> -Replicate_Do_Table
> -Replicate_Ignore_Table
> -Replicate_Wild_Do_Table
> -Replicate_Wild_Ignore_Table
> -Last_Errno 1364
> -Last_Error <Last_Error>
> -Skip_Counter 0
> -Exec_Master_Log_Pos #
> -Relay_Log_Space #
> -Until_Condition None
> -Until_Log_File
> -Until_Log_Pos 0
> -Master_SSL_Allowed No
> -Master_SSL_CA_File
> -Master_SSL_CA_Path
> -Master_SSL_Cert
> -Master_SSL_Cipher
> -Master_SSL_Key
> -Seconds_Behind_Master #
> -Master_SSL_Verify_Server_Cert No
> -Last_IO_Errno <Last_IO_Errno>
> -Last_IO_Error <Last_IO_Error>
> -Last_SQL_Errno 1364
> -Last_SQL_Error <Last_SQL_Error>
> -Replicate_Ignore_Server_Ids
> -Master_Server_Id 1
> -SET GLOBAL SQL_SLAVE_SKIP_COUNTER=2;
> -START SLAVE;
> +select count(*) from t1_nodef;
> +count(*)
> +1
> INSERT INTO t9 VALUES (2);
> **** On Master ****
> INSERT INTO t2 VALUES (2,4);
>
> === modified file 'mysql-test/suite/rpl/r/rpl_row_tabledefs_3innodb.result'
> --- a/mysql-test/suite/rpl/r/rpl_row_tabledefs_3innodb.result 2009-02-02 15:58:48
> +0000
> +++ b/mysql-test/suite/rpl/r/rpl_row_tabledefs_3innodb.result 2009-04-08 13:09:37
> +0000
> @@ -105,49 +105,9 @@ a b x
> 2 10 Foo is a bar
> INSERT INTO t9 VALUES (2);
> INSERT INTO t1_nodef VALUES (1,2);
> -SHOW SLAVE STATUS;
> -Slave_IO_State #
> -Master_Host 127.0.0.1
> -Master_User root
> -Master_Port #
> -Connect_Retry 1
> -Master_Log_File master-bin.000001
> -Read_Master_Log_Pos #
> -Relay_Log_File #
> -Relay_Log_Pos #
> -Relay_Master_Log_File master-bin.000001
> -Slave_IO_Running Yes
> -Slave_SQL_Running No
> -Replicate_Do_DB
> -Replicate_Ignore_DB
> -Replicate_Do_Table
> -Replicate_Ignore_Table
> -Replicate_Wild_Do_Table
> -Replicate_Wild_Ignore_Table
> -Last_Errno 1364
> -Last_Error <Last_Error>
> -Skip_Counter 0
> -Exec_Master_Log_Pos #
> -Relay_Log_Space #
> -Until_Condition None
> -Until_Log_File
> -Until_Log_Pos 0
> -Master_SSL_Allowed No
> -Master_SSL_CA_File
> -Master_SSL_CA_Path
> -Master_SSL_Cert
> -Master_SSL_Cipher
> -Master_SSL_Key
> -Seconds_Behind_Master #
> -Master_SSL_Verify_Server_Cert No
> -Last_IO_Errno <Last_IO_Errno>
> -Last_IO_Error <Last_IO_Error>
> -Last_SQL_Errno 1364
> -Last_SQL_Error <Last_SQL_Error>
> -Replicate_Ignore_Server_Ids
> -Master_Server_Id 1
> -SET GLOBAL SQL_SLAVE_SKIP_COUNTER=2;
> -START SLAVE;
> +select count(*) from t1_nodef;
> +count(*)
> +1
> INSERT INTO t9 VALUES (2);
> **** On Master ****
> INSERT INTO t2 VALUES (2,4);
>
> === modified file 'sql/log_event.cc'
> --- a/sql/log_event.cc 2009-03-09 12:17:41 +0000
> +++ b/sql/log_event.cc 2009-04-08 13:09:37 +0000
> @@ -8381,7 +8381,10 @@ Rows_log_event::write_row(const Relay_lo
> values should be checked. Maybe these two flags can be combined.
> */
> if ((error= prepare_record(table, &m_cols, m_width,
> - table->file->ht->db_type !=
> DB_TYPE_NDBCLUSTER)))
> + table->file->ht->db_type !=
> DB_TYPE_NDBCLUSTER,
> + (rli->sql_thd->variables.sql_mode &
> + (MODE_STRICT_TRANS_TABLES |
> + MODE_STRICT_ALL_TABLES)))))
> DBUG_RETURN(error);
>
> /* unpack row into table->record[0] */
>
> === modified file 'sql/rpl_record.cc'
> --- a/sql/rpl_record.cc 2009-03-05 21:13:37 +0000
> +++ b/sql/rpl_record.cc 2009-04-08 13:09:37 +0000
> @@ -373,14 +373,16 @@ unpack_row(Relay_log_info const *rli,
>
> @param table[in,out] Table whose record[0] buffer is prepared.
> @param cols[in] Vector of bits denoting columns that will not be checked.
> - @param check[in] Indicates if errors should be raised when checking default
> - values.
> -
> + @param check[in] Specifies if lack of default error needs checking.
> + @param abort_on_warning[in]
> + Controls how to react on lack of a field's default.
> + The parameter mimics the master side one for
> + @c check_that_all_fields_are_given_values.
> @retval 0 Success
> @retval ER_NO_DEFAULT_FOR_FIELD Default value could not be checked for a field
> */
> -int prepare_record(TABLE *const table,
> - const MY_BITMAP *cols, uint width, const bool check)
> +int prepare_record(TABLE *const table, const MY_BITMAP *cols, uint width,
> + const bool check, const bool abort_on_warning)
> {
> DBUG_ENTER("prepare_record");
>
> @@ -390,28 +392,34 @@ int prepare_record(TABLE *const table,
> if (!check)
> DBUG_RETURN(error);
>
> - /*
> - For fields that are not in the cols for the row, we check them if they
> - have default or can be null.
> + /**
> + For fields that are not in @c cols for the row, we check them if they
> + have a default. The check follows the same rules as the INSERT
> + query without specifying an explicit value for a field not having
> + the explicit default (@c check_that_all_fields_are_given_values()).
> */
>
> DBUG_PRINT_BITSET("debug", "cols: %s", cols);
> - for (Field **field_ptr= table->field ; *field_ptr ; ++field_ptr)
> + for (Field **field_ptr= table->field; *field_ptr; ++field_ptr)
> {
> if ((uint) (field_ptr - table->field) >= cols->n_bits ||
> !bitmap_is_set(cols, field_ptr - table->field))
> {
> - uint32 const mask= NOT_NULL_FLAG | NO_DEFAULT_VALUE_FLAG;
> Field *const f= *field_ptr;
> -
> - if (((f->flags & mask) == mask))
> + if ((f->flags & NO_DEFAULT_VALUE_FLAG) &&
> + (f->real_type() != MYSQL_TYPE_ENUM))
> {
> - my_error(ER_NO_DEFAULT_FOR_FIELD, MYF(0), f->field_name);
> - error = HA_ERR_ROWS_EVENT_APPLY;
> + push_warning_printf(current_thd, abort_on_warning?
> + MYSQL_ERROR::WARN_LEVEL_ERROR :
> + MYSQL_ERROR::WARN_LEVEL_WARN,
> + ER_NO_DEFAULT_FOR_FIELD,
> + ER(ER_NO_DEFAULT_FOR_FIELD),
> + f->field_name);
> + if (abort_on_warning)
> + error = HA_ERR_ROWS_EVENT_APPLY;
> }
> }
> }
> -
> DBUG_RETURN(error);
> }
>
>
> === modified file 'sql/rpl_record.h'
> --- a/sql/rpl_record.h 2008-09-09 08:19:21 +0000
> +++ b/sql/rpl_record.h 2009-04-08 13:09:37 +0000
> @@ -32,7 +32,8 @@ int unpack_row(Relay_log_info const *rli
> bool unpack_blobs);
>
> // Fill table's record[0] with default values.
> -int prepare_record(TABLE *const, const MY_BITMAP *cols, uint width, const bool);
> +int prepare_record(TABLE *const, const MY_BITMAP *cols, uint width,
> + const bool, const bool= FALSE);
> #endif
>
> #endif
>
>
--
Luís
