Hi, Nikita,
On May 05, Nikita Malyavin wrote:
> revision-id: 24c653be25a (mariadb-11.0.1-123-g24c653be25a)
> parent(s): c1cbda5a2c8
> author: Nikita Malyavin
> committer: Nikita Malyavin
> timestamp: 2023-05-04 20:49:43 +0300
> message:
>
> unpack_row: unpack a correct number of fields
>
> diff --git a/sql/rpl_record.cc b/sql/rpl_record.cc
> index 908d2c39d97..30bd80bdfc7 100644
> --- a/sql/rpl_record.cc
> +++ b/sql/rpl_record.cc
> @@ -228,24 +228,30 @@ 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);
I failed to understand this, could you please explain? The comment didn't
help much, unfortunately. The test case didn't either.
> + ++field_ptr)
> {
> + intptr fidx= field_ptr - begin_ptr;
intptr is a variant of int with sizeof(intptr) == sizeof(void*).
it's needed to cast a pointer to int, do some math, cast back.
here you need my_ptrdiff_t or ptrdiff_t
> /*
> 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 *conv_field= conv_table ? conv_table->field[fidx] : 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' (#%ld)",
> conv_field ? "" : "not ",
> - (*field_ptr)->field_name.str,
> - (long) (field_ptr - begin_ptr)));
> + dbg->field_name.str,
> + (long) ()));
eh? you surely meant `(long) fidx` ?
how did it even compile?
> DBUG_ASSERT(f != NULL);
Regards,
Sergei
VP of MariaDB Server Engineering
and secur...@mariadb.org
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
