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.

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

ddl: support alter algorithm INPLACE/INSTANT #8811

Merged
merged 17 commits into from
Feb 20, 2019
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
72 changes: 72 additions & 0 deletions ddl/db_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1141,3 +1141,75 @@ func (s *testIntegrationSuite) TestAlterColumn(c *C) {
_, err = s.tk.Exec("alter table mc modify column a bigint auto_increment") // Adds auto_increment should throw error
c.Assert(err, NotNil)
}

func (s *testIntegrationSuite) assertWarningExec(c *C, sql string) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding the warning message as an argument can be beneficial for calls to other functions.

_, err := s.tk.Exec(sql)
c.Assert(err, IsNil)
st := s.tk.Se.GetSessionVars().StmtCtx
c.Assert(st.WarningCount(), Equals, uint16(1))
c.Assert(ddl.ErrAlterOperationNotSupported.Equal(st.GetWarnings()[0].Err), IsTrue, Commentf("error:%v", err))
}

func (s *testIntegrationSuite) TestAlterAlgorithm(c *C) {
s.tk = testkit.NewTestKit(c, s.store)
s.tk.MustExec("use test")
s.tk.MustExec("drop table if exists t")
s.tk.MustExec("drop table if exists t1")
defer s.tk.MustExec("drop table if exists t")

s.tk.MustExec(`create table t(
a int,
b varchar(100),
c int,
INDEX idx_c(c)) PARTITION BY RANGE ( a ) (
PARTITION p0 VALUES LESS THAN (6),
PARTITION p1 VALUES LESS THAN (11),
PARTITION p2 VALUES LESS THAN (16),
PARTITION p3 VALUES LESS THAN (21)
)`)
s.assertWarningExec(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.assertWarningExec(c, "alter table t add index idx_b(b), ALGORITHM=INSTANT")
s.assertWarningExec(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.assertWarningExec(c, "alter table t drop index idx_b, ALGORITHM=INPLACE")
s.assertWarningExec(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.assertWarningExec(c, "alter table t rename to t1, ALGORITHM=COPY")
s.assertWarningExec(c, "alter table t1 rename to t, ALGORITHM=INPLACE")
s.tk.MustExec("alter table t rename to t1, ALGORITHM=INSTANT")
s.tk.MustExec("alter table t1 rename to t, ALGORITHM=DEFAULT")

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

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

s.assertWarningExec(c, "alter table t add partition (partition p4 values less than (2002)), ALGORITHM=COPY")
s.assertWarningExec(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.assertWarningExec(c, "alter table t drop partition p4, ALGORITHM=COPY")
s.assertWarningExec(c, "alter table t drop partition p5, ALGORITHM=INPLACE")
s.tk.MustExec("alter table t drop partition p6, ALGORITHM=INSTANT")

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

s.assertWarningExec(c, "alter table t default charset = utf8mb4, ALGORITHM=COPY")
s.assertWarningExec(c, "alter table t default charset = utf8mb4, ALGORITHM=INPLACE")
s.tk.MustExec("alter table t default charset = utf8mb4, ALGORITHM=INSTANT")
}
4 changes: 4 additions & 0 deletions ddl/ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ var (
ErrCoalesceOnlyOnHashPartition = terror.ClassDDL.New(codeCoalesceOnlyOnHashPartition, mysql.MySQLErrName[mysql.ErrCoalesceOnlyOnHashPartition])
// ErrViewWrongList returns create view must include all columns in the select clause
ErrViewWrongList = terror.ClassDDL.New(codeViewWrongList, mysql.MySQLErrName[mysql.ErrViewWrongList])
// ErrAlterOperationNotSupported returns when alter operations is not supported.
ErrAlterOperationNotSupported = terror.ClassDDL.New(codeNotSupportedAlterOperation, mysql.MySQLErrName[mysql.ErrAlterOperationNotSupportedReason])
// ErrTableIsNotView returns for table is not base table.
ErrTableIsNotView = terror.ClassDDL.New(codeErrWrongObject, "'%s.%s' is not VIEW")
)
Expand Down Expand Up @@ -680,6 +682,7 @@ const (
codeWarnDataTruncated = terror.ErrCode(mysql.WarnDataTruncated)
codeCoalesceOnlyOnHashPartition = terror.ErrCode(mysql.ErrCoalesceOnlyOnHashPartition)
codeUnknownPartition = terror.ErrCode(mysql.ErrUnknownPartition)
codeNotSupportedAlterOperation = terror.ErrCode(mysql.ErrAlterOperationNotSupportedReason)
)

func init() {
Expand Down Expand Up @@ -731,6 +734,7 @@ func init() {
codeWarnDataTruncated: mysql.WarnDataTruncated,
codeCoalesceOnlyOnHashPartition: mysql.ErrCoalesceOnlyOnHashPartition,
codeUnknownPartition: mysql.ErrUnknownPartition,
codeNotSupportedAlterOperation: mysql.ErrAlterOperationNotSupportedReason,
codeErrWrongObject: mysql.ErrWrongObject,
}
terror.ErrClassToMySQLCodes[terror.ClassDDL] = ddlMySQLErrCodes
Expand Down
72 changes: 72 additions & 0 deletions ddl/ddl_algorithm.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// Copyright 2018 PingCAP, Inc.
crazycs520 marked this conversation as resolved.
Show resolved Hide resolved
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// See the License for the specific language governing permissions and
// limitations under the License.

package ddl

import (
"fmt"

"github.com/pingcap/parser/ast"
)

// AlterAlgorithm is used to store supported alter algorithm.
// For now, TiDB only support AlterAlgorithmInplace and AlterAlgorithmInstant.
// The most alter operations are using instant algorithm, and only the add index is using inplace(not really inplace,
// 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 []ast.AlterAlgorithm
// If the alter algorithm is not given, the defAlgorithm will be used.
defAlgorithm ast.AlterAlgorithm
}

var (
instantAlgorithm = &AlterAlgorithm{
supported: []ast.AlterAlgorithm{ast.AlterAlgorithmInstant},
defAlgorithm: ast.AlterAlgorithmInstant,
}

inplaceAlgorithm = &AlterAlgorithm{
supported: []ast.AlterAlgorithm{ast.AlterAlgorithmInplace},
defAlgorithm: ast.AlterAlgorithmInplace,
}

defaultAlgorithm = ast.AlterAlgorithmInstant
)

func getProperAlgorithm(specify ast.AlterAlgorithm, algorithm *AlterAlgorithm) (ast.AlterAlgorithm, error) {
if specify == ast.AlterAlgorithmDefault {
return algorithm.defAlgorithm, nil
}

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

return algorithm.defAlgorithm, ErrAlterOperationNotSupported.GenWithStackByArgs(fmt.Sprintf("ALGORITHM=%s", specify), fmt.Sprintf("Cannot alter table by %s", specify), fmt.Sprintf("ALGORITHM=%s", algorithm.defAlgorithm))
}

// 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.
func ResolveAlterAlgorithm(alterSpec *ast.AlterTableSpec, specify ast.AlterAlgorithm) (ast.AlterAlgorithm, error) {
switch alterSpec.Tp {
// For now, TiDB only support inplace algorithm and instant algorithm.
case ast.AlterTableAddConstraint:
return getProperAlgorithm(specify, inplaceAlgorithm)
default:
return getProperAlgorithm(specify, instantAlgorithm)
}
}
107 changes: 107 additions & 0 deletions ddl/ddl_algorithm_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
// Copyright 2018 PingCAP, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// See the License for the specific language governing permissions and
// limitations under the License.

package ddl_test

import (
. "github.com/pingcap/check"
"github.com/pingcap/parser/ast"
"github.com/pingcap/tidb/ddl"
)

var _ = Suite(&testDDLAlgorithmSuite{})

var (
allAlgorithm = []ast.AlterAlgorithm{ast.AlterAlgorithmCopy,
ast.AlterAlgorithmInplace, ast.AlterAlgorithmInstant}
)

type testDDLAlgorithmSuite struct{}

type testCase struct {
alterSpec ast.AlterTableSpec
supportedAlgorithm []ast.AlterAlgorithm
defAlgorithm ast.AlterAlgorithm
}

func (s *testDDLAlgorithmSuite) TestFindAlterAlgorithm(c *C) {
instantAlgorithm := []ast.AlterAlgorithm{ast.AlterAlgorithmInstant}
inplaceAlgorithm := []ast.AlterAlgorithm{ast.AlterAlgorithmInplace}

testCases := []testCase{
{ast.AlterTableSpec{Tp: ast.AlterTableAddConstraint}, inplaceAlgorithm, ast.AlterAlgorithmInplace},
{ast.AlterTableSpec{Tp: ast.AlterTableAddColumns}, instantAlgorithm, ast.AlterAlgorithmInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableDropColumn}, instantAlgorithm, ast.AlterAlgorithmInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableDropPrimaryKey}, instantAlgorithm, ast.AlterAlgorithmInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableDropIndex}, instantAlgorithm, ast.AlterAlgorithmInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableDropForeignKey}, instantAlgorithm, ast.AlterAlgorithmInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableRenameTable}, instantAlgorithm, ast.AlterAlgorithmInstant},
{ast.AlterTableSpec{Tp: ast.AlterTableRenameIndex}, instantAlgorithm, ast.AlterAlgorithmInstant},

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

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

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

for _, tc := range testCases {
runAlterAlgorithmTestCases(c, &tc)
}
}

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

unsupported := make([]ast.AlterAlgorithm, 0, len(allAlgorithm))
Loop:
for _, alm := range allAlgorithm {
for _, almSupport := range tc.supportedAlgorithm {
if alm == almSupport {
continue Loop
}
}

unsupported = append(unsupported, alm)
}

// Test supported.
for _, alm := range tc.supportedAlgorithm {
algorithm, err = ddl.ResolveAlterAlgorithm(&tc.alterSpec, alm)
c.Assert(err, IsNil)
c.Assert(algorithm, Equals, alm)
}

// Test unsupported.
for _, alm := range unsupported {
algorithm, err = ddl.ResolveAlterAlgorithm(&tc.alterSpec, alm)
c.Assert(err, NotNil, Commentf("Tp:%v, alm:%s", tc.alterSpec.Tp, alm))
c.Assert(ddl.ErrAlterOperationNotSupported.Equal(err), IsTrue)
}
}
39 changes: 36 additions & 3 deletions ddl/ddl_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@ import (
"github.com/pingcap/tidb/util/mock"
"github.com/pingcap/tidb/util/schemautil"
"github.com/pingcap/tidb/util/set"
<<<<<<< HEAD
"github.com/sirupsen/logrus"
=======
log "github.com/sirupsen/logrus"
>>>>>>> upstream/master
crazycs520 marked this conversation as resolved.
Show resolved Hide resolved
)

func (d *ddl) CreateSchema(ctx sessionctx.Context, schema model.CIStr, charsetInfo *ast.CharsetOpt) (err error) {
Expand Down Expand Up @@ -1253,10 +1257,16 @@ func getCharsetAndCollateInTableOption(startIdx int, options []*ast.TableOption)
return
}

func (d *ddl) AlterTable(ctx sessionctx.Context, ident ast.Ident, specs []*ast.AlterTableSpec) (err error) {
// Only handle valid specs.
// resolveAlterTableSpec resolve alter table algorithm and remove ignore table spec in specs.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/resolve/resolves
s/remove/removes

// returns valied specs, and the occured error.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/returns/Return

func resolveAlterTableSpec(ctx sessionctx.Context, specs []*ast.AlterTableSpec) ([]*ast.AlterTableSpec, error) {
validSpecs := make([]*ast.AlterTableSpec, 0, len(specs))
algorithm := ast.AlterAlgorithmDefault
for _, spec := range specs {
if spec.Tp == ast.AlterTableAlgorithm {
// find the last AlterTableAlgorithm.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/find/Find

algorithm = spec.Algorithm
}
if isIgnorableSpec(spec.Tp) {
continue
}
Expand All @@ -1266,7 +1276,30 @@ func (d *ddl) AlterTable(ctx sessionctx.Context, ident ast.Ident, specs []*ast.A
if len(validSpecs) != 1 {
// TODO: Hanlde len(validSpecs) == 0.
// Now we only allow one schema changing at the same time.
return errRunMultiSchemaChanges
return nil, errRunMultiSchemaChanges
}

// Verify whether the algorithm is supported.
for _, spec := range validSpecs {
algorithm, err := ResolveAlterAlgorithm(spec, algorithm)
if err != nil {
logrus.Infof("err:%v", err)
// For the compatibility, we return warning instead of error.
ctx.GetSessionVars().StmtCtx.AppendError(err)
err = nil
}

spec.Algorithm = algorithm
}

// Only handle valid specs.
return validSpecs, nil
}

func (d *ddl) AlterTable(ctx sessionctx.Context, ident ast.Ident, specs []*ast.AlterTableSpec) (err error) {
validSpecs, err := resolveAlterTableSpec(ctx, specs)
if err != nil {
return errors.Trace(err)
}

for _, spec := range validSpecs {
Expand Down
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,5 @@ require (
sourcegraph.com/sourcegraph/appdash v0.0.0-20180531100431-4c381bd170b4
sourcegraph.com/sourcegraph/appdash-data v0.0.0-20151005221446-73f23eafcf67
)

replace github.com/pingcap/parser => github.com/winkyao/parser v0.0.0-20181221090435-0eaad9528419
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove this line?

Copy link
Contributor Author

@winkyao winkyao Dec 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line will be removed after the pingcap/parser#93 is merged, but now we need to replace the parser to pass the ci.

1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ github.com/ugorji/go/codec v0.0.0-20181127175209-856da096dbdf h1:BLcwkDfQ8QPXNXB
github.com/ugorji/go/codec v0.0.0-20181127175209-856da096dbdf/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0=
github.com/unrolled/render v0.0.0-20180914162206-b9786414de4d h1:ggUgChAeyge4NZ4QUw6lhHsVymzwSDJOZcE0s2X8S20=
github.com/unrolled/render v0.0.0-20180914162206-b9786414de4d/go.mod h1:tu82oB5W2ykJRVioYsB+IQKcft7ryBr7w12qMBUPyXg=
github.com/winkyao/parser v0.0.0-20181221090435-0eaad9528419/go.mod h1:R7TohkGdsluhClCn0ZTwhHDjjjqoiBr6DFFAPTaFvUI=
github.com/xiang90/probing v0.0.0-20160813154853-07dd2e8dfe18 h1:MPPkRncZLN9Kh4MEFmbnK4h3BD7AUmskWv2+EeZJCCs=
github.com/xiang90/probing v0.0.0-20160813154853-07dd2e8dfe18/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
go.uber.org/atomic v1.3.2 h1:2Oa65PReHzfn29GpvgsYwloV9AVFHPDk8tYxt2c2tr4=
Expand Down