![](https://secure.gravatar.com/avatar/e1d656a55b3b29f2f0872cde35c6143c.jpg?s=120&d=mm&r=g)
Hi Sachin, I've reviewed the code in bb-10.5-19708. It looks good. I suggest some cleanups. - Remove Field::binlog_type_info_slave, it's not used. - Remove Field::binlog_type_info_master. Please don't cache Binlog_type_info inside the Field itself. Let the caller code cache it in an array of Binlog_type_info pointers. - Change this method in Field: virtual Binlog_type_info *binlog_type_info(); to virtual Binlog_type_info *binlog_type_info(MEM_ROOT *) const; Let's pass mem_root as a parameter instead of using table->mem_root. Please add the 'const' qualifier! - Restore the 'const' qualifier to Field::enum_conv_type rpl_conv_type_from - Instead of doing this: Binlog_type_info * Field_new_decimal::binlog_type_info() { Field_num::binlog_type_info(); this->binlog_type_info_master->m_metadata= precision + (decimals() << 8); this->binlog_type_info_master->m_metadata_size= 2; return this->binlog_type_info_master; } Please add a number of Binlog_type_info constructor helpers, in addition to the current one. For example, for numeric types this would be good: Binlog_type_info(uchar type_code, uint16 metadata, uint8 metadata_size, binlog_signess_t signess) :m_type_code(type_code), m_metadata(metadata), m_metadata_size(metadata_size), m_signess(signess), m_cs(NULL), m_enum_typelib(NULL), m_set_typelib(NULL), m_geom_type(GEOM_GEOMETRY) { }; so you can do something simple like this: Binlog_type_info Field_new_decimal::binlog_type_info(MEM_ROOT *root) const { return new (root) Binlog_type_info(type(), precision + (decimals() << 8), 2, signess_arg); } - It seems you always set signess to SIGNESS_NOT_RELEVANT. Where is signess set to SIGNED or UNSIGNED? - This is something we've never used before: +#include <vector> +#include <string> +#include <functional> +#include <memory> We need to discuss this with Serg. Is it possible to avoid this? - There is a new test file mysql-test/suite/rpl/t/rpl_mdev_19708.test.diff but I could not find a corresponding .result.diff. Where is it? - Please use error names instead of numbers in tests: Instead of: --error 1231 SET GLOBAL binlog_row_metadata = -1; it should be: --error ER_WRONG_VALUE_FOR_VAR SET GLOBAL binlog_row_metadata = -1; You can find error names in include/mysqld_ername.h Thanks.