diff --git a/ddl/column_test.go b/ddl/column_test.go index c10696b1806cf..04368fd12c136 100644 --- a/ddl/column_test.go +++ b/ddl/column_test.go @@ -1193,8 +1193,8 @@ func (s *testColumnSuite) TestModifyColumn(c *C) { {"decimal(2,1)", "bigint", nil}, } for _, tt := range tests { - ftA := s.colDefStrToFieldType(c, tt.origin) - ftB := s.colDefStrToFieldType(c, tt.to) + ftA := s.colDefStrToFieldType(c, tt.origin, ctx) + ftB := s.colDefStrToFieldType(c, tt.to, ctx) err := checkModifyTypes(ctx, ftA, ftB, false) if err == nil { c.Assert(tt.err, IsNil, Commentf("origin:%v, to:%v", tt.origin, tt.to)) @@ -1204,13 +1204,13 @@ func (s *testColumnSuite) TestModifyColumn(c *C) { } } -func (s *testColumnSuite) colDefStrToFieldType(c *C, str string) *types.FieldType { +func (s *testColumnSuite) colDefStrToFieldType(c *C, str string, ctx sessionctx.Context) *types.FieldType { sqlA := "alter table t modify column a " + str stmt, err := parser.New().ParseOneStmt(sqlA, "", "") c.Assert(err, IsNil) colDef := stmt.(*ast.AlterTableStmt).Specs[0].NewColumns[0] chs, coll := charset.GetDefaultCharsetAndCollate() - col, _, err := buildColumnAndConstraint(nil, 0, colDef, nil, chs, coll) + col, _, err := buildColumnAndConstraint(ctx, 0, colDef, nil, chs, coll) c.Assert(err, IsNil) return &col.FieldType } diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 9e7c044ed2855..7d8995d620e80 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -587,6 +587,22 @@ func buildColumnAndConstraint( if err := setCharsetCollationFlenDecimal(colDef.Tp, chs, coll); err != nil { return nil, nil, errors.Trace(err) } + // Some column checks are left here, so varchar auto convert is performed. See also issue 30328 + sessVars := ctx.GetSessionVars() + if sessVars != nil && !sessVars.SQLMode.HasStrictMode() && colDef.Tp.Tp == mysql.TypeVarchar { + err := IsTooBigFieldLength(colDef.Tp.Flen, colDef.Name.Name.O, colDef.Tp.Charset) + if err != nil && terror.ErrorEqual(types.ErrTooBigFieldLength, err) { + colDef.Tp.Tp = mysql.TypeBlob + if err = adjustBlobTypesFlen(colDef.Tp, colDef.Tp.Charset); err != nil { + return nil, nil, err + } + if colDef.Tp.Charset == charset.CharsetBin { + sessVars.StmtCtx.AppendWarning(ErrAutoConvert.GenWithStackByArgs(colDef.Name.Name.O, "VARBINARY", "BLOB")) + } else { + sessVars.StmtCtx.AppendWarning(ErrAutoConvert.GenWithStackByArgs(colDef.Name.Name.O, "VARCHAR", "TEXT")) + } + } + } col, cts, err := columnDefToCol(ctx, offset, colDef, outPriKeyConstraint) if err != nil { return nil, nil, errors.Trace(err) diff --git a/ddl/error.go b/ddl/error.go index f95b5baf0e4a9..0dff742b45715 100644 --- a/ddl/error.go +++ b/ddl/error.go @@ -310,4 +310,7 @@ var ( errDependentByFunctionalIndex = dbterror.ClassDDL.NewStd(mysql.ErrDependentByFunctionalIndex) // errFunctionalIndexOnBlob when the expression of expression index returns blob or text. errFunctionalIndexOnBlob = dbterror.ClassDDL.NewStd(mysql.ErrFunctionalIndexOnBlob) + + // ErrAutoConvert is returned when auto convert happens + ErrAutoConvert = dbterror.ClassDDL.NewStd(mysql.ErrAutoConvert) ) diff --git a/errors.toml b/errors.toml index 0f27a704f6a89..d52efd55b9d5d 100644 --- a/errors.toml +++ b/errors.toml @@ -361,6 +361,11 @@ error = ''' Incorrect usage of %s and %s ''' +["ddl:1246"] +error = ''' +Converting column '%s' from %s to %s +''' + ["ddl:1248"] error = ''' Every derived table must have its own alias diff --git a/planner/core/preprocess.go b/planner/core/preprocess.go index 101e2a0c53479..e98b50521dbe2 100644 --- a/planner/core/preprocess.go +++ b/planner/core/preprocess.go @@ -30,6 +30,7 @@ import ( "github.com/pingcap/tidb/meta/autoid" "github.com/pingcap/tidb/parser" "github.com/pingcap/tidb/parser/ast" + "github.com/pingcap/tidb/parser/charset" "github.com/pingcap/tidb/parser/format" "github.com/pingcap/tidb/parser/model" "github.com/pingcap/tidb/parser/mysql" @@ -782,8 +783,20 @@ func (p *preprocessor) checkCreateTableGrammar(stmt *ast.CreateTableStmt) { countPrimaryKey := 0 for _, colDef := range stmt.Cols { if err := checkColumn(colDef); err != nil { - p.err = err - return + if !p.ctx.GetSessionVars().SQLMode.HasStrictMode() && + colDef.Tp.Tp == mysql.TypeVarchar && colDef.Tp.Charset != "" && + terror.ErrorEqual(err, types.ErrTooBigFieldLength) { + // Convert to BLOB or TEXT, see also issue 30328 + colDef.Tp.Tp = mysql.TypeBlob + if colDef.Tp.Charset == charset.CharsetBin { + p.ctx.GetSessionVars().StmtCtx.AppendWarning(ddl.ErrAutoConvert.GenWithStackByArgs(colDef.Name.Name.O, "VARBINARY", "BLOB")) + } else { + p.ctx.GetSessionVars().StmtCtx.AppendWarning(ddl.ErrAutoConvert.GenWithStackByArgs(colDef.Name.Name.O, "VARCHAR", "TEXT")) + } + } else { + p.err = err + return + } } isPrimary, err := checkColumnOptions(stmt.TemporaryKeyword != ast.TemporaryNone, colDef.Options) if err != nil { diff --git a/planner/core/preprocess_test.go b/planner/core/preprocess_test.go index ff25091b4eeb8..84aaa8094be70 100644 --- a/planner/core/preprocess_test.go +++ b/planner/core/preprocess_test.go @@ -364,3 +364,27 @@ func (s *testValidatorSuite) TestDropGlobalTempTable(c *C) { s.runSQL(c, "drop global temporary table temp, ltemp1", false, core.ErrDropTableOnTemporaryTable) s.runSQL(c, "drop global temporary table test2.temp2, temp1", false, nil) } + +// For issue 30328 +func (s *testValidatorSuite) TestLargeVarcharAutoConv(c *C) { + _, err := s.se.Execute(context.Background(), "use test") + c.Assert(err, IsNil) + s.runSQL(c, "CREATE TABLE t1(a varbinary(70000), b varchar(70000000))", false, + errors.New("[types:1074]Column length too big for column 'a' (max = 65535); use BLOB or TEXT instead")) + + _, err = s.se.Execute(context.Background(), "SET sql_mode = 'NO_ENGINE_SUBSTITUTION'") + c.Assert(err, IsNil) + s.runSQL(c, "CREATE TABLE t1(a varbinary(70000), b varchar(70000000));", false, nil) + s.runSQL(c, "CREATE TABLE t1(a varbinary(70000), b varchar(70000000) charset utf8mb4);", false, nil) + warnCnt := s.se.GetSessionVars().StmtCtx.WarningCount() + // It is only 3 as for first stmt, ddl will append a warning for column b + c.Assert(warnCnt, Equals, uint16(3)) + warns := s.se.GetSessionVars().StmtCtx.GetWarnings() + for i := range warns { + c.Assert(terror.ErrorEqual(warns[i].Err, ddl.ErrAutoConvert), IsTrue) + } + + s.se.GetSessionVars().StmtCtx.SetWarnings(warns[:0]) + _, err = s.se.Execute(context.Background(), "SET sql_mode = 'STRICT_TRANS_TABLES,NO_ENGINE_SUBSTITUTION'") + c.Assert(err, IsNil) +}