Improve compatibility for ALTER TABLE algorithms

[kolbe]

Feature Request

Is your feature request related to a problem? Please describe:

The changes implemented to address #8428 introduce incompatibilities with MySQL that make migrations difficult.

mysql> ALTER TABLE t1   MODIFY COLUMN e enum('A', 'B') NOT NULL DEFAULT 'A', ALGORITHM = inplace;
ERROR 1846 (0A000): ALGORITHM=INPLACE is not supported. Reason: Cannot alter table by INPLACE. Try ALGORITHM=INSTANT.

Describe the feature you'd like:

Our support of ALTER TABLE algorithms tries to match their names literally to the kind of operation being performed by TiDB, not to the kind of operation that a user of MySQL would expect.

We should add at least syntactic support for the same algorithms supported in MySQL. We could either do this by always allowing INSTANT/INPLACE for any ALTER TABLE statement in TiDB, or, if we feel there's a danger of surprise or worse, we could add a server option to give that behavior.

Describe alternatives you've considered:

A comment format that causes TiDB to ignore a statement but allow it to be executed by MySQL could be another option, though that would require modifications to a user's application or testing mechanisms.

Teachability, Documentation, Adoption, Migration Strategy:

[ghost]

I think the fix can be straight forward: make these purely assertions that ALGORITHM=x means "this or better", whereby:

INSTANT > INPLACE > COPY

This can be documented as a MySQL incompatibility, but I think it's reasonable. MySQL supports COPY for everything, because that's how it started.. everything was a copy, and then it started adding optimizations. TiDB was designed better from the start where many of these things are optimized.

I did a quick test in MySQL, and you really can force all 3 behaviors on some operations (even though my "copy" was faster than instant). I think guaranteeing "this or better" is fine:

DROP TABLE IF EXISTS t1;
CREATE TABLE t1 (
 id BIGINT NOT NULL PRIMARY KEY auto_increment,
 pad1 VARBINARY(1024)
);

INSERT INTO t1 SELECT NULL, RANDOM_BYTES(1024) FROM dual;
INSERT INTO t1 SELECT NULL, RANDOM_BYTES(1024) FROM t1 a JOIN t1 b JOIN t1 c LIMIT 10000;
INSERT INTO t1 SELECT NULL, RANDOM_BYTES(1024) FROM t1 a JOIN t1 b JOIN t1 c LIMIT 10000;
INSERT INTO t1 SELECT NULL, RANDOM_BYTES(1024) FROM t1 a JOIN t1 b JOIN t1 c LIMIT 10000;
INSERT INTO t1 SELECT NULL, RANDOM_BYTES(1024) FROM t1 a JOIN t1 b JOIN t1 c LIMIT 10000;
INSERT INTO t1 SELECT NULL, RANDOM_BYTES(1024) FROM t1 a JOIN t1 b JOIN t1 c LIMIT 10000;
INSERT INTO t1 SELECT NULL, RANDOM_BYTES(1024) FROM t1 a JOIN t1 b JOIN t1 c LIMIT 10000;
INSERT INTO t1 SELECT NULL, RANDOM_BYTES(1024) FROM t1 a JOIN t1 b JOIN t1 c LIMIT 10000;
INSERT INTO t1 SELECT NULL, RANDOM_BYTES(1024) FROM t1 a JOIN t1 b JOIN t1 c LIMIT 10000;
INSERT INTO t1 SELECT NULL, RANDOM_BYTES(1024) FROM t1 a JOIN t1 b JOIN t1 c LIMIT 10000;
INSERT INTO t1 SELECT NULL, RANDOM_BYTES(1024) FROM t1 a JOIN t1 b JOIN t1 c LIMIT 10000;
INSERT INTO t1 SELECT NULL, RANDOM_BYTES(1024) FROM t1 a JOIN t1 b JOIN t1 c LIMIT 10000;

ALTER TABLE t1 ADD pad2 VARBINARY(1024), ALGORITHM=INSTANT;
ALTER TABLE t1 ADD pad3 VARBINARY(1024), ALGORITHM=INPLACE;
ALTER TABLE t1 ADD pad4 VARBINARY(1024), ALGORITHM=COPY;
..

mysql [localhost:8021] {msandbox} (test) > ALTER TABLE t1 ADD pad2 VARBINARY(1024), ALGORITHM=INSTANT;
Query OK, 0 rows affected (0.06 sec)
Records: 0  Duplicates: 0  Warnings: 0

mysql [localhost:8021] {msandbox} (test) > ALTER TABLE t1 ADD pad3 VARBINARY(1024), ALGORITHM=INPLACE;
Query OK, 0 rows affected (13.95 sec)
Records: 0  Duplicates: 0  Warnings: 0

mysql [localhost:8021] {msandbox} (test) > ALTER TABLE t1 ADD pad4 VARBINARY(1024), ALGORITHM=COPY;
Query OK, 81010 rows affected (1.69 sec)
Records: 81010  Duplicates: 0  Warnings: 0

[zz-jason]

I mark this feature request as accepted. @bb7133 PTAL if we can improve this.

[bb7133]

I think the fix can be straight forward: make these purely assertions that ALGORITHM=x means "this or better”

I think this is valuable & reasonable, thanks @nullnotnil

[bb7133]

INSTANT > INPLACE > COPY

For now, the COPY algorithm is applicable for all DDL statements but INPLACE/INSTANT is not:

tidb> alter table t add column c int, algorithm=copy;
Query OK, 0 rows affected, 1 warning (0.03 sec)

tidb> show warnings;
+-------+------+---------------------------------------------------------------------------------------------+
| Level | Code | Message                                                                                     |
+-------+------+---------------------------------------------------------------------------------------------+
| Error | 1846 | ALGORITHM=COPY is not supported. Reason: Cannot alter table by COPY. Try ALGORITHM=INSTANT. |
+-------+------+---------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

IMHO if a 'better' algorithm is chosen by TiDB, we raise a warning just like the message for COPY, @nullnotnil are you fine with it?

[ghost]

IMHO if a 'better' algorithm is chosen by TiDB, we raise a warning just like the message for COPY, @nullnotnil are you fine with it?

LGTM