> - 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.
The new table in online alter can have fewer columns than the old one, but we still have to go
through all the old table's columns.
For the replication case, the slave table can be narrower than the master one.
See rpl_extra_col_master.test. I found it by adding a corresponding assertion.
> + ++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
While refactoring, I found that `i` is what I needed. So, removing fidx altogether, in favor of `i`.
> /*
> 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?
Oops. Will also be `i` with %u format.