Hi Mats!
Source patch is good, but some problems with test case: see below.
/Sven
Mats Kindahl wrote:
[...]
> diff -Nrup a/mysql-test/suite/bugs/t/rpl_bug31583.test
> b/mysql-test/suite/bugs/t/rpl_bug31583.test
> --- /dev/null Wed Dec 31 16:00:00 196900
> +++ b/mysql-test/suite/bugs/t/rpl_bug31583.test 2007-12-05 20:00:06 +01:00
> @@ -0,0 +1,21 @@
> +#
> +# BUG#31583: 5.1-telco-6.1 -> 5.1.22. Slave returns Error in unknown event
> +
> +# This is a problem for any update statement replicating from an old
> +# server to a new server. The bug consisted of a new slave trying to
> +# read two column bitmaps, but there is only one available in the old
> +# format.
> +
> +# This test case should be executed replicating from an old server to
> +# a new server, so make sure you have one handy.
These instructions aren't enough to set up the test. AFAIU, one needs to
follow the instructions by skozlov in the bug report in order to execute
the test. This cannot be done in any easy way, especially as the patched
m4r file does not work with the newest version. I'd suggest to instead
check in a binlog file from the old version and read that instead.
> +
> +source include/master-slave.inc;
> +
> +CREATE TABLE t1 ( a INT, b INT DEFAULT -3 );
> +
> +INSERT INTO t1 VALUES (1, DEFAULT);
> +UPDATE t1 SET a = 3;
> +SELECT * FROM t1 ORDER BY a;
> +sync_slave_with_master;
> +SELECT * FROM t1 ORDER BY a;
I'm not sure what this test is supposed to do. Shouldn't the second
select be on the slave (after connection slave)?
> +
> diff -Nrup a/sql/log_event.cc b/sql/log_event.cc
> --- a/sql/log_event.cc 2007-11-28 12:11:13 +01:00
> +++ b/sql/log_event.cc 2007-12-05 20:00:06 +01:00
> @@ -490,6 +490,9 @@ const char* Log_event::get_type_str()
> case USER_VAR_EVENT: return "User var";
> case FORMAT_DESCRIPTION_EVENT: return "Format_desc";
> case TABLE_MAP_EVENT: return "Table_map";
> + case PRE_GA_WRITE_ROWS_EVENT: return "Write_rows_event_old";
> + case PRE_GA_UPDATE_ROWS_EVENT: return "Update_rows_event_old";
> + case PRE_GA_DELETE_ROWS_EVENT: return "Delete_rows_event_old";
> case WRITE_ROWS_EVENT: return "Write_rows";
> case UPDATE_ROWS_EVENT: return "Update_rows";
> case DELETE_ROWS_EVENT: return "Delete_rows";
> @@ -1015,6 +1018,8 @@ Log_event* Log_event::read_log_event(con
> DBUG_ENTER("Log_event::read_log_event(char*,...)");
> DBUG_ASSERT(description_event != 0);
> DBUG_PRINT("info", ("binlog_version: %d", description_event->binlog_version));
> + DBUG_DUMP("data", (unsigned char*) buf, event_len);
> +
> /* Check the integrity */
> if (event_len < EVENT_LEN_OFFSET ||
> buf[EVENT_TYPE_OFFSET] >= ENUM_END_EVENT ||
> diff -Nrup a/sql/log_event_old.cc b/sql/log_event_old.cc
> --- a/sql/log_event_old.cc 2007-11-21 16:53:44 +01:00
> +++ b/sql/log_event_old.cc 2007-12-05 20:00:06 +01:00
> @@ -1317,6 +1317,7 @@ Old_rows_log_event::Old_rows_log_event(c
> post_header_len));
>
> const char *post_start= buf + common_header_len;
> + DBUG_DUMP("post_header", (uchar*) post_start, post_header_len);
> post_start+= RW_MAPID_OFFSET;
> if (post_header_len == 6)
> {
> @@ -1358,38 +1359,11 @@ Old_rows_log_event::Old_rows_log_event(c
> DBUG_VOID_RETURN;
> }
>
> - m_cols_ai.bitmap= m_cols.bitmap; /* See explanation in is_valid() */
> -
> - if (event_type == PRE_GA_UPDATE_ROWS_EVENT)
> - {
> - DBUG_PRINT("debug", ("Reading from %p", ptr_after_width));
> -
> - /* if bitmap_init fails, caught in is_valid() */
> - if (likely(!bitmap_init(&m_cols_ai,
> - m_width <= sizeof(m_bitbuf_ai)*8 ? m_bitbuf_ai :
> NULL,
> - m_width,
> - false)))
> - {
> - DBUG_PRINT("debug", ("Reading from %p", ptr_after_width));
> - memcpy(m_cols_ai.bitmap, ptr_after_width, (m_width + 7) / 8);
> - create_last_word_mask(&m_cols_ai);
> - ptr_after_width+= (m_width + 7) / 8;
> - DBUG_DUMP("m_cols_ai", (uchar*) m_cols_ai.bitmap,
> - no_bytes_in_map(&m_cols_ai));
> - }
> - else
> - {
> - // Needed because bitmap_init() does not set it to null on failure
> - m_cols_ai.bitmap= 0;
> - DBUG_VOID_RETURN;
> - }
> - }
> -
> const uchar* const ptr_rows_data= (const uchar*) ptr_after_width;
> -
> size_t const data_size= event_len - (ptr_rows_data - (const uchar *) buf);
> DBUG_PRINT("info",("m_table_id: %lu m_flags: %d m_width: %lu data_size: %lu",
> m_table_id, m_flags, m_width, (ulong) data_size));
> + DBUG_DUMP("rows_data", (uchar*) ptr_rows_data, data_size);
>
> m_rows_buf= (uchar*) my_malloc(data_size, MYF(MY_WME));
> if (likely((bool)m_rows_buf))
> @@ -1419,24 +1393,18 @@ Old_rows_log_event::~Old_rows_log_event(
>
> int Old_rows_log_event::get_data_size()
> {
> - int const type_code= get_type_code();
> -
> uchar buf[sizeof(m_width)+1];
> uchar *end= net_store_length(buf, (m_width + 7) / 8);
>
> DBUG_EXECUTE_IF("old_row_based_repl_4_byte_map_id_master",
> return 6 + no_bytes_in_map(&m_cols) + (end - buf) +
> - (type_code == PRE_GA_UPDATE_ROWS_EVENT ?
> no_bytes_in_map(&m_cols_ai) : 0) +
> (m_rows_cur - m_rows_buf););
> int data_size= ROWS_HEADER_LEN;
> data_size+= no_bytes_in_map(&m_cols);
> data_size+= end - buf;
>
> - if (type_code == PRE_GA_UPDATE_ROWS_EVENT)
> - data_size+= no_bytes_in_map(&m_cols_ai);
> -
> data_size+= (m_rows_cur - m_rows_buf);
> - return data_size;
> + return data_size;
> }
>
>
> @@ -2011,16 +1979,6 @@ bool Old_rows_log_event::write_data_body
> DBUG_DUMP("m_cols", (uchar*) m_cols.bitmap, no_bytes_in_map(&m_cols));
> res= res || my_b_safe_write(file, (uchar*) m_cols.bitmap,
> no_bytes_in_map(&m_cols));
> - /*
> - TODO[refactor write]: Remove the "down cast" here (and elsewhere).
> - */
> - if (get_type_code() == PRE_GA_UPDATE_ROWS_EVENT)
> - {
> - DBUG_DUMP("m_cols_ai", (uchar*) m_cols_ai.bitmap,
> - no_bytes_in_map(&m_cols_ai));
> - res= res || my_b_safe_write(file, (uchar*) m_cols_ai.bitmap,
> - no_bytes_in_map(&m_cols_ai));
> - }
> DBUG_DUMP("rows", m_rows_buf, data_size);
> res= res || my_b_safe_write(file, m_rows_buf, (size_t) data_size);
>
> @@ -2831,36 +2789,8 @@ Update_rows_log_event_old::Update_rows_l
>
> // This constructor should not be reached.
> assert(0);
> -
> - init(cols);
> -}
> -
> -
> -void Update_rows_log_event_old::init(MY_BITMAP const *cols)
> -{
> - /* if bitmap_init fails, caught in is_valid() */
> - if (likely(!bitmap_init(&m_cols_ai,
> - m_width <= sizeof(m_bitbuf_ai)*8 ? m_bitbuf_ai : NULL,
> - m_width,
> - false)))
> - {
> - /* Cols can be zero if this is a dummy binrows event */
> - if (likely(cols != NULL))
> - {
> - memcpy(m_cols_ai.bitmap, cols->bitmap, no_bytes_in_map(cols));
> - create_last_word_mask(&m_cols_ai);
> - }
> - }
> }
> #endif /* !defined(MYSQL_CLIENT) */
> -
> -
> -Update_rows_log_event_old::~Update_rows_log_event_old()
> -{
> - if (m_cols_ai.bitmap == m_bitbuf_ai) // no my_malloc happened
> - m_cols_ai.bitmap= 0; // so no my_free in bitmap_free
> - bitmap_free(&m_cols_ai); // To pair with bitmap_init().
> -}
>
>
> /*
> diff -Nrup a/sql/log_event_old.h b/sql/log_event_old.h
> --- a/sql/log_event_old.h 2007-11-20 19:49:34 +01:00
> +++ b/sql/log_event_old.h 2007-12-05 20:00:06 +01:00
> @@ -175,14 +175,6 @@ protected:
> ulong m_table_id; /* Table ID */
> MY_BITMAP m_cols; /* Bitmap denoting columns available */
> ulong m_width; /* The width of the columns bitmap */
> - /*
> - Bitmap for columns available in the after image, if present. These
> - fields are only available for Update_rows events. Observe that the
> - width of both the before image COLS vector and the after image
> - COLS vector is the same: the number of columns of the table on the
> - master.
> - */
> - MY_BITMAP m_cols_ai;
>
> ulong m_master_reclength; /* Length of record on master side */
>
> @@ -442,12 +434,8 @@ public:
> Update_rows_log_event_old(THD*, TABLE*, ulong table_id,
> MY_BITMAP const *cols,
> bool is_transactional);
> -
> - void init(MY_BITMAP const *cols);
> #endif
>
> - virtual ~Update_rows_log_event_old();
> -
> #ifdef HAVE_REPLICATION
> Update_rows_log_event_old(const char *buf, uint event_len,
> const Format_description_log_event *description_event);
> @@ -465,11 +453,6 @@ public:
> cols, fields, before_record, after_record);
> }
> #endif
> -
> - virtual bool is_valid() const
> - {
> - return Old_rows_log_event::is_valid() && m_cols_ai.bitmap;
> - }
>
> protected:
> #ifdef MYSQL_CLIENT
>
--
Sven Sandberg, Software Engineer
MySQL AB, www.mysql.com
