Luis, hello.
The patch is approved.
I have just couple of "footnotes" :-) for your deliberation.
> #At file:///home/lsoares/Workspace/bzr/work/bugfixing/47312/mysql-5.1-bugteam/ based
> on revid:luis.soares@stripped
>
> 3124 Luis Soares 2009-09-15
> BUG#47312: RBR: Disabling key on slave breaks replication:
> HA_ERR_WRONG_INDEX
>
> In RBR, if disabling keys on slave table it breaks replication if
> updating or deleting a record. When the slave thread tries to
> finding the row, by searching the storage engine, it checkes
> whether the table has a key or not. If it has one, then the slave
> thread uses it to search the record.
>
> Unfortunately, the slave only checks whether the key exists or
~~~~~~~~~~~~~
I would not advice expressing emotions :-)
> not, it does not verify if it is active. Should the key be
> disabled (eg, DBA has issued an ALTER TABLE ... DISABLE KEYS)
> then it will result in error: HA_ERR_WRONG_INDEX.
>
> This patch addresses this issue by making the slave thread also
> verify whether the key is active or not.
>
ok
> added:
> mysql-test/suite/rpl/r/rpl_row_disabled_slave_key.result
> mysql-test/suite/rpl/t/rpl_row_disabled_slave_key.test
> modified:
> sql/log_event.cc
> === added file 'mysql-test/suite/rpl/r/rpl_row_disabled_slave_key.result'
> --- a/mysql-test/suite/rpl/r/rpl_row_disabled_slave_key.result 1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/rpl/r/rpl_row_disabled_slave_key.result 2009-09-15 09:55:53
> +0000
> @@ -0,0 +1,26 @@
> +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;
> +SET SQL_LOG_BIN=0;
> +CREATE TABLE t (a int, b int, c int, key(b));
> +SET SQL_LOG_BIN=1;
> +CREATE TABLE t (a int, b int, c int);
> +INSERT INTO t VALUES (1,2,4);
> +INSERT INTO t VALUES (4,3,4);
> +DELETE FROM t;
> +DROP TABLE t;
> +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 t (a int, b int, c int, key(b));
> +ALTER TABLE t DISABLE KEYS;
> +INSERT INTO t VALUES (1,2,4);
> +INSERT INTO t VALUES (4,3,4);
> +DELETE FROM t;
> +DROP TABLE t;
>
> === added file 'mysql-test/suite/rpl/t/rpl_row_disabled_slave_key.test'
> --- a/mysql-test/suite/rpl/t/rpl_row_disabled_slave_key.test 1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/rpl/t/rpl_row_disabled_slave_key.test 2009-09-15 09:55:53
> +0000
> @@ -0,0 +1,73 @@
> +# BUG#47312: RBR: Disabling key on slave breaks replication:
> +# HA_ERR_WRONG_INDEX
> +#
> +# Description
> +# ===========
> +#
> +# This test case checks whether disabling a key on a slave breaks
> +# replication or not.
> +#
> +# Case #1, shows that while not using ALTER TABLE... DISABLE KEYS and
> +# the slave has no key defined while the master has one, replication
> +# won't break.
> +#
> +# Case #2, shows that before patch for BUG#47312, if defining key on
> +# slave table, and later disable it, replication would break. This
> +# has been fixed.
> +#
> +
> +-- source include/master-slave.inc
> +-- source include/have_binlog_format_row.inc
> +
> +#
> +# Case #1: master has key, but slave has not.
> +# Replication does not break.
> +#
> +
> +SET SQL_LOG_BIN=0;
> +CREATE TABLE t (a int, b int, c int, key(b));
> +SET SQL_LOG_BIN=1;
> +
> +-- connection slave
> +
> +CREATE TABLE t (a int, b int, c int);
> +
> +-- connection master
> +
> +INSERT INTO t VALUES (1,2,4);
> +INSERT INTO t VALUES (4,3,4);
> +DELETE FROM t;
> +
> +-- sync_slave_with_master
> +
> +-- connection master
> +DROP TABLE t;
> +
> +-- sync_slave_with_master
> +
> +#
> +# Case #2: master has key, slave also has one,
> +# but it gets disabled sometime.
> +# Replication does not break anymore.
> +#
> +-- source include/master-slave-reset.inc
> +-- connection master
> +
> +CREATE TABLE t (a int, b int, c int, key(b));
> +
> +-- sync_slave_with_master
> +
> +ALTER TABLE t DISABLE KEYS;
> +
> +-- connection master
> +
> +INSERT INTO t VALUES (1,2,4);
> +INSERT INTO t VALUES (4,3,4);
> +DELETE FROM t;
> +
> +-- sync_slave_with_master
> +
> +-- connection master
> +DROP TABLE t;
> +
> +-- sync_slave_with_master
>
> === modified file 'sql/log_event.cc'
> --- a/sql/log_event.cc 2009-09-10 10:05:53 +0000
> +++ b/sql/log_event.cc 2009-09-15 09:55:53 +0000
> @@ -8835,11 +8835,11 @@ int Rows_log_event::find_row(const Relay
> */
> store_record(table,record[1]);
>
> - if (table->s->keys > 0)
> + if (table->s->keys > 0 && table->s->keys_in_use.is_set(0))
> {
> DBUG_PRINT("info",("locating record using primary key (index_read)"));
>
> - /* We have a key: search the table using the index */
> + /* We have an active key: search the table using the index */
it could be said the 0th key is active.
> if (!table->file->inited && (error=
> table->file->ha_index_init(0, FALSE)))
> {
> DBUG_PRINT("info",("ha_index_init returns error %d",error));
>
>
> # Bazaar merge directive format 2 (Bazaar 0.90)
> # revision_id: luis.soares@stripped
> # target_branch: file:///home/lsoares/Workspace/bzr/work/bugfixing\
> # /47312/mysql-5.1-bugteam/
> # testament_sha1: 6828cda04580e0141d4133b890bc25b43dcf5c40
> # timestamp: 2009-09-15 10:55:59 +0100
> # base_revision_id: luis.soares@stripped\
> # nvzyipdt40e21rx7
> #
> # Begin bundle
> IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWS7z4VwABGbfgEGQWff//3/v
> /+C/7//wYAou+2NLa21gGtWmqtNIgChUqhIgSgkNJpqnmpMw1T9FNPU8jU8p6jyh6TNJvVAADJoc
> ZMmjENNDATQxNGmTEDIwmjTTCDJjjJk0YhpoYCaGJo0yYgZGE0aaYQZMJEmkJhNE2kyk/U9UegZT
> EGBANMmgaDQ0OMmTRiGmhgJoYmjTJiBkYTRpphBkwkkCAaE00AqfkDUwR6inpp6hB5TaT0gbQiF9
> Uydf5yiiwVDA7LaqdUmArzByY/v3lNNi3+VyjkPAwb3muD/o/j2s7lV154Id0C0WQNQ3x2xve1y2
> lwGJVwHiYZMktjg7TPXhW+F1Kd6ragmy533fnPVoDjMOhx1EFLyogwMoRCqbrA/glfHT/t2gPFZQ
> oNnP3Cwm/5p7UTLb4uk1pR0em3V+/Kd7b4cTWU6jcER7AKyF8khOHssEz1g5wvA+mmu54EHu5c2+
> e3+GL4W4hbbDKtpQRXV5xR1dJqcVLbYskKEOmNnIojj6VjztdF9Upjwsvk8Oo/2L/gnBdRLEoqH4
> k41NbNMWO4LCZyvuRyK4E4q5S9jZIQFHquma2KyhhhTZEH9WLNluevC4xrGFCmtxjBQ94zZ0qWRw
> A/QDEHfQYYaNIMvwjiJjOvMluhffi97SjCMxONfuCwVziaC0Ub9Jt9D7bJNM4+67r88fYksrvs6E
> CPo7KAjyiFuD8lzj3vp2wJbQa9GXzZeMLanAIYMdMcmZZRKO9hK90joKr2SkGCgMYSA4KDA4bQ3h
> xDIPES2iXqOIxPaHkDCOng2AczQdtPo3CNzg+s7DxaLViTn9sdjdZb8w4pMnz9XLv56g60k8jUHL
> gETM3r4XQlnkZC1DYlPlrvzhnUSS1mdeasym+ggiRkM4k0oOaPulSEg+MQYCVTUYhq6FQ9dffMUm
> VUbo/jgG8IhxvwZioYxb+5oPfIcSy8Xv2xXQpMw0KiIkxXHTNnc+wH5mcJmwuFoNpkOPJPGUDFeI
> Qs4GVmwhff9obggkHCko1ravI2+nAmThCv7CBqe4wocLSTLiA41jyo8xGfd3utqu1BqB6FDGJIGe
> r1rIDh5qUoqG0wLyvAY1IWydV2vADrCIYzQ5V0iZ6sHYMOWtawhbaawgONoX/vIR5u2BMqz1lWIV
> 4gFBvjwIXDthzzBaP43lOzgOkIfdoESr108CJYSDmQrPAjTXpB3CBCgiTmjHiakvITmivnEM+HFT
> VGq3whBKSomMXDFrmTiKtWsRAgjCY83lww+AZZy0b2dhkE3Rgbk9wONh3jLpNEbbry8vJJDIMqmo
> QS9o0BjBcYkhrYFQUDi8ORadYd57zzrKm66x7GeUYDg7URwCorPR6btioxxsuKBJWESxhxMhlVz9
> B6GBaptyz0ikRbjjGHAigwqH6CJ49PtDHveLE8rPYVZTxtC7nr4g0K7cP3gfYL3LEqMbbf7/8HtE
> HgWo5arRUQw6VAQDX95AwYmhWim8JCtTA/YBgoCK+oVBFfQLFXA0BWngPApBwwkQZBUMwX+BwvD/
> EmmAbAMw2pEhiFYWK1fUX7B+6xCSRUvqOA/gJhpA0hYK0CwIEmG4WwLAJsdANapQDRMtS/kLkyoC
> pbfQXEOpPO9MdTAfoj8IwfGAokO6Au8XogxLGZD6iVBANR1F0DN8uB7zoPhE3jnSRjAuET4lWIs2
> BgWR92KyVI515gVn3VDCiHowVflUP1hc8oYzaySpEfkkqCQj1hhaIHjKRZs3SAhWMQNpeDIXIKnS
> axs5HrJKHq7XMkkgqcSvcMfI3mj8fDckjVTa3VWB8qej1XtY80nifomw2olYTGJD0fIp6wMyRSfF
> bTsXz7BxZVosUJwzUk6tJWJSZ9+BWq2IfU6OKEdCvh5mw00NZcbi/BFpaiIJLBF4uOs5EQfmYb10
> KMGWBFMXyEzwtapCxdfw2PpBoxjV0BJ6YDRdcDUuIOFEEN0Y2DMmHQ73FV8Ux1ckzyhxvKBdxW1Y
> TOW0N1FgMRFEcixXsA+9XGQxwJ0KgcKvTk5/5uFlXRvPpqIeDsMprhkq0yYD2jIkkRZMmKNjlnQU
> 8AuqjwTdSeJfewbm4DWA5xJKYPzl1KQjCaKkrkIrqtA9Gy5Jio5iHbDx2nMRQeR2EDtCwNv+l6lp
> Brh2w9fHthFlufyPdab9UQVLO8wxf1h37UD7m07f+y3RMDb4IzEYodyAa0Q8b/DklnBOPNg+An/F
> 7G4R/as6EVxAbizCO9gG0cbTkIhIxQdwPLty8RkqaFQU+5OXiJpeTX44VYOTn31XddTWCD2eq195
> e0hsYHL6lph1WkKBLozEfOPqEWHkQ0FhrR0rmsvXgtXf1DKdT7UlvG+bM4UubBE8BFXvLQtU+Nwd
> AIBNgSyLexckmGBlAYHAdLTw4CKwIhTRV8PaT2EiJ7LTLNfrXeewchlCzHie1JXZ70MBSMcQyiEP
> U+htum0Ie3nViFm82HHd1PK1OrzUhqm+GxDGdtJO5iJ0CZouzNF1ExgWtlLETz5shQjXeXR7Wl7w
> YU0eTBq9mdAgsQMyTV635fSIu68zbHxeSRLHwrvh3UlL25jaQ0/E0iF5KxUV2AwKZMIolJOLDwuV
> 4DNBwy8WKwAcTiLiERcqLr72XPn3IeT68qQmph44gfYBL13XZZDv3GPf4GhcMw45CNiaGucL5WFG
> mlw2KqeHXNRAzKZCHCZJdgdnKHTVF61Mnzq5OOGGAa+7ZbkGgcsZAy8TajLoGYjPJ51Rc6Sc6WMe
> uIlppy1M1UBHcLCTxkiVKjSVsUEuZNyNM13YrUNbOyoUCwsLhFFqTCULkMUJUOo6w5iVW1oG93bF
> rWBoQjmdCAZq2CsR0GwtPgkowJI9zCqYQ1f/mrkwYtpsG3Dh8il0qI8RKTBF84BBPsD9QN7BxTAh
> 4HdZaUpvIVxID4M5STrLqSIbS7dnK8gT43IogOL2A1HPo0K1HidxrvIbaa8FTNBuUO2iILmjGFx6
> disK6vVIlJrPDCA0826lV3u3tWBx9Qb0GbbQ2ktForcfKY9UhHjQrzxDDpsCArVaxkNkhjbEster
> cXo9hpHAYsS2O1bHIc+l4pMScOk+lVTRXJaF3v8xEY37JzDqtEVUR5HRTEf/F3JFOFCQLvPhXA==
>
>
cheers,
Andrei
