← Back to team overview

maria-developers team mailing list archive

Re: Please review a patch for MDEV-19284 and MDEV-19285 (instant ALTER)

 

Hi Bar,

I see that these bugs were based on wrong assumptions that ASCII or
UCS2 columns would only contain valid ASCII or UTF-16 data. We should
have tested these assumptions during the development of MDEV-15564.
Luckily these were found before the GA release of MariaDB 10.4.

Here is just a quick note before I will return to work on Tuesday.

Please remove ,algorithm=copy from tests. Instead, use
--enable_info
alter table ...
--disable_info
and make sure that the table contains at least 1 row when the ALTER
TABLE is executed. In that way, ALGORITHM=COPY will report that a
nonzero amount of rows were affected, while the native ALTER would
report that 0 rows were affected.

I would also suggest testing with some bad data (non-ASCII chars in
ASCII column, or Unicode surrogate pairs in UCS2) to see what kind of
errors would be triggered (if any), or what the converted data would
look like after ALTER IGNORE TABLE.

Please run the commit comment through a spell-checker. I spotted the
typos "correspoding" and "actuallt".

Also, please mention in the commit comment that these are regressions
caused by MDEV-15564. That should make our lifes easier, should we
ever decide to backport MDEV-15564 to earlier versions.

I will review the code and tests in detail next week. I hope that
Eugene can review it as well.

Best regards,

Marko


Follow ups

References