From c7f65fe565ac83d75eec105e6c196c6be4886e90 Mon Sep 17 00:00:00 2001 From: Lynn Date: Thu, 11 Apr 2024 16:43:04 +0800 Subject: [PATCH 1/3] *: support add column for default value expression --- pkg/ddl/column.go | 15 +++++++++++++++ pkg/ddl/ddl_api.go | 4 +++- pkg/parser/model/model.go | 1 + .../t/ddl/default_as_expression.test | 9 +++------ 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/pkg/ddl/column.go b/pkg/ddl/column.go index 8123231a934fc..369fe65311990 100644 --- a/pkg/ddl/column.go +++ b/pkg/ddl/column.go @@ -1948,6 +1948,7 @@ func modifyColsFromNull2NotNull(w *worker, dbInfo *model.DBInfo, tblInfo *model. func generateOriginDefaultValue(col *model.ColumnInfo, ctx sessionctx.Context) (any, error) { var err error odValue := col.GetDefaultValue() + logutil.BgLogger().Info(fmt.Sprintf("112------default val: %v \n", odValue)) if odValue == nil && mysql.HasNotNullFlag(col.GetFlag()) { switch col.GetType() { // Just use enum field's first element for OriginDefaultValue. @@ -1979,6 +1980,20 @@ func generateOriginDefaultValue(col *model.ColumnInfo, ctx sessionctx.Context) ( } else if col.GetType() == mysql.TypeDatetime { odValue = types.NewTime(types.FromGoTime(t), col.GetType(), col.GetDecimal()).String() } + return odValue, nil + } + + if col.DefaultIsExpr { + oldValue := strings.ToLower(odValue.(string)) + // It's checked in getFuncCallDefaultValue. + if strings.Contains(oldValue, fmt.Sprintf("%s(%s(),", ast.DateFormat, ast.Now)) || strings.Contains(oldValue, ast.StrToDate) { + odValue, err = table.GetColDefaultValue(ctx.GetExprCtx(), col) + if err != nil { + return nil, types.ErrInvalidDefault.GenWithStackByArgs(col.Name) + } + } else { + return nil, errors.Trace(dbterror.ErrBinlogUnsafeSystemFunction.GenWithStackByArgs()) + } } return odValue, nil } diff --git a/pkg/ddl/ddl_api.go b/pkg/ddl/ddl_api.go index 2fc71ffa4e4c6..ee22f65b16420 100644 --- a/pkg/ddl/ddl_api.go +++ b/pkg/ddl/ddl_api.go @@ -4311,7 +4311,7 @@ func CreateNewColumn(ctx sessionctx.Context, schema *model.DBInfo, spec *ast.Alt return nil, errors.Trace(err) } return nil, errors.Trace(dbterror.ErrAddColumnWithSequenceAsDefault.GenWithStackByArgs(specNewColumn.Name.Name.O)) - case ast.Rand, ast.UUID, ast.UUIDToBin, ast.Replace, ast.Upper, ast.DateFormat, ast.StrToDate: + case ast.Rand, ast.UUID, ast.UUIDToBin, ast.Replace, ast.Upper: return nil, errors.Trace(dbterror.ErrBinlogUnsafeSystemFunction.GenWithStackByArgs()) } } @@ -4339,6 +4339,7 @@ func CreateNewColumn(ctx sessionctx.Context, schema *model.DBInfo, spec *ast.Alt return nil, errors.Trace(err) } + logutil.BgLogger().Info(fmt.Sprintf("111------default val: %v \n", col.DefaultValue)) originDefVal, err := generateOriginDefaultValue(col.ToInfo(), ctx) if err != nil { return nil, errors.Trace(err) @@ -5543,6 +5544,7 @@ func SetDefaultValue(ctx sessionctx.Context, col *table.Column, option *ast.Colu func setDefaultValueWithBinaryPadding(col *table.Column, value any) error { err := col.SetDefaultValue(value) + logutil.BgLogger().Info(fmt.Sprintf("110------default val: %v \n", col.DefaultValue)) if err != nil { return err } diff --git a/pkg/parser/model/model.go b/pkg/parser/model/model.go index 88056ae9a40ae..400a5edd64172 100644 --- a/pkg/parser/model/model.go +++ b/pkg/parser/model/model.go @@ -269,6 +269,7 @@ func (c *ColumnInfo) GetOriginDefaultValue() interface{} { // SetDefaultValue sets the default value. func (c *ColumnInfo) SetDefaultValue(value interface{}) error { c.DefaultValue = value + fmt.Printf("default val: %v \n", c.DefaultValue) if c.GetType() == mysql.TypeBit { // For mysql.TypeBit type, the default value storage format must be a string. // Other value such as int must convert to string format first. diff --git a/tests/integrationtest/t/ddl/default_as_expression.test b/tests/integrationtest/t/ddl/default_as_expression.test index 2c86e6352cbfe..e73587b38b0dc 100644 --- a/tests/integrationtest/t/ddl/default_as_expression.test +++ b/tests/integrationtest/t/ddl/default_as_expression.test @@ -12,7 +12,6 @@ create table t5 (c int(10), c1 date default (date_format(now(),_utf8mb4'%Y-%m-%d create table t6 (c int(10), c1 varchar(256) default (date_format(now(),'%b %d %Y %h:%i %p'))); -- error 3770 create table t7 (c int(10), c1 varchar(256) default (date_format(now(),'%Y-%m-%d %H:%i:%s %p'))); --- error 1674 alter table t0 add column c2 date default (date_format(now(),'%Y-%m')); # insert records SET @x := NOW(); @@ -91,8 +90,6 @@ create table t1 (c int(10), c1 varchar(256) default (REPLACE(UPPER('dfdkj-kjkl-d alter table t add column c2 varchar(32) default (REPLACE(UPPER(UUID()), '-', '')); -- error 1674 alter table t add column c3 int default (UPPER(UUID())); -# Alter support "REPLACE(UPPER('dfdkj-kjkl-d'), '-', '')", we need to support this DDL. --- error 1674 alter table t add column c4 int default (REPLACE(UPPER('dfdkj-kjkl-d'), '-', '')); # insert records @@ -143,10 +140,9 @@ set session sql_mode=@sqlMode; create table t6 (c int(10), c1 varchar(32) default (str_to_date(upper('1980-01-01'),'%Y-%m-%d'))); -- error 3770 create table t6 (c int(10), c1 varchar(32) default (str_to_date('1980-01-01',upper('%Y-%m-%d')))); -# TODO: We need to support it. --- error 1674 +-- error 0,1292 +alter table t0 add column c3 datetime default (str_to_date('1980-01-01','%Y-%m-%d')); alter table t0 add column c3 varchar(32) default (str_to_date('1980-01-01','%Y-%m-%d')); --- error 1674 alter table t0 add column c4 int default (str_to_date('1980-01-01','%Y-%m-%d')); # insert records @@ -216,6 +212,7 @@ create table t2 (c int(10), c1 varchar(256) default (upper(substring_index('fjks create table t2 (c int(10), c1 varchar(256) default (upper(substring_index(user(),'x',1)))); -- error 1674 alter table t add column c2 varchar(32) default (upper(substring_index(user(),'@',1))); +# We don't support "upper(substring_index('fjks@jkkl','@',1))", so we return an error as (upper(substring_index(user(),'@',1))). -- error 1674 alter table t add column c3 int default (upper(substring_index('fjks@jkkl','@',1))); -- error 1292 From ee8dad0f452472d5833d3dc5ff53ff6321df1e79 Mon Sep 17 00:00:00 2001 From: Lynn Date: Fri, 12 Apr 2024 10:17:18 +0800 Subject: [PATCH 2/3] *: add tests --- pkg/ddl/column.go | 32 ++++++---- pkg/ddl/column_type_change_test.go | 47 ++++++++++---- pkg/ddl/db_change_test.go | 7 ++ pkg/ddl/db_integration_test.go | 6 ++ pkg/ddl/ddl_api.go | 2 - pkg/parser/model/model.go | 1 - .../r/ddl/default_as_expression.result | 64 ++++++++++++------- .../t/ddl/default_as_expression.test | 28 ++++---- 8 files changed, 125 insertions(+), 62 deletions(-) diff --git a/pkg/ddl/column.go b/pkg/ddl/column.go index 369fe65311990..2e68cbe48da51 100644 --- a/pkg/ddl/column.go +++ b/pkg/ddl/column.go @@ -1948,8 +1948,9 @@ func modifyColsFromNull2NotNull(w *worker, dbInfo *model.DBInfo, tblInfo *model. func generateOriginDefaultValue(col *model.ColumnInfo, ctx sessionctx.Context) (any, error) { var err error odValue := col.GetDefaultValue() - logutil.BgLogger().Info(fmt.Sprintf("112------default val: %v \n", odValue)) - if odValue == nil && mysql.HasNotNullFlag(col.GetFlag()) { + if odValue == nil && mysql.HasNotNullFlag(col.GetFlag()) || + // It's for drop column and modify column. + (col.DefaultIsExpr && odValue != strings.ToUpper(ast.CurrentTimestamp) && ctx == nil) { switch col.GetType() { // Just use enum field's first element for OriginDefaultValue. case mysql.TypeEnum: @@ -1983,16 +1984,25 @@ func generateOriginDefaultValue(col *model.ColumnInfo, ctx sessionctx.Context) ( return odValue, nil } - if col.DefaultIsExpr { - oldValue := strings.ToLower(odValue.(string)) + if col.DefaultIsExpr && ctx != nil { + valStr, ok := odValue.(string) + if !ok { + return nil, dbterror.ErrDefValGeneratedNamedFunctionIsNotAllowed.GenWithStackByArgs(col.Name.String()) + } + oldValue := strings.ToLower(valStr) // It's checked in getFuncCallDefaultValue. - if strings.Contains(oldValue, fmt.Sprintf("%s(%s(),", ast.DateFormat, ast.Now)) || strings.Contains(oldValue, ast.StrToDate) { - odValue, err = table.GetColDefaultValue(ctx.GetExprCtx(), col) - if err != nil { - return nil, types.ErrInvalidDefault.GenWithStackByArgs(col.Name) - } - } else { - return nil, errors.Trace(dbterror.ErrBinlogUnsafeSystemFunction.GenWithStackByArgs()) + if !strings.Contains(oldValue, fmt.Sprintf("%s(%s(),", ast.DateFormat, ast.Now)) && + !strings.Contains(oldValue, ast.StrToDate) { + return nil, errors.Trace(dbterror.ErrBinlogUnsafeSystemFunction) + } + + defVal, err := table.GetColDefaultValue(ctx.GetExprCtx(), col) + if err != nil { + return nil, errors.Trace(err) + } + odValue, err = defVal.ToString() + if err != nil { + return nil, errors.Trace(err) } } return odValue, nil diff --git a/pkg/ddl/column_type_change_test.go b/pkg/ddl/column_type_change_test.go index 0296cd87c6430..f2e60ccaca52a 100644 --- a/pkg/ddl/column_type_change_test.go +++ b/pkg/ddl/column_type_change_test.go @@ -498,31 +498,52 @@ func TestChangingColOriginDefaultValueAfterAddColAndCastFail(t *testing.T) { if job.SchemaState == model.StateWriteOnly || job.SchemaState == model.StateWriteReorganization { tbl := external.GetTableByName(t, tk, "test", "t") if len(tbl.WritableCols()) != 4 { - errMsg := fmt.Sprintf("cols len:%v", len(tbl.WritableCols())) + errMsg := fmt.Sprintf("job ID:%d, cols len:%v", job.ID, len(tbl.WritableCols())) checkErr = errors.New("assert the writable column number error" + errMsg) return } - originalDV := fmt.Sprintf("%v", tbl.WritableCols()[3].OriginDefaultValue) - expectVal := "0000-00-00 00:00:00" - if originalDV != expectVal { - errMsg := fmt.Sprintf("expect: %v, got: %v", expectVal, originalDV) - checkErr = errors.New("assert the write only column origin default value error" + errMsg) - return + // modify column x + if job.ID == 107 { + originalDV := fmt.Sprintf("%v", tbl.WritableCols()[3].OriginDefaultValue) + expectVal := "0000-00-00 00:00:00" + if originalDV != expectVal { + errMsg := fmt.Sprintf("job ID:%d, expect: %v, got: %v", job.ID, expectVal, originalDV) + checkErr = errors.New("assert the write only column origin default value error" + errMsg) + return + } + // The cast value will be inserted into changing column too. + _, err := tk1.Exec("UPDATE t SET a = '18apf' WHERE x = '' AND a = 'mul'") + if err != nil { + checkErr = err + return + } } - // The casted value will be inserted into changing column too. - _, err := tk1.Exec("UPDATE t SET a = '18apf' WHERE x = '' AND a = 'mul'") - if err != nil { - checkErr = err - return + // modify column b + if job.ID == 108 { + originalDV := fmt.Sprintf("%v", tbl.WritableCols()[3].OriginDefaultValue) + expectVal := "" + if originalDV != expectVal { + errMsg := fmt.Sprintf("job ID:%d, expect: %v, got: %v", job.ID, expectVal, originalDV) + checkErr = errors.New("assert the write only column origin default value error" + errMsg) + return + } + // The cast value will be inserted into changing column too. + _, err := tk1.Exec("UPDATE t SET a = '18apf' WHERE a = '1'") + if err != nil { + checkErr = err + return + } } } } dom.DDL().SetHook(hook) tk.MustExec("alter table t modify column x DATETIME NULL DEFAULT '3771-02-28 13:00:11' AFTER b;") + tk.MustExec("insert into t(a) value('1')") + tk.MustExec("alter table t modify column b varchar(256) default (REPLACE(UPPER(UUID()), '-', ''));") dom.DDL().SetHook(originalHook) require.NoError(t, checkErr) - tk.MustQuery("select * from t order by a").Check(testkit.Rows()) + tk.MustQuery("select * from t order by a").Check(testkit.Rows("18apf -729850476163 3771-02-28 13:00:11")) tk.MustExec("drop table if exists t") } diff --git a/pkg/ddl/db_change_test.go b/pkg/ddl/db_change_test.go index 8984002bd21af..8bf0422d2aef2 100644 --- a/pkg/ddl/db_change_test.go +++ b/pkg/ddl/db_change_test.go @@ -136,6 +136,8 @@ func TestDropNotNullColumn(t *testing.T) { tk.MustExec("insert into t2 values(3, '11:22:33')") tk.MustExec("create table t3 (id int, d json not null)") tk.MustExec("insert into t3 values(4, d)") + tk.MustExec("create table t4 (id int, e varchar(256) default (REPLACE(UPPER(UUID()), '-', '')) not null)") + tk.MustExec("insert into t4 values(4, 3)") tk1 := testkit.NewTestKit(t, store) tk1.MustExec("use test") @@ -161,6 +163,8 @@ func TestDropNotNullColumn(t *testing.T) { _, checkErr = tk1.Exec("insert into t2 set id = 3") case 3: _, checkErr = tk1.Exec("insert into t3 set id = 4") + case 4: + _, checkErr = tk1.Exec("insert into t4 set id = 5") } } } @@ -177,6 +181,9 @@ func TestDropNotNullColumn(t *testing.T) { sqlNum++ tk.MustExec("alter table t3 drop column d") require.NoError(t, checkErr) + sqlNum++ + tk.MustExec("alter table t4 drop column e") + require.NoError(t, checkErr) d.SetHook(originalCallback) tk.MustExec("drop table t, t1, t2, t3") } diff --git a/pkg/ddl/db_integration_test.go b/pkg/ddl/db_integration_test.go index f41de8b89b207..235d4bea29412 100644 --- a/pkg/ddl/db_integration_test.go +++ b/pkg/ddl/db_integration_test.go @@ -1657,6 +1657,12 @@ func TestDefaultValueAsExpressions(t *testing.T) { tk.MustExec("create table t2(c int(10), c1 varchar(256) default (REPLACE(UPPER(UUID()), '-', '')), index idx(c1));") tk.MustExec("insert into t2(c) values (1),(2),(3);") tk.MustGetErrCode("alter table t2 modify column c1 varchar(30) default 'xx';", errno.WarnDataTruncated) + // test add column for enum + nowStr := time.Now().Format("2006-01") + sql := fmt.Sprintf("alter table t2 add column c3 enum('%v','n')", nowStr) + " default (date_format(now(),'%Y-%m'))" + tk.MustExec(sql) + tk.MustExec("insert into t2(c) values (4);") + tk.MustQuery("select c3 from t2").Check(testkit.Rows(nowStr, nowStr, nowStr, nowStr)) } func TestChangingDBCharset(t *testing.T) { diff --git a/pkg/ddl/ddl_api.go b/pkg/ddl/ddl_api.go index ee22f65b16420..6b93bc6921239 100644 --- a/pkg/ddl/ddl_api.go +++ b/pkg/ddl/ddl_api.go @@ -4339,7 +4339,6 @@ func CreateNewColumn(ctx sessionctx.Context, schema *model.DBInfo, spec *ast.Alt return nil, errors.Trace(err) } - logutil.BgLogger().Info(fmt.Sprintf("111------default val: %v \n", col.DefaultValue)) originDefVal, err := generateOriginDefaultValue(col.ToInfo(), ctx) if err != nil { return nil, errors.Trace(err) @@ -5544,7 +5543,6 @@ func SetDefaultValue(ctx sessionctx.Context, col *table.Column, option *ast.Colu func setDefaultValueWithBinaryPadding(col *table.Column, value any) error { err := col.SetDefaultValue(value) - logutil.BgLogger().Info(fmt.Sprintf("110------default val: %v \n", col.DefaultValue)) if err != nil { return err } diff --git a/pkg/parser/model/model.go b/pkg/parser/model/model.go index 400a5edd64172..88056ae9a40ae 100644 --- a/pkg/parser/model/model.go +++ b/pkg/parser/model/model.go @@ -269,7 +269,6 @@ func (c *ColumnInfo) GetOriginDefaultValue() interface{} { // SetDefaultValue sets the default value. func (c *ColumnInfo) SetDefaultValue(value interface{}) error { c.DefaultValue = value - fmt.Printf("default val: %v \n", c.DefaultValue) if c.GetType() == mysql.TypeBit { // For mysql.TypeBit type, the default value storage format must be a string. // Other value such as int must convert to string format first. diff --git a/tests/integrationtest/r/ddl/default_as_expression.result b/tests/integrationtest/r/ddl/default_as_expression.result index 429a2c48d6d9c..87c7038cf13dd 100644 --- a/tests/integrationtest/r/ddl/default_as_expression.result +++ b/tests/integrationtest/r/ddl/default_as_expression.result @@ -10,8 +10,6 @@ create table t6 (c int(10), c1 varchar(256) default (date_format(now(),'%b %d %Y Error 3770 (HY000): Default value expression of column 'c1' contains a disallowed function: `KindString %b %d %Y %h:%i %p`. create table t7 (c int(10), c1 varchar(256) default (date_format(now(),'%Y-%m-%d %H:%i:%s %p'))); Error 3770 (HY000): Default value expression of column 'c1' contains a disallowed function: `KindString %Y-%m-%d %H:%i:%s %p`. -alter table t0 add column c2 date default (date_format(now(),'%Y-%m')); -Error 1674 (HY000): Statement is unsafe because it uses a system function that may return a different value on the slave SET @x := NOW(); insert into t0(c) values (1); insert into t0 values (2, default); @@ -63,15 +61,23 @@ t2 CREATE TABLE `t2` ( `c` int(10) DEFAULT NULL, `c1` varchar(256) DEFAULT date_format(now(), _utf8mb4'%Y-%m-%d %H.%i.%s') ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin +alter table t0 add column c2 date default (date_format(now(),'%Y-%m')); +Error 1292 (22007): Incorrect datetime value: '2024-04' alter table t0 add index idx(c1); alter table t1 add index idx(c1); -insert into t0 values (3, default); +alter table t0 add column c2 date default (date_format(now(),'%Y-%m-%d')); +alter table t0 add column c3 enum('y','n') default (date_format(now(),'%Y-%m-%d')); +Error 1265 (01000): Data truncated for column '%s' at row %d +alter table t0 add column c4 blob default (date_format(now(),'%Y-%m-%d')); +insert into t0 values (3, default, default, default); insert into t1 values (3, default); show create table t0; Table Create Table t0 CREATE TABLE `t0` ( `c` int(10) DEFAULT NULL, `c1` varchar(256) DEFAULT date_format(now(), _utf8mb4'%Y-%m'), + `c2` date DEFAULT date_format(now(), _utf8mb4'%Y-%m-%d'), + `c4` blob DEFAULT date_format(now(), _utf8mb4'%Y-%m-%d'), KEY `idx` (`c1`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin show create table t1; @@ -83,13 +89,15 @@ t1 CREATE TABLE `t1` ( ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin alter table t0 modify column c1 varchar(30) default 'xx'; alter table t1 modify column c1 varchar(30) default 'xx'; -insert into t0 values (4, default); +insert into t0 values (4, default, default, default); insert into t1 values (4, default); show create table t0; Table Create Table t0 CREATE TABLE `t0` ( `c` int(10) DEFAULT NULL, `c1` varchar(30) DEFAULT 'xx', + `c2` date DEFAULT date_format(now(), _utf8mb4'%Y-%m-%d'), + `c4` blob DEFAULT date_format(now(), _utf8mb4'%Y-%m-%d'), KEY `idx` (`c1`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin show create table t1; @@ -102,7 +110,7 @@ t1 CREATE TABLE `t1` ( alter table t0 modify column c1 datetime DEFAULT (date_format(now(), '%Y-%m-%d')); Error 1292 (22007): Incorrect datetime value: