Vicențiu Ciorbaru <[email protected]> writes: > I'd like a review from you for the whole patch, while I work on adding all > the relevant tests from MySQL. To me, it seems like a pretty big change
> https://github.com/MariaDB/server/compare/10.1-MDEV-6877-binlog_row_image It looks ok. A few comments below. Though generally, it seems best to stay as closely as possible to MySQL code, right? So you should take them only as suggestions, probably. - Kristian. > + virtual bool read_write_bitmaps_cmp(TABLE *table) > + case DELETE_ROWS_EVENT: > + case UPDATE_ROWS_EVENT: > + case WRITE_ROWS_EVENT: > + default: > + /* > + We should just compare bitmaps for Delete, Write > + or Update rows events. > + */ > + DBUG_ASSERT(0); Why is this a virtual method? There is nothing in the patch that overrides it, at it asserts if called with arbitrary Log_event, so seems it should just be a normal method, maybe even in class Rows_log_event() with a static_cast<> at the call site? > + // Unpack the current row into m_table->record[0], but with > + // a different columns bitmap. > + int unpack_current_row(rpl_group_info *rgi, MY_BITMAP const *cols) The style guide uses /*\n ...\n*/ for multi-line comments I think. > @@ -6217,24 +6237,96 @@ int THD::binlog_delete_row(TABLE* table, bool > is_trans, > > uchar *row_data= memory.slot(0); > > - size_t const len= pack_row(table, cols, row_data, record); > + DBUG_DUMP("table->read_set", (uchar*) table->read_set->bitmap, > (table->s->fields + 7) / 8); Overlong line (style guide). _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
