From 0267a7fbdf1c8564daca066df116c51a28ad773c Mon Sep 17 00:00:00 2001 From: Du Chuan Date: Mon, 4 Mar 2019 11:19:11 +0800 Subject: [PATCH] ddl: validate fsp for datetime/timestamp if they are used as defaultvalue of datetime/timestamp columns (#9327) --- cmd/explaintest/r/topn_push_down.result | 20 ++++++------- cmd/explaintest/t/topn_push_down.test | 20 ++++++------- ddl/ddl.go | 5 +++- ddl/ddl_api.go | 27 +++++++++++++---- executor/ddl_test.go | 40 +++++++++++++++++++++++++ 5 files changed, 85 insertions(+), 27 deletions(-) diff --git a/cmd/explaintest/r/topn_push_down.result b/cmd/explaintest/r/topn_push_down.result index b5a8f27907..f7d630574b 100644 --- a/cmd/explaintest/r/topn_push_down.result +++ b/cmd/explaintest/r/topn_push_down.result @@ -4,7 +4,7 @@ CREATE TABLE `tr` ( `domain_type` tinyint(4) NOT NULL, `business_type` tinyint(4) NOT NULL, `trade_type` tinyint(4) NOT NULL DEFAULT '1', -`trade_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, +`trade_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3), `trade_status` tinyint(4) NOT NULL DEFAULT '0', `trade_pay_status` tinyint(4) NOT NULL DEFAULT '0', `delivery_type` tinyint(4) NOT NULL DEFAULT '0', @@ -24,10 +24,10 @@ CREATE TABLE `tr` ( `device_identy` varchar(36) NOT NULL, `uuid` varchar(32) NOT NULL, `status_flag` tinyint(4) NOT NULL, -`client_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, +`client_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3), `client_update_time` timestamp(3) NULL DEFAULT NULL, -`server_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, -`server_update_time` timestamp(3) DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, +`server_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3), +`server_update_time` timestamp(3) DEFAULT CURRENT_TIMESTAMP(3) ON UPDATE CURRENT_TIMESTAMP(3), `creator_id` bigint(20) DEFAULT NULL, `creator_name` varchar(32) DEFAULT NULL, `updator_id` bigint(20) DEFAULT NULL, @@ -66,10 +66,10 @@ CREATE TABLE `p` ( `device_identy` varchar(36) NOT NULL, `uuid` varchar(32) NOT NULL, `status_flag` tinyint(4) NOT NULL DEFAULT '1', -`client_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, +`client_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3), `client_update_time` timestamp(3) NULL DEFAULT NULL, -`server_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, -`server_update_time` timestamp(3) DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, +`server_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3), +`server_update_time` timestamp(3) DEFAULT CURRENT_TIMESTAMP(3) ON UPDATE CURRENT_TIMESTAMP(3), `creator_id` bigint(20) DEFAULT NULL, `creator_name` varchar(32) DEFAULT NULL, `updator_id` bigint(20) DEFAULT NULL, @@ -117,10 +117,10 @@ CREATE TABLE `te` ( `device_identy` varchar(36) NOT NULL, `uuid` varchar(32) NOT NULL, `status_flag` tinyint(4) NOT NULL, -`client_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, +`client_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3), `client_update_time` timestamp(3) NULL DEFAULT NULL, -`server_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, -`server_update_time` timestamp(3) DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, +`server_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3), +`server_update_time` timestamp(3) DEFAULT CURRENT_TIMESTAMP(3) ON UPDATE CURRENT_TIMESTAMP(3), `creator_id` bigint(20) DEFAULT NULL, `creator_name` varchar(32) DEFAULT NULL, `updator_id` bigint(20) DEFAULT NULL, diff --git a/cmd/explaintest/t/topn_push_down.test b/cmd/explaintest/t/topn_push_down.test index e37f0e7512..54dab77c25 100644 --- a/cmd/explaintest/t/topn_push_down.test +++ b/cmd/explaintest/t/topn_push_down.test @@ -4,7 +4,7 @@ CREATE TABLE `tr` ( `domain_type` tinyint(4) NOT NULL, `business_type` tinyint(4) NOT NULL, `trade_type` tinyint(4) NOT NULL DEFAULT '1', - `trade_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + `trade_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3), `trade_status` tinyint(4) NOT NULL DEFAULT '0', `trade_pay_status` tinyint(4) NOT NULL DEFAULT '0', `delivery_type` tinyint(4) NOT NULL DEFAULT '0', @@ -24,10 +24,10 @@ CREATE TABLE `tr` ( `device_identy` varchar(36) NOT NULL, `uuid` varchar(32) NOT NULL, `status_flag` tinyint(4) NOT NULL, - `client_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + `client_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3), `client_update_time` timestamp(3) NULL DEFAULT NULL, - `server_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, - `server_update_time` timestamp(3) DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + `server_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3), + `server_update_time` timestamp(3) DEFAULT CURRENT_TIMESTAMP(3) ON UPDATE CURRENT_TIMESTAMP(3), `creator_id` bigint(20) DEFAULT NULL, `creator_name` varchar(32) DEFAULT NULL, `updator_id` bigint(20) DEFAULT NULL, @@ -68,10 +68,10 @@ CREATE TABLE `p` ( `device_identy` varchar(36) NOT NULL, `uuid` varchar(32) NOT NULL, `status_flag` tinyint(4) NOT NULL DEFAULT '1', - `client_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + `client_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3), `client_update_time` timestamp(3) NULL DEFAULT NULL, - `server_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, - `server_update_time` timestamp(3) DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + `server_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3), + `server_update_time` timestamp(3) DEFAULT CURRENT_TIMESTAMP(3) ON UPDATE CURRENT_TIMESTAMP(3), `creator_id` bigint(20) DEFAULT NULL, `creator_name` varchar(32) DEFAULT NULL, `updator_id` bigint(20) DEFAULT NULL, @@ -121,10 +121,10 @@ CREATE TABLE `te` ( `device_identy` varchar(36) NOT NULL, `uuid` varchar(32) NOT NULL, `status_flag` tinyint(4) NOT NULL, - `client_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + `client_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3), `client_update_time` timestamp(3) NULL DEFAULT NULL, - `server_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, - `server_update_time` timestamp(3) DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + `server_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3), + `server_update_time` timestamp(3) DEFAULT CURRENT_TIMESTAMP(3) ON UPDATE CURRENT_TIMESTAMP(3), `creator_id` bigint(20) DEFAULT NULL, `creator_name` varchar(32) DEFAULT NULL, `updator_id` bigint(20) DEFAULT NULL, diff --git a/ddl/ddl.go b/ddl/ddl.go index 706074ecb5..7f9394543e 100644 --- a/ddl/ddl.go +++ b/ddl/ddl.go @@ -123,6 +123,8 @@ var ( // errBlobCantHaveDefault forbids to give not null default value to TEXT/BLOB/JSON. errBlobCantHaveDefault = terror.ClassDDL.New(codeBlobCantHaveDefault, mysql.MySQLErrName[mysql.ErrBlobCantHaveDefault]) errTooLongIndexComment = terror.ClassDDL.New(codeErrTooLongIndexComment, mysql.MySQLErrName[mysql.ErrTooLongIndexComment]) + // ErrInvalidDefaultValue returns for invalid default value for columns. + ErrInvalidDefaultValue = terror.ClassDDL.New(codeInvalidDefaultValue, mysql.MySQLErrName[mysql.ErrInvalidDefault]) // ErrGeneratedColumnRefAutoInc forbids to refer generated columns to auto-increment columns . ErrGeneratedColumnRefAutoInc = terror.ClassDDL.New(codeErrGeneratedColumnRefAutoInc, mysql.MySQLErrName[mysql.ErrGeneratedColumnRefAutoInc]) // ErrUnsupportedAddPartition returns for does not support add partitions. @@ -648,7 +650,7 @@ const ( codeBadField = 1054 codeTooLongIdent = 1059 codeDupKeyName = 1061 - codeInvalidDefaultValue = 1067 + codeInvalidDefaultValue = mysql.ErrInvalidDefault codeTooLongKey = 1071 codeKeyColumnDoesNotExits = mysql.ErrKeyColumnDoesNotExits codeIncorrectPrefixKey = 1089 @@ -747,6 +749,7 @@ func init() { codeUnknownPartition: mysql.ErrUnknownPartition, codeNotSupportedAlterOperation: mysql.ErrAlterOperationNotSupportedReason, codeErrWrongObject: mysql.ErrWrongObject, + codeInvalidDefaultValue: mysql.ErrInvalidDefault, codeErrGeneratedColumnRefAutoInc: mysql.ErrGeneratedColumnRefAutoInc, } terror.ErrClassToMySQLCodes[terror.ClassDDL] = ddlMySQLErrCodes diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 9b17e120ec..812405b536 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -38,6 +38,7 @@ import ( "github.com/pingcap/tidb/table" "github.com/pingcap/tidb/table/tables" "github.com/pingcap/tidb/types" + "github.com/pingcap/tidb/types/parser_driver" "github.com/pingcap/tidb/util/mock" "github.com/pingcap/tidb/util/schemautil" "github.com/pingcap/tidb/util/set" @@ -478,13 +479,27 @@ func columnDefToCol(ctx sessionctx.Context, offset int, colDef *ast.ColumnDef, o return col, constraints, nil } -func getDefaultValue(ctx sessionctx.Context, c *ast.ColumnOption, t *types.FieldType) (interface{}, error) { +func getDefaultValue(ctx sessionctx.Context, colName string, c *ast.ColumnOption, t *types.FieldType) (interface{}, error) { tp, fsp := t.Tp, t.Decimal if tp == mysql.TypeTimestamp || tp == mysql.TypeDatetime { + switch x := c.Expr.(type) { + case *ast.FuncCallExpr: + if x.FnName.L == ast.CurrentTimestamp { + defaultFsp := 0 + if len(x.Args) == 1 { + if val := x.Args[0].(*driver.ValueExpr); val != nil { + defaultFsp = int(val.GetInt64()) + } + } + if defaultFsp != fsp { + return nil, ErrInvalidDefaultValue.GenWithStackByArgs(colName) + } + } + } vd, err := expression.GetTimeValue(ctx, c.Expr, tp, fsp) value := vd.GetValue() if err != nil { - return nil, errors.Trace(err) + return nil, ErrInvalidDefaultValue.GenWithStackByArgs(colName) } // Value is nil means `default null`. @@ -1696,7 +1711,7 @@ func (d *ddl) AddColumn(ctx sessionctx.Context, ti ast.Ident, spec *ast.AlterTab // If new column is a generated column, do validation. // NOTE: Because now we can only append columns to table, - // we dont't need check whether the column refers other + // we don't need check whether the column refers other // generated columns occurring later in table. for _, option := range specNewColumn.Options { if option.Tp == ast.ColumnOptionGenerated { @@ -1718,7 +1733,7 @@ func (d *ddl) AddColumn(ctx sessionctx.Context, ti ast.Ident, spec *ast.AlterTab return ErrTooLongIdent.GenWithStackByArgs(colName) } - // Ingore table constraints now, maybe return error later. + // Ignore table constraints now, maybe return error later. // We use length(t.Cols()) as the default offset firstly, we will change the // column's offset later. col, _, err = buildColumnAndConstraint(ctx, len(t.Cols()), specNewColumn, nil, t.Meta().Charset, schema.Charset) @@ -2020,9 +2035,9 @@ func modifiable(origin *types.FieldType, to *types.FieldType) error { func setDefaultValue(ctx sessionctx.Context, col *table.Column, option *ast.ColumnOption) (bool, error) { hasDefaultValue := false - value, err := getDefaultValue(ctx, option, &col.FieldType) + value, err := getDefaultValue(ctx, col.Name.L, option, &col.FieldType) if err != nil { - return hasDefaultValue, ErrColumnBadNull.GenWithStack("invalid default value - %s", err) + return hasDefaultValue, errors.Trace(err) } if hasDefaultValue, value, err = checkColumnDefaultValue(ctx, col, value); err != nil { diff --git a/executor/ddl_test.go b/executor/ddl_test.go index fa12114c46..0216b50214 100644 --- a/executor/ddl_test.go +++ b/executor/ddl_test.go @@ -685,3 +685,43 @@ func (s *testSuite3) TestIssue9205(c *C) { ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin", )) } + +func (s *testSuite3) TestCheckDefaultFsp(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec(`drop table if exists t;`) + + _, err := tk.Exec("create table t ( tt timestamp default now(1));") + c.Assert(err.Error(), Equals, "[ddl:1067]Invalid default value for 'tt'") + + _, err = tk.Exec("create table t ( tt timestamp(1) default current_timestamp);") + c.Assert(err.Error(), Equals, "[ddl:1067]Invalid default value for 'tt'") + + _, err = tk.Exec("create table t ( tt timestamp(1) default now(2));") + c.Assert(err.Error(), Equals, "[ddl:1067]Invalid default value for 'tt'") + + tk.MustExec("create table t ( tt timestamp(1) default now(1));") + tk.MustExec("create table t2 ( tt timestamp default current_timestamp());") + tk.MustExec("create table t3 ( tt timestamp default current_timestamp(0));") + + _, err = tk.Exec("alter table t add column ttt timestamp default now(2);") + c.Assert(err.Error(), Equals, "[ddl:1067]Invalid default value for 'ttt'") + + _, err = tk.Exec("alter table t add column ttt timestamp(5) default current_timestamp;") + c.Assert(err.Error(), Equals, "[ddl:1067]Invalid default value for 'ttt'") + + _, err = tk.Exec("alter table t add column ttt timestamp(5) default now(2);") + c.Assert(err.Error(), Equals, "[ddl:1067]Invalid default value for 'ttt'") + + _, err = tk.Exec("alter table t modify column tt timestamp(1) default now();") + c.Assert(err.Error(), Equals, "[ddl:1067]Invalid default value for 'tt'") + + _, err = tk.Exec("alter table t modify column tt timestamp(4) default now(5);") + c.Assert(err.Error(), Equals, "[ddl:1067]Invalid default value for 'tt'") + + _, err = tk.Exec("alter table t change column tt tttt timestamp(4) default now(5);") + c.Assert(err.Error(), Equals, "[ddl:1067]Invalid default value for 'tttt'") + + _, err = tk.Exec("alter table t change column tt tttt timestamp(1) default now();") + c.Assert(err.Error(), Equals, "[ddl:1067]Invalid default value for 'tttt'") +}