Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ddl: improve compatibility for ALTER TABLE algorithms (#19270) #19364

Merged
merged 2 commits into from
Aug 21, 2020
Merged

ddl: improve compatibility for ALTER TABLE algorithms (#19270) #19364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5d454a1
cherry pick #19270 to release-4.0
xhebox Aug 21, 2020
cfc5055
fix conflict
bb7133 Aug 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 14 additions & 13 deletions ddl/db_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1252,14 +1252,14 @@ func (s *testIntegrationSuite3) TestChangeColumnPosition(c *C) {
s.tk.MustExec("alter table position4 add index t(b)")
s.tk.MustExec("alter table position4 change column b c int first")
createSQL := s.tk.MustQuery("show create table position4").Rows()[0][1]
exceptedSQL := []string{
expectedSQL := []string{
"CREATE TABLE `position4` (",
" `c` int(11) DEFAULT NULL,",
" `a` int(11) DEFAULT NULL,",
" KEY `t` (`c`)",
") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin",
}
c.Assert(createSQL, Equals, strings.Join(exceptedSQL, "\n"))
c.Assert(createSQL, Equals, strings.Join(expectedSQL, "\n"))
}

func (s *testIntegrationSuite2) TestAddIndexAfterAddColumn(c *C) {
Expand Down Expand Up @@ -1477,51 +1477,52 @@ func (s *testIntegrationSuite3) TestAlterAlgorithm(c *C) {
PARTITION p2 VALUES LESS THAN (16),
PARTITION p3 VALUES LESS THAN (21)
)`)
s.assertAlterErrorExec(c, "alter table t modify column a bigint, ALGORITHM=INPLACE;")
s.assertAlterWarnExec(c, "alter table t modify column a bigint, ALGORITHM=INPLACE;")
s.tk.MustExec("alter table t modify column a bigint, ALGORITHM=INPLACE, ALGORITHM=INSTANT;")
s.tk.MustExec("alter table t modify column a bigint, ALGORITHM=DEFAULT;")

// Test add/drop index
s.assertAlterErrorExec(c, "alter table t add index idx_b(b), ALGORITHM=INSTANT")
s.assertAlterWarnExec(c, "alter table t add index idx_b1(b), ALGORITHM=COPY")
s.tk.MustExec("alter table t add index idx_b2(b), ALGORITHM=INPLACE")
s.assertAlterErrorExec(c, "alter table t drop index idx_b, ALGORITHM=INPLACE")
s.tk.MustExec("alter table t add index idx_b3(b), ALGORITHM=DEFAULT")
s.assertAlterWarnExec(c, "alter table t drop index idx_b3, ALGORITHM=INPLACE")
s.assertAlterWarnExec(c, "alter table t drop index idx_b1, ALGORITHM=COPY")
s.tk.MustExec("alter table t drop index idx_b2, ALGORITHM=INSTANT")

// Test rename
s.assertAlterWarnExec(c, "alter table t rename to t1, ALGORITHM=COPY")
s.assertAlterErrorExec(c, "alter table t1 rename to t, ALGORITHM=INPLACE")
s.tk.MustExec("alter table t1 rename to t, ALGORITHM=INSTANT")
s.assertAlterWarnExec(c, "alter table t1 rename to t2, ALGORITHM=INPLACE")
s.tk.MustExec("alter table t2 rename to t, ALGORITHM=INSTANT")
s.tk.MustExec("alter table t rename to t1, ALGORITHM=DEFAULT")
s.tk.MustExec("alter table t1 rename to t")

// Test rename index
s.assertAlterWarnExec(c, "alter table t rename index idx_c to idx_c1, ALGORITHM=COPY")
s.assertAlterErrorExec(c, "alter table t rename index idx_c1 to idx_c, ALGORITHM=INPLACE")
s.tk.MustExec("alter table t rename index idx_c1 to idx_c, ALGORITHM=INSTANT")
s.assertAlterWarnExec(c, "alter table t rename index idx_c1 to idx_c2, ALGORITHM=INPLACE")
s.tk.MustExec("alter table t rename index idx_c2 to idx_c, ALGORITHM=INSTANT")
s.tk.MustExec("alter table t rename index idx_c to idx_c1, ALGORITHM=DEFAULT")

// partition.
s.assertAlterWarnExec(c, "alter table t ALGORITHM=COPY, truncate partition p1")
s.assertAlterErrorExec(c, "alter table t ALGORITHM=INPLACE, truncate partition p2")
s.assertAlterWarnExec(c, "alter table t ALGORITHM=INPLACE, truncate partition p2")
s.tk.MustExec("alter table t ALGORITHM=INSTANT, truncate partition p3")

s.assertAlterWarnExec(c, "alter table t add partition (partition p4 values less than (2002)), ALGORITHM=COPY")
s.assertAlterErrorExec(c, "alter table t add partition (partition p5 values less than (3002)), ALGORITHM=INPLACE")
s.assertAlterWarnExec(c, "alter table t add partition (partition p5 values less than (3002)), ALGORITHM=INPLACE")
s.tk.MustExec("alter table t add partition (partition p6 values less than (4002)), ALGORITHM=INSTANT")

s.assertAlterWarnExec(c, "alter table t ALGORITHM=COPY, drop partition p4")
s.assertAlterErrorExec(c, "alter table t ALGORITHM=INPLACE, drop partition p5")
s.assertAlterWarnExec(c, "alter table t ALGORITHM=INPLACE, drop partition p5")
s.tk.MustExec("alter table t ALGORITHM=INSTANT, drop partition p6")

// Table options
s.assertAlterWarnExec(c, "alter table t comment = 'test', ALGORITHM=COPY")
s.assertAlterErrorExec(c, "alter table t comment = 'test', ALGORITHM=INPLACE")
s.assertAlterWarnExec(c, "alter table t comment = 'test', ALGORITHM=INPLACE")
s.tk.MustExec("alter table t comment = 'test', ALGORITHM=INSTANT")

s.assertAlterWarnExec(c, "alter table t default charset = utf8mb4, ALGORITHM=COPY")
s.assertAlterErrorExec(c, "alter table t default charset = utf8mb4, ALGORITHM=INPLACE")
s.assertAlterWarnExec(c, "alter table t default charset = utf8mb4, ALGORITHM=INPLACE")
s.tk.MustExec("alter table t default charset = utf8mb4, ALGORITHM=INSTANT")
}

Expand Down
18 changes: 14 additions & 4 deletions ddl/ddl_algorithm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
// because we never block the DML but costs some time to backfill the index data)
// See https://dev.mysql.com/doc/refman/8.0/en/alter-table.html#alter-table-performance.
type AlterAlgorithm struct {
// supported MUST store algorithms in the order 'INSTANT, INPLACE, COPY'
supported []ast.AlgorithmType
// If the alter algorithm is not given, the defAlgorithm will be used.
defAlgorithm ast.AlgorithmType
Expand All @@ -47,18 +48,27 @@ func getProperAlgorithm(specify ast.AlgorithmType, algorithm *AlterAlgorithm) (a
return algorithm.defAlgorithm, nil
}

r := ast.AlgorithmTypeDefault

for _, a := range algorithm.supported {
if specify == a {
return specify, nil
if specify <= a {
r = a
break
}
}

return algorithm.defAlgorithm, ErrAlterOperationNotSupported.GenWithStackByArgs(fmt.Sprintf("ALGORITHM=%s", specify), fmt.Sprintf("Cannot alter table by %s", specify), fmt.Sprintf("ALGORITHM=%s", algorithm.defAlgorithm))
var err error
if specify != r {
err = ErrAlterOperationNotSupported.GenWithStackByArgs(fmt.Sprintf("ALGORITHM=%s", specify), fmt.Sprintf("Cannot alter table by %s", specify), fmt.Sprintf("ALGORITHM=%s", algorithm.defAlgorithm))
}
return r, err
}

// ResolveAlterAlgorithm resolves the algorithm of the alterSpec.
// If specify algorithm is not supported by the alter action, errAlterOperationNotSupported will be returned.
// If specify is the ast.AlterAlgorithmDefault, then the default algorithm of the alter action will be returned.
// If specify algorithm is not supported by the alter action, it will try to find a better algorithm in the order `INSTANT > INPLACE > COPY`, errAlterOperationNotSupported will be returned.
// E.g. INSTANT may be returned if specify=INPLACE
// If failed to choose any valid algorithm, AlgorithmTypeDefault and errAlterOperationNotSupported will be returned
func ResolveAlterAlgorithm(alterSpec *ast.AlterTableSpec, specify ast.AlgorithmType) (ast.AlgorithmType, error) {
switch alterSpec.Tp {
// For now, TiDB only support inplace algorithm and instant algorithm.
Expand Down
72 changes: 40 additions & 32 deletions ddl/ddl_algorithm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,42 +31,48 @@ type testDDLAlgorithmSuite struct{}
type testCase struct {
alterSpec ast.AlterTableSpec
supportedAlgorithm []ast.AlgorithmType
defAlgorithm ast.AlgorithmType
expectedAlgorithm []ast.AlgorithmType
}

func (s *testDDLAlgorithmSuite) TestFindAlterAlgorithm(c *C) {
instantAlgorithm := []ast.AlgorithmType{ast.AlgorithmTypeInstant}
inplaceAlgorithm := []ast.AlgorithmType{ast.AlgorithmTypeInplace}
supportedInstantAlgorithms := []ast.AlgorithmType{ast.AlgorithmTypeDefault, ast.AlgorithmTypeCopy, ast.AlgorithmTypeInplace, ast.AlgorithmTypeInstant}
expectedInstantAlgorithms := []ast.AlgorithmType{ast.AlgorithmTypeInstant, ast.AlgorithmTypeInstant, ast.AlgorithmTypeInstant, ast.AlgorithmTypeInstant}

testCases := []testCase{
{ast.AlterTableSpec{Tp: ast.AlterTableAddConstraint}, inplaceAlgorithm, ast.AlgorithmTypeInplace},
{ast.AlterTableSpec{Tp: ast.AlterTableAddColumns}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableDropColumn}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableDropPrimaryKey}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableDropIndex}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableDropForeignKey}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableRenameTable}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableRenameIndex}, instantAlgorithm, ast.AlgorithmTypeInstant},
{
ast.AlterTableSpec{Tp: ast.AlterTableAddConstraint},
[]ast.AlgorithmType{ast.AlgorithmTypeDefault, ast.AlgorithmTypeCopy, ast.AlgorithmTypeInplace},
[]ast.AlgorithmType{ast.AlgorithmTypeInplace, ast.AlgorithmTypeInplace, ast.AlgorithmTypeInplace},
},

{ast.AlterTableSpec{Tp: ast.AlterTableAddColumns}, supportedInstantAlgorithms, expectedInstantAlgorithms},
{ast.AlterTableSpec{Tp: ast.AlterTableDropColumn}, supportedInstantAlgorithms, expectedInstantAlgorithms},
{ast.AlterTableSpec{Tp: ast.AlterTableDropPrimaryKey}, supportedInstantAlgorithms, expectedInstantAlgorithms},
{ast.AlterTableSpec{Tp: ast.AlterTableDropIndex}, supportedInstantAlgorithms, expectedInstantAlgorithms},
{ast.AlterTableSpec{Tp: ast.AlterTableDropForeignKey}, supportedInstantAlgorithms, expectedInstantAlgorithms},
{ast.AlterTableSpec{Tp: ast.AlterTableRenameTable}, supportedInstantAlgorithms, expectedInstantAlgorithms},
{ast.AlterTableSpec{Tp: ast.AlterTableRenameIndex}, supportedInstantAlgorithms, expectedInstantAlgorithms},

// Alter table options.
{ast.AlterTableSpec{Tp: ast.AlterTableOption, Options: []*ast.TableOption{{Tp: ast.TableOptionShardRowID}}}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableOption, Options: []*ast.TableOption{{Tp: ast.TableOptionAutoIncrement}}}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableOption, Options: []*ast.TableOption{{Tp: ast.TableOptionComment}}}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableOption, Options: []*ast.TableOption{{Tp: ast.TableOptionCharset}}}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableOption, Options: []*ast.TableOption{{Tp: ast.TableOptionCollate}}}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableOption, Options: []*ast.TableOption{{Tp: ast.TableOptionShardRowID}}}, supportedInstantAlgorithms, expectedInstantAlgorithms},
{ast.AlterTableSpec{Tp: ast.AlterTableOption, Options: []*ast.TableOption{{Tp: ast.TableOptionAutoIncrement}}}, supportedInstantAlgorithms, expectedInstantAlgorithms},
{ast.AlterTableSpec{Tp: ast.AlterTableOption, Options: []*ast.TableOption{{Tp: ast.TableOptionComment}}}, supportedInstantAlgorithms, expectedInstantAlgorithms},
{ast.AlterTableSpec{Tp: ast.AlterTableOption, Options: []*ast.TableOption{{Tp: ast.TableOptionCharset}}}, supportedInstantAlgorithms, expectedInstantAlgorithms},
{ast.AlterTableSpec{Tp: ast.AlterTableOption, Options: []*ast.TableOption{{Tp: ast.TableOptionCollate}}}, supportedInstantAlgorithms, expectedInstantAlgorithms},

// TODO: after we support migrate the data of partitions, change below cases.
{ast.AlterTableSpec{Tp: ast.AlterTableCoalescePartitions}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableAddPartitions}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableDropPartition}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableTruncatePartition}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableCoalescePartitions}, supportedInstantAlgorithms, expectedInstantAlgorithms},
{ast.AlterTableSpec{Tp: ast.AlterTableAddPartitions}, supportedInstantAlgorithms, expectedInstantAlgorithms},
{ast.AlterTableSpec{Tp: ast.AlterTableDropPartition}, supportedInstantAlgorithms, expectedInstantAlgorithms},
{ast.AlterTableSpec{Tp: ast.AlterTableTruncatePartition}, supportedInstantAlgorithms, expectedInstantAlgorithms},
{ast.AlterTableSpec{Tp: ast.AlterTableExchangePartition}, supportedInstantAlgorithms, expectedInstantAlgorithms},

// TODO: after we support lock a table, change the below case.
{ast.AlterTableSpec{Tp: ast.AlterTableLock}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableLock}, supportedInstantAlgorithms, expectedInstantAlgorithms},
// TODO: after we support changing the column type, below cases need to change.
{ast.AlterTableSpec{Tp: ast.AlterTableModifyColumn}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableChangeColumn}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableAlterColumn}, instantAlgorithm, ast.AlgorithmTypeInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableModifyColumn}, supportedInstantAlgorithms, expectedInstantAlgorithms},
{ast.AlterTableSpec{Tp: ast.AlterTableChangeColumn}, supportedInstantAlgorithms, expectedInstantAlgorithms},
{ast.AlterTableSpec{Tp: ast.AlterTableAlterColumn}, supportedInstantAlgorithms, expectedInstantAlgorithms},
}

for _, tc := range testCases {
Expand All @@ -75,10 +81,6 @@ func (s *testDDLAlgorithmSuite) TestFindAlterAlgorithm(c *C) {
}

func runAlterAlgorithmTestCases(c *C, tc *testCase) {
algorithm, err := ddl.ResolveAlterAlgorithm(&tc.alterSpec, ast.AlgorithmTypeDefault)
c.Assert(err, IsNil)
c.Assert(algorithm, Equals, tc.defAlgorithm)

unsupported := make([]ast.AlgorithmType, 0, len(allAlgorithm))
Loop:
for _, alm := range allAlgorithm {
Expand All @@ -91,16 +93,22 @@ Loop:
unsupported = append(unsupported, alm)
}

var algorithm ast.AlgorithmType
var err error

// Test supported.
for _, alm := range tc.supportedAlgorithm {
for i, alm := range tc.supportedAlgorithm {
algorithm, err = ddl.ResolveAlterAlgorithm(&tc.alterSpec, alm)
c.Assert(err, IsNil)
c.Assert(algorithm, Equals, alm)
if err != nil {
c.Assert(ddl.ErrAlterOperationNotSupported.Equal(err), IsTrue)
}
c.Assert(algorithm, Equals, tc.expectedAlgorithm[i])
}

// Test unsupported.
for _, alm := range unsupported {
_, err = ddl.ResolveAlterAlgorithm(&tc.alterSpec, alm)
algorithm, err = ddl.ResolveAlterAlgorithm(&tc.alterSpec, alm)
c.Assert(algorithm, Equals, ast.AlgorithmTypeDefault)
c.Assert(err, NotNil, Commentf("Tp:%v, alm:%s", tc.alterSpec.Tp, alm))
c.Assert(ddl.ErrAlterOperationNotSupported.Equal(err), IsTrue)
}
Expand Down
6 changes: 3 additions & 3 deletions ddl/ddl_api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2053,11 +2053,11 @@ func resolveAlterTableSpec(ctx sessionctx.Context, specs []*ast.AlterTableSpec)
for _, spec := range validSpecs {
resolvedAlgorithm, err := ResolveAlterAlgorithm(spec, algorithm)
if err != nil {
if algorithm != ast.AlgorithmTypeCopy {
// If TiDB failed to choose a better algorithm, report the error
if resolvedAlgorithm == ast.AlgorithmTypeDefault {
return nil, errors.Trace(err)
}
// For the compatibility, we return warning instead of error when the algorithm is COPY,
// because the COPY ALGORITHM is not supported in TiDB.
// For the compatibility, we return warning instead of error when a better algorithm is chosed by TiDB
ctx.GetSessionVars().StmtCtx.AppendError(err)
}

Expand Down
5 changes: 3 additions & 2 deletions executor/seqtest/seq_executor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ func (s *seqTestSuite) TestShow(c *C) {
d int UNIQUE KEY,
index (b) invisible,
index (d) invisible)`)
excepted :=
expected :=
"t CREATE TABLE `t` (\n" +
" `a` int(11) DEFAULT NULL,\n" +
" `b` int(11) DEFAULT NULL,\n" +
Expand All @@ -298,7 +298,8 @@ func (s *seqTestSuite) TestShow(c *C) {
" UNIQUE KEY `c` (`c`),\n" +
" UNIQUE KEY `d_2` (`d`)\n" +
") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin"
tk.MustQuery("show create table t").Check(testkit.Rows(excepted))
tk.MustQuery("show create table t").Check(testkit.Rows(expected))
tk.MustExec("drop table t")

testSQL = "SHOW VARIABLES LIKE 'character_set_results';"
result = tk.MustQuery(testSQL)
Expand Down