Details
-
New Feature
-
Status: Closed (View Workflow)
-
Critical
-
Resolution: Fixed
Description
The reasons for removing alter-algorithm are:
- alter-algorithm was introduced as a replacement for the old_alter_table that was used
to force the usage of the original alter table algorithm (copy) in the cases where the new
alter algorithm did not work. The new option was added as a way to force the usage of
a specific algorithm when it should instead have made it possible to disable algorithms
that would not work for some reason. - alter-algorithm introduced some cases where ALTER TABLE would not work without
specifying the ALGORITHM=XXX option together with ALTER TABLE - Having different values of alter-algorithm on master and slave could cause slave to
stop unexpectedly. - ALTER TABLE FORCE, as used by mariadb-upgrade, would not always work if
alter-algorithm was set for the server. - As part of the
MDEV-33449"improving repair of tables" it become clear that alter- algorithm made it harder to provide a better and more consistent ALTER TABLE FORCE and REPAIR TABLE and it would be better to remove it.
The task is to remove alter-algorithm but keep the variable as no-op to not affect old setups.
Attachments
Issue Links
- causes
-
MDEV-33737 The way of ignoring alter-algorithm is inconsistent with other options and with itself
-
- Closed
-
- includes
-
-
- Closed
-
- is part of
-
-
- Closed
-
- relates to
-
MDEV-13134 Introduce ALTER TABLE attributes ALGORITHM=NOCOPY and ALGORITHM=INSTANT
-
- Closed
-
-
-
- Closed
-
-
-
- Closed
-
Activity
Current status:
MDEV-33737needs to be addressed, after it's closed I will need to look at the result;- GCOV shows two not covered lines (line numbers as of e87a953ae62d0a83abddc0668be6773133f04b35, but the same misses exist in bb-11.5-monty):
===File sql/sql_alter.cc:189 : + algorithm(thd) == Alter_info::ALTER_TABLE_ALGORITHM_DEFAULT)===File sql/sql_table.cc:11150 : + alter_info->algorithm(thd) == Alter_info::ALTER_TABLE_ALGORITHM_DEFAULT)An example of a test which covers these:
CREATE TABLE t (a INT) ENGINE=MyISAM;
CREATE TABLE tm (a INT) ENGINE=MERGE, UNION(t);
ALTER TABLE tm COMMENT 'x', LOCK=SHARED;
Test results
as of bb-11.5-MDEV-32188-timestamps e9157560f
Repeating the note from MDEV-33737, while the main problem there has been fixed, the warning that is currently produced happens too earlier during the startup, so it isn't actually written into the error log unless STDERR is redirected there explicitly (as MTR does). However, I guess it will have to do for now; in any case, we have a general problem with having too many different mechanisms for deprecating, removing, ignoring, disabling and not-using variables, which we already had complaints about, it needs to be sorted out in a more systematic way than for one variable at a time.
I don't have objections against pushing this feature into 11.5 main and releasing with 11.5.1 RC; however, the branch/revision mentioned above cannot be used for this, as it contains two other features (MDEV-32188 and MDEV-33449) which haven't yet been signed off.
Also, since commits for different features are intervened in the branch, I couldn't check that the coverage misses mentioned before have been eliminated. Later I'll run another coverage check for all three features together.
It does not seem to me that the removed parameter alter_algorithm was ever deprecated in any earlier release. MDEV-30905 in 11.2 had removed the earlier deprecated (MDEV-18650) alias old_alter_table of alter_algorithm.
to test this feature separately please use bb-11.5-
MDEV-32188-timestamps