Skip to content

Commit

Permalink
unpack_row: unpack a correct number of fields
Browse files Browse the repository at this point in the history
  • Loading branch information
FooBarrior committed May 6, 2023
1 parent 975cd6c commit c3879da
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 13 deletions.
15 changes: 15 additions & 0 deletions mysql-test/main/alter_table_online_debug.result
Expand Up @@ -1346,6 +1346,21 @@ c x
2 123456
3 123456
drop table t;
# Test that all the fields are unpacked.
create table t (a int, b int) engine=innodb;
insert into t values (NULL, 123), (NULL, 456);
set debug_sync= "alter_table_copy_end signal copy wait_for goon";
alter table t drop a, add primary key(b), algorithm=copy;
connection con1;
set debug_sync= "now wait_for copy";
update t set b = b + 100;
set debug_sync= "now signal goon";
connection default;
select * from t;
b
223
556
drop table t;
#
# MDEV-31058 ER_KEY_NOT_FOUND upon concurrent CHANGE column to autoinc
# and DML
Expand Down
20 changes: 20 additions & 0 deletions mysql-test/main/alter_table_online_debug.test
Expand Up @@ -1527,6 +1527,26 @@ set debug_sync= "now signal goon";
select * from t;
drop table t;

--echo # Test that all the fields are unpacked.
create table t (a int, b int) engine=innodb;
insert into t values (NULL, 123), (NULL, 456);

set debug_sync= "alter_table_copy_end signal copy wait_for goon";
send alter table t drop a, add primary key(b), algorithm=copy;
--connection con1
set debug_sync= "now wait_for copy";
update t set b = b + 100;

set debug_sync= "now signal goon";

--connection default
--reap
select * from t;

drop table t;



--echo #
--echo # MDEV-31058 ER_KEY_NOT_FOUND upon concurrent CHANGE column to autoinc
--echo # and DML
Expand Down
30 changes: 17 additions & 13 deletions sql/rpl_record.cc
Expand Up @@ -228,24 +228,28 @@ int unpack_row(const rpl_group_info *rgi, TABLE *table, uint const colcnt,
const TABLE *conv_table= rpl_data.conv_table;
DBUG_PRINT("debug", ("Table data: tabldef: %p, conv_table: %p",
tabledef, conv_table));

bool is_online_alter= rpl_data.is_online_alter();
DBUG_ASSERT(rgi);

for (field_ptr= begin_ptr; field_ptr < end_ptr && *field_ptr; ++field_ptr)
for (field_ptr= begin_ptr; field_ptr < end_ptr
/* In Online Alter conv_table can be wider than
original table, but we need to unpack it all. */
&& (*field_ptr || is_online_alter);
++field_ptr)
{
/*
If there is a conversion table, we pick up the field pointer to
the conversion table. If the conversion table or the field
pointer is NULL, no conversions are necessary.
*/
Field *conv_field=
conv_table ? conv_table->field[field_ptr - begin_ptr] : NULL;
Field *const f=
conv_field ? conv_field : *field_ptr;
DBUG_PRINT("debug", ("Conversion %srequired for field '%s' (#%ld)",
Field *conv_field= conv_table ? conv_table->field[i] : NULL;
Field *const f= conv_field ? conv_field : *field_ptr;
#ifdef DBUG_TRACE
Field *dbg= is_online_alter ? f : *field_ptr;
#endif
DBUG_PRINT("debug", ("Conversion %srequired for field '%s' (#%u)",
conv_field ? "" : "not ",
(*field_ptr)->field_name.str,
(long) (field_ptr - begin_ptr)));
dbg->field_name.str, i));
DBUG_ASSERT(f != NULL);

/*
Expand All @@ -254,7 +258,7 @@ int unpack_row(const rpl_group_info *rgi, TABLE *table, uint const colcnt,
*/
if (bitmap_is_set(cols, (uint)(field_ptr - begin_ptr)))
{
if (!rpl_data.is_online_alter())
if (!is_online_alter)
(*field_ptr)->set_has_explicit_value();
if ((null_mask & 0xFF) == 0)
{
Expand Down Expand Up @@ -351,7 +355,7 @@ int unpack_row(const rpl_group_info *rgi, TABLE *table, uint const colcnt,
conv_field->sql_type(source_type);
conv_field->val_str(&value_string);
DBUG_PRINT("debug", ("Copying field '%s' of type '%s' with value '%s'",
(*field_ptr)->field_name.str,
dbg->field_name.str,
source_type.c_ptr_safe(), value_string.c_ptr_safe()));
#endif
copy.set(*field_ptr, f, TRUE);
Expand All @@ -362,7 +366,7 @@ int unpack_row(const rpl_group_info *rgi, TABLE *table, uint const colcnt,
(*field_ptr)->sql_type(target_type);
(*field_ptr)->val_str(&value_string);
DBUG_PRINT("debug", ("Value of field '%s' of type '%s' is now '%s'",
(*field_ptr)->field_name.str,
dbg->field_name.str,
target_type.c_ptr_safe(), value_string.c_ptr_safe()));
#endif
}
Expand Down Expand Up @@ -442,7 +446,7 @@ int unpack_row(const rpl_group_info *rgi, TABLE *table, uint const colcnt,
*current_row_end = pack_ptr;
if (master_reclength)
{
if (*field_ptr)
if (!is_online_alter && *field_ptr)
*master_reclength = (ulong)((*field_ptr)->ptr - table->record[0]);
else
*master_reclength = table->s->reclength;
Expand Down
2 changes: 2 additions & 0 deletions sql/sql_table.cc
Expand Up @@ -12070,6 +12070,8 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
#ifdef HAVE_REPLICATION
if (online && error < 0)
{
MEM_UNDEFINED(from->record[0], from->s->rec_buff_length * 2);
MEM_UNDEFINED(to->record[0], to->s->rec_buff_length * 2);
thd_progress_next_stage(thd);
Table_map_log_event table_event(thd, from, from->s->table_map_id,
from->file->has_transactions());
Expand Down

0 comments on commit c3879da

Please sign in to comment.