Skip to content

Commit

Permalink
MDEV-16288 ALTER TABLE…ALGORITHM=DEFAULT does not override alter_algo…
Browse files Browse the repository at this point in the history
…rithm

- ALTER_ALGORITHM should be substituted when there is no mention of
algorithm in alter statement.
- Introduced algorithm(thd) in Alter_info. It returns the
user requested algorithm. If user doesn't specify algorithm explicitly then
it returns alter_algorithm variable.
- changed algorithm() to get_algorithm(thd) to return algorithm name for
displaying the error.
- set_requested_algorithm(algo_value) to avoid direct assignment on
requested_algorithm variable.
- Avoid direct access of requested_algorithm to encapsulate
requested_algorithm variable
  • Loading branch information
Thirunarayanan committed May 4, 2020
1 parent f98017b commit ec9908b
Show file tree
Hide file tree
Showing 13 changed files with 143 additions and 50 deletions.
21 changes: 14 additions & 7 deletions mysql-test/suite/innodb/r/alter_algorithm,INPLACE.rdiff
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
--- alter_algorithm.result
+++ alter_algorithm.reject
@@ -7,40 +7,40 @@
--- alter_algorithm.result 2020-04-30 21:39:48.923115511 +0530
+++ alter_algorithm.reject 2020-04-30 21:45:04.131642093 +0530
@@ -7,43 +7,43 @@
INSERT INTO t1(f1, f2, f3) VALUES(1, 1, 1);
SELECT @@alter_algorithm;
@@alter_algorithm
Expand Down Expand Up @@ -54,11 +54,16 @@
-affected rows: 1
-info: Records: 1 Duplicates: 0 Warnings: 0
+affected rows: 0
+info: Records: 0 Duplicates: 0 Warnings: 0
ALTER TABLE t1 FORCE, ALGORITHM=DEFAULT;
-affected rows: 1
-info: Records: 1 Duplicates: 0 Warnings: 0
+affected rows: 0
+info: Records: 0 Duplicates: 0 Warnings: 0
DROP TABLE t1;
affected rows: 0
CREATE TABLE t1(f1 INT PRIMARY KEY, f2 INT NOT NULL,
@@ -53,22 +53,22 @@
@@ -56,22 +56,22 @@
FOREIGN KEY fidx(f1) REFERENCES t1(f1))ENGINE=INNODB;
INSERT INTO t1(f1, f2, f4, f5) VALUES(1, 2, 3, 4);
ALTER TABLE t1 ADD INDEX idx1(f4), page_compressed=1;
Expand Down Expand Up @@ -91,7 +96,7 @@
DROP TABLE t2, t1;
affected rows: 0
CREATE TABLE t1(f1 INT NOT NULL,
@@ -81,27 +81,27 @@
@@ -84,28 +84,27 @@
INSERT INTO t1(f1, f2) VALUES(1, 1);
# Add column at the end of the table
ALTER TABLE t1 ADD COLUMN f4 char(100) default 'BIG WALL';
Expand Down Expand Up @@ -119,7 +124,9 @@
+info: Records: 0 Duplicates: 0 Warnings: 0
# Rename table
ALTER TABLE t1 RENAME t3;
affected rows: 0
-affected rows: 1
-info: Records: 1 Duplicates: 0 Warnings: 0
+affected rows: 0
# Drop Virtual Column
ALTER TABLE t3 DROP COLUMN vcol;
-affected rows: 1
Expand All @@ -129,7 +136,7 @@
# Column length varies
ALTER TABLE t2 CHANGE f3 f3 VARCHAR(20);
affected rows: 0
@@ -109,12 +109,12 @@
@@ -113,12 +112,12 @@
SET foreign_key_checks = 0;
affected rows: 0
ALTER TABLE t3 ADD FOREIGN KEY fidx(f2) REFERENCES t2(f1);
Expand Down
21 changes: 14 additions & 7 deletions mysql-test/suite/innodb/r/alter_algorithm,INSTANT.rdiff
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
--- alter_algorithm.result
+++ alter_algorithm.reject
@@ -7,40 +7,32 @@
--- alter_algorithm.result 2020-04-30 21:39:48.923115511 +0530
+++ alter_algorithm.reject 2020-04-30 21:47:08.245465018 +0530
@@ -7,43 +7,35 @@
INSERT INTO t1(f1, f2, f3) VALUES(1, 1, 1);
SELECT @@alter_algorithm;
@@alter_algorithm
Expand Down Expand Up @@ -47,10 +47,15 @@
-affected rows: 1
-info: Records: 1 Duplicates: 0 Warnings: 0
+Got one of the listed errors
ALTER TABLE t1 FORCE, ALGORITHM=DEFAULT;
-affected rows: 1
-info: Records: 1 Duplicates: 0 Warnings: 0
+affected rows: 0
+info: Records: 0 Duplicates: 0 Warnings: 0
DROP TABLE t1;
affected rows: 0
CREATE TABLE t1(f1 INT PRIMARY KEY, f2 INT NOT NULL,
@@ -53,22 +45,17 @@
@@ -56,22 +48,17 @@
FOREIGN KEY fidx(f1) REFERENCES t1(f1))ENGINE=INNODB;
INSERT INTO t1(f1, f2, f4, f5) VALUES(1, 2, 3, 4);
ALTER TABLE t1 ADD INDEX idx1(f4), page_compressed=1;
Expand Down Expand Up @@ -78,7 +83,7 @@
DROP TABLE t2, t1;
affected rows: 0
CREATE TABLE t1(f1 INT NOT NULL,
@@ -81,27 +68,27 @@
@@ -84,28 +71,27 @@
INSERT INTO t1(f1, f2) VALUES(1, 1);
# Add column at the end of the table
ALTER TABLE t1 ADD COLUMN f4 char(100) default 'BIG WALL';
Expand Down Expand Up @@ -106,7 +111,9 @@
+info: Records: 0 Duplicates: 0 Warnings: 0
# Rename table
ALTER TABLE t1 RENAME t3;
affected rows: 0
-affected rows: 1
-info: Records: 1 Duplicates: 0 Warnings: 0
+affected rows: 0
# Drop Virtual Column
ALTER TABLE t3 DROP COLUMN vcol;
-affected rows: 1
Expand All @@ -116,7 +123,7 @@
# Column length varies
ALTER TABLE t2 CHANGE f3 f3 VARCHAR(20);
affected rows: 0
@@ -109,12 +96,12 @@
@@ -113,12 +99,12 @@
SET foreign_key_checks = 0;
affected rows: 0
ALTER TABLE t3 ADD FOREIGN KEY fidx(f2) REFERENCES t2(f1);
Expand Down
21 changes: 14 additions & 7 deletions mysql-test/suite/innodb/r/alter_algorithm,NOCOPY.rdiff
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
--- alter_algorithm.result
+++ alter_algorithm.reject
@@ -7,40 +7,32 @@
--- alter_algorithm.result 2020-04-30 21:39:48.923115511 +0530
+++ alter_algorithm.reject 2020-04-30 21:52:10.785967739 +0530
@@ -7,43 +7,35 @@
INSERT INTO t1(f1, f2, f3) VALUES(1, 1, 1);
SELECT @@alter_algorithm;
@@alter_algorithm
Expand Down Expand Up @@ -47,10 +47,15 @@
-affected rows: 1
-info: Records: 1 Duplicates: 0 Warnings: 0
+Got one of the listed errors
ALTER TABLE t1 FORCE, ALGORITHM=DEFAULT;
-affected rows: 1
-info: Records: 1 Duplicates: 0 Warnings: 0
+affected rows: 0
+info: Records: 0 Duplicates: 0 Warnings: 0
DROP TABLE t1;
affected rows: 0
CREATE TABLE t1(f1 INT PRIMARY KEY, f2 INT NOT NULL,
@@ -53,22 +45,22 @@
@@ -56,22 +48,22 @@
FOREIGN KEY fidx(f1) REFERENCES t1(f1))ENGINE=INNODB;
INSERT INTO t1(f1, f2, f4, f5) VALUES(1, 2, 3, 4);
ALTER TABLE t1 ADD INDEX idx1(f4), page_compressed=1;
Expand Down Expand Up @@ -83,7 +88,7 @@
DROP TABLE t2, t1;
affected rows: 0
CREATE TABLE t1(f1 INT NOT NULL,
@@ -81,27 +73,27 @@
@@ -84,28 +76,27 @@
INSERT INTO t1(f1, f2) VALUES(1, 1);
# Add column at the end of the table
ALTER TABLE t1 ADD COLUMN f4 char(100) default 'BIG WALL';
Expand Down Expand Up @@ -111,7 +116,9 @@
+info: Records: 0 Duplicates: 0 Warnings: 0
# Rename table
ALTER TABLE t1 RENAME t3;
affected rows: 0
-affected rows: 1
-info: Records: 1 Duplicates: 0 Warnings: 0
+affected rows: 0
# Drop Virtual Column
ALTER TABLE t3 DROP COLUMN vcol;
-affected rows: 1
Expand All @@ -121,7 +128,7 @@
# Column length varies
ALTER TABLE t2 CHANGE f3 f3 VARCHAR(20);
affected rows: 0
@@ -109,12 +101,12 @@
@@ -113,12 +104,12 @@
SET foreign_key_checks = 0;
affected rows: 0
ALTER TABLE t3 ADD FOREIGN KEY fidx(f2) REFERENCES t2(f1);
Expand Down
6 changes: 5 additions & 1 deletion mysql-test/suite/innodb/r/alter_algorithm.result
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ info: Records: 1 Duplicates: 0 Warnings: 0
ALTER TABLE t1 ENGINE=INNODB;
affected rows: 1
info: Records: 1 Duplicates: 0 Warnings: 0
ALTER TABLE t1 FORCE, ALGORITHM=DEFAULT;
affected rows: 1
info: Records: 1 Duplicates: 0 Warnings: 0
DROP TABLE t1;
affected rows: 0
CREATE TABLE t1(f1 INT PRIMARY KEY, f2 INT NOT NULL,
Expand Down Expand Up @@ -97,7 +100,8 @@ affected rows: 1
info: Records: 1 Duplicates: 0 Warnings: 0
# Rename table
ALTER TABLE t1 RENAME t3;
affected rows: 0
affected rows: 1
info: Records: 1 Duplicates: 0 Warnings: 0
# Drop Virtual Column
ALTER TABLE t3 DROP COLUMN vcol;
affected rows: 1
Expand Down
20 changes: 20 additions & 0 deletions mysql-test/suite/innodb/r/alter_algorithm2.result
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,23 @@ DROP PROCEDURE p1;
affected rows: 0
DROP PROCEDURE p2;
affected rows: 0
CREATE TABLE t1(id INT PRIMARY KEY,
col1 INT UNSIGNED NOT NULL UNIQUE)ENGINE=InnoDB;
affected rows: 0
INSERT INTO t1 VALUES(1,1),(2,2),(3,3);
affected rows: 3
info: Records: 3 Duplicates: 0 Warnings: 0
SET ALTER_ALGORITHM=INSTANT;
affected rows: 0
ALTER TABLE t1 DROP COLUMN col1;
ERROR 0A000: ALGORITHM=INSTANT is not supported for this operation. Try ALGORITHM=INPLACE
ALTER TABLE t1 DROP COLUMN col1, ALGORITHM=NOCOPY;
ERROR 0A000: ALGORITHM=NOCOPY is not supported for this operation. Try ALGORITHM=INPLACE
ALTER TABLE t1 DROP COLUMN col1, ALGORITHM=DEFAULT;
affected rows: 0
info: Records: 0 Duplicates: 0 Warnings: 0
ALTER TABLE t1 DROP PRIMARY KEY, ALGORITHM=DEFAULT;
affected rows: 3
info: Records: 3 Duplicates: 0 Warnings: 0
DROP TABLE t1;
affected rows: 0
4 changes: 4 additions & 0 deletions mysql-test/suite/innodb/t/alter_algorithm.test
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ ALTER TABLE t1 ROW_FORMAT=COMPRESSED;
--error $error_code
ALTER TABLE t1 ENGINE=INNODB;

# Irrespective of alter_algorithm value, the following command
# should succeed because of explicitly mentioning ALGORTHM=DEFAULT
ALTER TABLE t1 FORCE, ALGORITHM=DEFAULT;

DROP TABLE t1;
--disable_info

Expand Down
12 changes: 12 additions & 0 deletions mysql-test/suite/innodb/t/alter_algorithm2.test
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,16 @@ call p1();
DROP TABLE t1;
DROP PROCEDURE p1;
DROP PROCEDURE p2;

CREATE TABLE t1(id INT PRIMARY KEY,
col1 INT UNSIGNED NOT NULL UNIQUE)ENGINE=InnoDB;
INSERT INTO t1 VALUES(1,1),(2,2),(3,3);
SET ALTER_ALGORITHM=INSTANT;
--error ER_ALTER_OPERATION_NOT_SUPPORTED
ALTER TABLE t1 DROP COLUMN col1;
--error ER_ALTER_OPERATION_NOT_SUPPORTED
ALTER TABLE t1 DROP COLUMN col1, ALGORITHM=NOCOPY;
ALTER TABLE t1 DROP COLUMN col1, ALGORITHM=DEFAULT;
ALTER TABLE t1 DROP PRIMARY KEY, ALGORITHM=DEFAULT;
DROP TABLE t1;
--disable_info
34 changes: 23 additions & 11 deletions sql/sql_alter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ bool Alter_info::set_requested_algorithm(const LEX_CSTRING *str)
return false;
}

void Alter_info::set_requested_algorithm(enum_alter_table_algorithm algo_val)
{
requested_algorithm= algo_val;
}

bool Alter_info::set_requested_lock(const LEX_CSTRING *str)
{
Expand All @@ -86,13 +90,16 @@ bool Alter_info::set_requested_lock(const LEX_CSTRING *str)
return false;
}

const char* Alter_info::algorithm() const
const char* Alter_info::algorithm_clause(THD *thd) const
{
switch (requested_algorithm) {
switch (algorithm(thd)) {
case ALTER_TABLE_ALGORITHM_INPLACE:
return "ALGORITHM=INPLACE";
case ALTER_TABLE_ALGORITHM_COPY:
return "ALGORITHM=COPY";
case ALTER_TABLE_ALGORITHM_NONE:
DBUG_ASSERT(0);
/* Fall through */
case ALTER_TABLE_ALGORITHM_DEFAULT:
return "ALGORITHM=DEFAULT";
case ALTER_TABLE_ALGORITHM_NOCOPY:
Expand Down Expand Up @@ -123,9 +130,6 @@ const char* Alter_info::lock() const
bool Alter_info::supports_algorithm(THD *thd, enum_alter_inplace_result result,
const Alter_inplace_info *ha_alter_info)
{
if (requested_algorithm == Alter_info::ALTER_TABLE_ALGORITHM_DEFAULT)
requested_algorithm = (Alter_info::enum_alter_table_algorithm) thd->variables.alter_algorithm;

switch (result) {
case HA_ALTER_INPLACE_EXCLUSIVE_LOCK:
case HA_ALTER_INPLACE_SHARED_LOCK:
Expand All @@ -134,26 +138,26 @@ bool Alter_info::supports_algorithm(THD *thd, enum_alter_inplace_result result,
return false;
case HA_ALTER_INPLACE_COPY_NO_LOCK:
case HA_ALTER_INPLACE_COPY_LOCK:
if (requested_algorithm >= Alter_info::ALTER_TABLE_ALGORITHM_NOCOPY)
if (algorithm(thd) >= Alter_info::ALTER_TABLE_ALGORITHM_NOCOPY)
{
ha_alter_info->report_unsupported_error(algorithm(),
ha_alter_info->report_unsupported_error(algorithm_clause(thd),
"ALGORITHM=INPLACE");
return true;
}
return false;
case HA_ALTER_INPLACE_NOCOPY_NO_LOCK:
case HA_ALTER_INPLACE_NOCOPY_LOCK:
if (requested_algorithm == Alter_info::ALTER_TABLE_ALGORITHM_INSTANT)
if (algorithm(thd) == Alter_info::ALTER_TABLE_ALGORITHM_INSTANT)
{
ha_alter_info->report_unsupported_error("ALGORITHM=INSTANT",
"ALGORITHM=NOCOPY");
return true;
}
return false;
case HA_ALTER_INPLACE_NOT_SUPPORTED:
if (requested_algorithm >= Alter_info::ALTER_TABLE_ALGORITHM_INPLACE)
if (algorithm(thd) >= Alter_info::ALTER_TABLE_ALGORITHM_INPLACE)
{
ha_alter_info->report_unsupported_error(algorithm(),
ha_alter_info->report_unsupported_error(algorithm_clause(thd),
"ALGORITHM=COPY");
return true;
}
Expand All @@ -174,7 +178,7 @@ bool Alter_info::supports_lock(THD *thd, enum_alter_inplace_result result,
case HA_ALTER_INPLACE_EXCLUSIVE_LOCK:
// If SHARED lock and no particular algorithm was requested, use COPY.
if (requested_lock == Alter_info::ALTER_TABLE_LOCK_SHARED &&
requested_algorithm == Alter_info::ALTER_TABLE_ALGORITHM_DEFAULT &&
algorithm(thd) == Alter_info::ALTER_TABLE_ALGORITHM_DEFAULT &&
thd->variables.alter_algorithm ==
Alter_info::ALTER_TABLE_ALGORITHM_DEFAULT)
return false;
Expand Down Expand Up @@ -237,6 +241,14 @@ bool Alter_info::vers_prohibited(THD *thd) const
return false;
}

Alter_info::enum_alter_table_algorithm
Alter_info::algorithm(const THD *thd) const
{
if (requested_algorithm == ALTER_TABLE_ALGORITHM_NONE)
return (Alter_info::enum_alter_table_algorithm) thd->variables.alter_algorithm;
return requested_algorithm;
}


Alter_table_ctx::Alter_table_ctx()
: datetime_field(NULL), error_if_not_empty(false),
Expand Down
Loading

0 comments on commit ec9908b

Please sign in to comment.