From e586960027bf2f634cbe7ed6a930acbafd45280c Mon Sep 17 00:00:00 2001 From: crazycs Date: Wed, 28 Feb 2024 13:46:59 +0800 Subject: [PATCH] table: fix issue of get default value from column when column doesn't have default value (#51309) close pingcap/tidb#50043, close pingcap/tidb#51324 --- pkg/ddl/db_integration_test.go | 5 +- pkg/errctx/context.go | 1 + pkg/executor/insert_common.go | 3 + pkg/executor/test/executor/executor_test.go | 101 ++++++++++++++++++++ pkg/planner/core/planbuilder.go | 7 +- pkg/table/BUILD.bazel | 1 + pkg/table/column.go | 48 ++++++---- 7 files changed, 147 insertions(+), 19 deletions(-) diff --git a/pkg/ddl/db_integration_test.go b/pkg/ddl/db_integration_test.go index 083a12234db9d..2101711c335eb 100644 --- a/pkg/ddl/db_integration_test.go +++ b/pkg/ddl/db_integration_test.go @@ -1173,7 +1173,10 @@ func TestAlterColumn(t *testing.T) { tk.MustExec("alter table test_alter_column alter column d set default null") tk.MustExec("alter table test_alter_column alter column a drop default") tk.MustGetErrCode("insert into test_alter_column set b = 'd', c = 'dd'", errno.ErrNoDefaultForField) - tk.MustQuery("select a from test_alter_column").Check(testkit.Rows("111", "222", "222", "123")) + tk.MustGetErrCode("insert into test_alter_column set a = DEFAULT, b = 'd', c = 'dd'", errno.ErrNoDefaultForField) + tk.MustGetErrCode("insert into test_alter_column values (DEFAULT, 'd', 'dd', DEFAULT)", errno.ErrNoDefaultForField) + tk.MustExec("insert into test_alter_column set a = NULL, b = 'd', c = 'dd'") + tk.MustQuery("select a from test_alter_column").Check(testkit.Rows("111", "222", "222", "123", "")) // for failing tests sql := "alter table db_not_exist.test_alter_column alter column b set default 'c'" diff --git a/pkg/errctx/context.go b/pkg/errctx/context.go index 35104a7de5a09..0a1f48161b484 100644 --- a/pkg/errctx/context.go +++ b/pkg/errctx/context.go @@ -211,6 +211,7 @@ func init() { ErrGroupBadNull: { errno.ErrBadNull, errno.ErrWarnNullToNotnull, + errno.ErrNoDefaultForField, }, ErrGroupDividedByZero: { errno.ErrDivisionByZero, diff --git a/pkg/executor/insert_common.go b/pkg/executor/insert_common.go index 562841a6d39b6..77d25e7a26d76 100644 --- a/pkg/executor/insert_common.go +++ b/pkg/executor/insert_common.go @@ -584,6 +584,9 @@ func (e *InsertValues) getColDefaultValue(idx int, col *table.Column) (d types.D if col.DefaultIsExpr && col.DefaultExpr != nil { defaultVal, err = table.EvalColDefaultExpr(e.Ctx().GetExprCtx(), col.ToInfo(), col.DefaultExpr) } else { + if err := table.CheckNoDefaultValueForInsert(e.Ctx().GetSessionVars().StmtCtx, col.ToInfo()); err != nil { + return types.Datum{}, err + } defaultVal, err = table.GetColDefaultValue(e.Ctx().GetExprCtx(), col.ToInfo()) } if err != nil { diff --git a/pkg/executor/test/executor/executor_test.go b/pkg/executor/test/executor/executor_test.go index dc658d0811264..eb792ba36f971 100644 --- a/pkg/executor/test/executor/executor_test.go +++ b/pkg/executor/test/executor/executor_test.go @@ -2795,3 +2795,104 @@ func TestIssue38756(t *testing.T) { tk.MustQuery("(SELECT DISTINCT SQRT(1) FROM t)").Check(testkit.Rows("1")) tk.MustQuery("SELECT DISTINCT cast(1 as double) FROM t").Check(testkit.Rows("1")) } + +func TestIssue50043(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + // Test simplified case by update. + tk.MustExec("use test") + tk.MustExec("create table t (c1 boolean ,c2 decimal ( 37 , 17 ), unique key idx1 (c1 ,c2),unique key idx2 ( c1 ));") + tk.MustExec("insert into t values (0,NULL);") + tk.MustExec("alter table t alter column c2 drop default;") + tk.MustExec("update t set c2 = 5 where c1 = 0;") + tk.MustQuery("select * from t order by c1,c2").Check(testkit.Rows("0 5.00000000000000000")) + + // Test simplified case by insert on duplicate key update. + tk.MustExec("drop table if exists t") + tk.MustExec("create table t (c1 boolean ,c2 decimal ( 37 , 17 ), unique key idx1 (c1 ,c2));") + tk.MustExec("alter table t alter column c2 drop default;") + tk.MustExec("alter table t add unique key idx4 ( c1 );") + tk.MustExec("insert into t values (0, NULL), (1, 1);") + tk.MustExec("insert into t values (0, 2) ,(1, 3) on duplicate key update c2 = 5;") + tk.MustQuery("show warnings").Check(testkit.Rows()) + tk.MustQuery("select * from t order by c1,c2").Check(testkit.Rows("0 5.00000000000000000", "1 5.00000000000000000")) + + // Test Issue 50043. + tk.MustExec("drop table if exists t") + tk.MustExec("create table t (c1 boolean ,c2 decimal ( 37 , 17 ), unique key idx1 (c1 ,c2));") + tk.MustExec("alter table t alter column c2 drop default;") + tk.MustExec("alter table t add unique key idx4 ( c1 );") + tk.MustExec("insert into t values (0, NULL), (1, 1);") + tk.MustExec("insert ignore into t values (0, 2) ,(1, 3) on duplicate key update c2 = 5, c1 = 0") + tk.MustQuery("select * from t order by c1,c2").Check(testkit.Rows("0 5.00000000000000000", "1 1.00000000000000000")) +} + +func TestIssue51324(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("create table t (id int key, a int, b enum('a', 'b'))") + tk.MustGetErrMsg("insert into t values ()", "[table:1364]Field 'id' doesn't have a default value") + tk.MustExec("insert into t set id = 1") + tk.MustExec("insert into t set id = 2, a = NULL, b = NULL") + tk.MustExec("insert into t set id = 3, a = DEFAULT, b = DEFAULT") + tk.MustQuery("select * from t order by id").Check(testkit.Rows("1 ", "2 ", "3 ")) + + tk.MustExec("alter table t alter column a drop default") + tk.MustExec("alter table t alter column b drop default") + tk.MustGetErrMsg("insert into t set id = 4;", "[table:1364]Field 'a' doesn't have a default value") + tk.MustExec("insert into t set id = 5, a = NULL, b = NULL;") + tk.MustGetErrMsg("insert into t set id = 6, a = DEFAULT, b = DEFAULT;", "[table:1364]Field 'a' doesn't have a default value") + tk.MustQuery("select * from t order by id").Check(testkit.Rows("1 ", "2 ", "3 ", "5 ")) + + tk.MustExec("insert ignore into t set id = 4;") + tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1364 Field 'a' doesn't have a default value")) + tk.MustExec("insert ignore into t set id = 6, a = DEFAULT, b = DEFAULT;") + tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1364 Field 'a' doesn't have a default value")) + tk.MustQuery("select * from t order by id").Check(testkit.Rows("1 ", "2 ", "3 ", "4 ", "5 ", "6 ")) + tk.MustExec("update t set id = id + 10") + tk.MustQuery("show warnings").Check(testkit.Rows()) + tk.MustQuery("select * from t order by id").Check(testkit.Rows("11 ", "12 ", "13 ", "14 ", "15 ", "16 ")) + + // Test not null case. + tk.MustExec("drop table t") + tk.MustExec("create table t (id int key, a int not null, b enum('a', 'b') not null)") + tk.MustGetErrMsg("insert into t values ()", "[table:1364]Field 'id' doesn't have a default value") + tk.MustGetErrMsg("insert into t set id = 1", "[table:1364]Field 'a' doesn't have a default value") + tk.MustGetErrMsg("insert into t set id = 2, a = NULL, b = NULL", "[table:1048]Column 'a' cannot be null") + tk.MustGetErrMsg("insert into t set id = 2, a = 2, b = NULL", "[table:1048]Column 'b' cannot be null") + tk.MustGetErrMsg("insert into t set id = 3, a = DEFAULT, b = DEFAULT", "[table:1364]Field 'a' doesn't have a default value") + tk.MustExec("alter table t alter column a drop default") + tk.MustExec("alter table t alter column b drop default") + tk.MustExec("insert ignore into t set id = 4;") + tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1364 Field 'a' doesn't have a default value")) + tk.MustExec("insert ignore into t set id = 5, a = NULL, b = NULL;") + tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1048 Column 'a' cannot be null", "Warning 1048 Column 'b' cannot be null")) + tk.MustExec("insert ignore into t set id = 6, a = 6, b = NULL;") + tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1048 Column 'b' cannot be null")) + tk.MustExec("insert ignore into t set id = 7, a = DEFAULT, b = DEFAULT;") + tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1364 Field 'a' doesn't have a default value")) + tk.MustQuery("select * from t order by id").Check(testkit.Rows("4 0 a", "5 0 ", "6 6 ", "7 0 a")) + + // Test add column with OriginDefaultValue case. + tk.MustExec("drop table t") + tk.MustExec("create table t (id int, unique key idx (id))") + tk.MustExec("insert into t values (1)") + tk.MustExec("alter table t add column a int default 1") + tk.MustExec("alter table t add column b int default null") + tk.MustExec("alter table t add column c int not null") + tk.MustExec("alter table t add column d int not null default 1") + tk.MustExec("insert ignore into t (id) values (2)") + tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1364 Field 'c' doesn't have a default value")) + tk.MustExec("insert ignore into t (id) values (1),(2) on duplicate key update id = id+10") + tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1364 Field 'c' doesn't have a default value")) + tk.MustExec("alter table t alter column a drop default") + tk.MustExec("alter table t alter column b drop default") + tk.MustExec("alter table t alter column c drop default") + tk.MustExec("alter table t alter column d drop default") + tk.MustExec("insert ignore into t (id) values (3)") + tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1364 Field 'a' doesn't have a default value", "Warning 1364 Field 'b' doesn't have a default value", "Warning 1364 Field 'c' doesn't have a default value", "Warning 1364 Field 'd' doesn't have a default value")) + tk.MustExec("insert ignore into t (id) values (11),(12),(3) on duplicate key update id = id+10") + tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1364 Field 'a' doesn't have a default value", "Warning 1364 Field 'b' doesn't have a default value", "Warning 1364 Field 'c' doesn't have a default value", "Warning 1364 Field 'd' doesn't have a default value")) + tk.MustQuery("select * from t order by id").Check(testkit.Rows("13 0 0", "21 1 0 1", "22 1 0 1")) +} diff --git a/pkg/planner/core/planbuilder.go b/pkg/planner/core/planbuilder.go index 923d1f64d19a8..28de96c043bbe 100644 --- a/pkg/planner/core/planbuilder.go +++ b/pkg/planner/core/planbuilder.go @@ -3549,7 +3549,7 @@ func genAuthErrForGrantStmt(sctx PlanContext, dbName string) error { return plannererrors.ErrDBaccessDenied.FastGenByArgs(u, h, dbName) } -func (b *PlanBuilder) getDefaultValue(col *table.Column) (*expression.Constant, error) { +func (b *PlanBuilder) getDefaultValueForInsert(col *table.Column) (*expression.Constant, error) { var ( value types.Datum err error @@ -3557,6 +3557,9 @@ func (b *PlanBuilder) getDefaultValue(col *table.Column) (*expression.Constant, if col.DefaultIsExpr && col.DefaultExpr != nil { value, err = table.EvalColDefaultExpr(b.ctx, col.ToInfo(), col.DefaultExpr) } else { + if err := table.CheckNoDefaultValueForInsert(b.ctx.GetSessionVars().StmtCtx, col.ToInfo()); err != nil { + return nil, err + } value, err = table.GetColDefaultValue(b.ctx, col.ToInfo()) } if err != nil { @@ -3845,7 +3848,7 @@ func (b PlanBuilder) getInsertColExpr(ctx context.Context, insertPlan *Insert, m // See note in the end of the function. Only default for generated columns are OK. return nil, nil } - outExpr, err = b.getDefaultValue(refCol) + outExpr, err = b.getDefaultValueForInsert(refCol) case *driver.ValueExpr: outExpr = &expression.Constant{ Value: x.Datum, diff --git a/pkg/table/BUILD.bazel b/pkg/table/BUILD.bazel index d0c4f1ea355cb..23e8a9b333519 100644 --- a/pkg/table/BUILD.bazel +++ b/pkg/table/BUILD.bazel @@ -23,6 +23,7 @@ go_library( "//pkg/parser/mysql", "//pkg/parser/types", "//pkg/sessionctx", + "//pkg/sessionctx/stmtctx", "//pkg/sessionctx/variable", "//pkg/table/context", "//pkg/types", diff --git a/pkg/table/column.go b/pkg/table/column.go index f89a21edb4ab1..4c1630d5ae0da 100644 --- a/pkg/table/column.go +++ b/pkg/table/column.go @@ -34,6 +34,7 @@ import ( "github.com/pingcap/tidb/pkg/parser/mysql" field_types "github.com/pingcap/tidb/pkg/parser/types" "github.com/pingcap/tidb/pkg/sessionctx" + "github.com/pingcap/tidb/pkg/sessionctx/stmtctx" "github.com/pingcap/tidb/pkg/sessionctx/variable" "github.com/pingcap/tidb/pkg/types" "github.com/pingcap/tidb/pkg/util/chunk" @@ -536,6 +537,23 @@ func GetColOriginDefaultValueWithoutStrictSQLMode(ctx expression.BuildContext, c }) } +// CheckNoDefaultValueForInsert checks if the column has no default value before insert data. +// CheckNoDefaultValueForInsert extracts the check logic from getColDefaultValueFromNil, +// since getColDefaultValueFromNil function is public path and both read/write and other places use it. +// But CheckNoDefaultValueForInsert logic should only check before insert. +func CheckNoDefaultValueForInsert(sc *stmtctx.StatementContext, col *model.ColumnInfo) error { + if mysql.HasNoDefaultValueFlag(col.GetFlag()) && !col.DefaultIsExpr && col.GetDefaultValue() == nil && col.GetType() != mysql.TypeEnum { + ignoreErr := sc.ErrGroupLevel(errctx.ErrGroupBadNull) != errctx.LevelError + if !ignoreErr { + return ErrNoDefaultValue.GenWithStackByArgs(col.Name) + } + if !mysql.HasNotNullFlag(col.GetFlag()) { + sc.AppendWarning(ErrNoDefaultValue.FastGenByArgs(col.Name)) + } + } + return nil +} + // GetColDefaultValue gets default value of the column. func GetColDefaultValue(ctx expression.BuildContext, col *model.ColumnInfo) (types.Datum, error) { defaultValue := col.GetDefaultValue() @@ -624,22 +642,19 @@ func getColDefaultValue(ctx expression.BuildContext, col *model.ColumnInfo, defa } func getColDefaultValueFromNil(ctx expression.BuildContext, col *model.ColumnInfo, args *getColOriginDefaultValue) (types.Datum, error) { - if !mysql.HasNotNullFlag(col.GetFlag()) && !mysql.HasNoDefaultValueFlag(col.GetFlag()) { + if !mysql.HasNotNullFlag(col.GetFlag()) { return types.Datum{}, nil } if col.GetType() == mysql.TypeEnum { // For enum type, if no default value and not null is set, // the default value is the first element of the enum list - if mysql.HasNotNullFlag(col.GetFlag()) { - defEnum, err := types.ParseEnumValue(col.FieldType.GetElems(), 1) - if err != nil { - return types.Datum{}, err - } - return types.NewCollateMysqlEnumDatum(defEnum, col.GetCollate()), nil + defEnum, err := types.ParseEnumValue(col.FieldType.GetElems(), 1) + if err != nil { + return types.Datum{}, err } - return types.Datum{}, nil + return types.NewCollateMysqlEnumDatum(defEnum, col.GetCollate()), nil } - if mysql.HasAutoIncrementFlag(col.GetFlag()) && !mysql.HasNoDefaultValueFlag(col.GetFlag()) { + if mysql.HasAutoIncrementFlag(col.GetFlag()) { // Auto increment column doesn't have default value and we should not return error. return GetZeroValue(col), nil } @@ -653,15 +668,16 @@ func getColDefaultValueFromNil(ctx expression.BuildContext, col *model.ColumnInf } if !strictSQLMode { sc.AppendWarning(ErrNoDefaultValue.FastGenByArgs(col.Name)) - if mysql.HasNotNullFlag(col.GetFlag()) { - return GetZeroValue(col), nil - } - if mysql.HasNoDefaultValueFlag(col.GetFlag()) { - return types.Datum{}, nil - } + return GetZeroValue(col), nil } ec := sc.ErrCtx() - if ec.HandleError(ErrColumnCantNull.FastGenByArgs(col.Name)) == nil { + var err error + if mysql.HasNoDefaultValueFlag(col.GetFlag()) { + err = ErrNoDefaultValue.FastGenByArgs(col.Name) + } else { + err = ErrColumnCantNull.FastGenByArgs(col.Name) + } + if ec.HandleError(err) == nil { return GetZeroValue(col), nil } return types.Datum{}, ErrNoDefaultValue.GenWithStackByArgs(col.Name)