From d4b9de2b181af367a53bb2e5dd017d15a78b8b6d Mon Sep 17 00:00:00 2001 From: winkyao Date: Tue, 19 Feb 2019 16:05:20 +0800 Subject: [PATCH] address comments --- ddl/db_integration_test.go | 46 +++++++++++++++++++++----------------- ddl/ddl_api.go | 4 ++-- 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/ddl/db_integration_test.go b/ddl/db_integration_test.go index 1b089a161b549..786401ff773b0 100644 --- a/ddl/db_integration_test.go +++ b/ddl/db_integration_test.go @@ -1211,12 +1211,16 @@ func (s *testIntegrationSuite) TestAlterColumn(c *C) { c.Assert(err, NotNil) } -func (s *testIntegrationSuite) assertWarningExec(c *C, sql string) { +func (s *testIntegrationSuite) assertWarningExec(c *C, sql string, expectedWarn *terror.Error) { _, 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)) + c.Assert(expectedWarn.Equal(st.GetWarnings()[0].Err), IsTrue, Commentf("error:%v", err)) +} + +func (s *testIntegrationSuite) assertAlterWarnExec(c *C, sql string) { + s.assertWarningExec(c, sql, ddl.ErrAlterOperationNotSupported) } func (s *testIntegrationSuite) TestAlterAlgorithm(c *C) { @@ -1236,49 +1240,49 @@ func (s *testIntegrationSuite) TestAlterAlgorithm(c *C) { 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.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.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.assertAlterWarnExec(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.assertWarningExec(c, "alter table t drop index idx_b, ALGORITHM=INPLACE") - s.assertWarningExec(c, "alter table t drop index idx_b1, ALGORITHM=COPY") + s.assertAlterWarnExec(c, "alter table t drop index idx_b, 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.assertWarningExec(c, "alter table t rename to t1, ALGORITHM=COPY") - s.assertWarningExec(c, "alter table t1 rename to t, ALGORITHM=INPLACE") + s.assertAlterWarnExec(c, "alter table t rename to t1, ALGORITHM=COPY") + s.assertAlterWarnExec(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.assertAlterWarnExec(c, "alter table t rename index idx_c to idx_c1, ALGORITHM=COPY") + s.assertAlterWarnExec(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.assertAlterWarnExec(c, "alter table t truncate partition p1, ALGORITHM=COPY") + s.assertAlterWarnExec(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.assertAlterWarnExec(c, "alter table t add partition (partition p4 values less than (2002)), ALGORITHM=COPY") + 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.assertWarningExec(c, "alter table t drop partition p4, ALGORITHM=COPY") - s.assertWarningExec(c, "alter table t drop partition p5, ALGORITHM=INPLACE") + s.assertAlterWarnExec(c, "alter table t drop partition p4, ALGORITHM=COPY") + s.assertAlterWarnExec(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.assertAlterWarnExec(c, "alter table t comment = 'test', ALGORITHM=COPY") + s.assertAlterWarnExec(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.assertAlterWarnExec(c, "alter table t default charset = utf8mb4, ALGORITHM=COPY") + s.assertAlterWarnExec(c, "alter table t default charset = utf8mb4, ALGORITHM=INPLACE") s.tk.MustExec("alter table t default charset = utf8mb4, ALGORITHM=INSTANT") } diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index eb64bbea089f5..7e31ba95b8966 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -1424,14 +1424,14 @@ func getCharsetAndCollateInTableOption(startIdx int, options []*ast.TableOption) return } -// resolveAlterTableSpec resolve alter table algorithm and remove ignore table spec in specs. +// resolveAlterTableSpec resolves alter table algorithm and removes ignore table spec in specs. // returns valied specs, and the occured error. 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. + // Find the last AlterTableAlgorithm. algorithm = spec.Algorithm } if isIgnorableSpec(spec.Tp) {