From 00cf04a7aa2ff204829fa561dbd09f0a0b69697d Mon Sep 17 00:00:00 2001 From: wjHuang Date: Tue, 5 Jan 2021 13:45:45 +0800 Subject: [PATCH 1/4] cherry pick #22154 to release-4.0 Signed-off-by: ti-srebot --- ddl/db_test.go | 12 ++++++++++++ ddl/generated_column.go | 9 +++++++++ expression/builtin.go | 17 ++++++++++++++++- 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index e97d13fe67b28..ce9431738775c 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -2952,6 +2952,10 @@ func (s *testDBSuite3) TestGeneratedColumnDDL(c *C) { " `full_name` varchar(255) GENERATED ALWAYS AS (concat(`last_name`, _utf8mb4' ', `first_name`)) VIRTUAL\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) + // Test incorrect parameter count. + tk.MustGetErrCode("create table test_gv_incorrect_pc(a double, b int as (lower(a, 2)))", errno.ErrWrongParamcountToNativeFct) + tk.MustGetErrCode("create table test_gv_incorrect_pc(a double, b int as (lower(a, 2)) stored)", errno.ErrWrongParamcountToNativeFct) + genExprTests := []struct { stmt string err int @@ -2982,6 +2986,14 @@ func (s *testDBSuite3) TestGeneratedColumnDDL(c *C) { // Add stored generated column through alter table. {`alter table test_gv_ddl add column d int as (b+2) stored`, errno.ErrUnsupportedOnGeneratedColumn}, {`alter table test_gv_ddl modify column b int as (a + 8) stored`, errno.ErrUnsupportedOnGeneratedColumn}, + + // Add generated column with incorrect parameter count. + {`alter table test_gv_ddl add column z int as (lower(a, 2))`, errno.ErrWrongParamcountToNativeFct}, + {`alter table test_gv_ddl add column z int as (lower(a, 2)) stored`, errno.ErrWrongParamcountToNativeFct}, + + // Modify generated column with incorrect parameter count. + {`alter table test_gv_ddl modify column b int as (lower(a, 2))`, errno.ErrWrongParamcountToNativeFct}, + {`alter table test_gv_ddl change column b b int as (lower(a, 2))`, errno.ErrWrongParamcountToNativeFct}, } for _, tt := range genExprTests { s.tk.MustGetErrCode(tt.stmt, tt.err) diff --git a/ddl/generated_column.go b/ddl/generated_column.go index 915ae5e8e1252..eccb3036118f3 100644 --- a/ddl/generated_column.go +++ b/ddl/generated_column.go @@ -229,6 +229,7 @@ type illegalFunctionChecker struct { hasIllegalFunc bool hasAggFunc bool hasWindowFunc bool + otherErr error } func (c *illegalFunctionChecker) Enter(inNode ast.Node) (outNode ast.Node, skipChildren bool) { @@ -240,6 +241,11 @@ func (c *illegalFunctionChecker) Enter(inNode ast.Node) (outNode ast.Node, skipC c.hasIllegalFunc = true return inNode, true } + err := expression.VerifyArgsWrapper(node.FnName.L, len(node.Args)) + if err != nil { + c.otherErr = err + return inNode, true + } case *ast.SubqueryExpr, *ast.ValuesExpr, *ast.VariableExpr: // Subquery & `values(x)` & variable is not allowed c.hasIllegalFunc = true @@ -283,6 +289,9 @@ func checkGeneratedWithAutoInc(tableInfo *model.TableInfo, newColumnDef *ast.Col if err := checkAutoIncrementRef(newColumnDef.Name.Name.L, dependColNames, tableInfo); err != nil { return errors.Trace(err) } + if c.otherErr != nil { + return c.otherErr + } return nil } diff --git a/expression/builtin.go b/expression/builtin.go index eccdf0817025a..714e733df4e1a 100644 --- a/expression/builtin.go +++ b/expression/builtin.go @@ -539,17 +539,32 @@ type baseFunctionClass struct { } func (b *baseFunctionClass) verifyArgs(args []Expression) error { - l := len(args) + return b.verifyArgsByCount(len(args)) +} + +func (b *baseFunctionClass) verifyArgsByCount(l int) error { if l < b.minArgs || (b.maxArgs != -1 && l > b.maxArgs) { return ErrIncorrectParameterCount.GenWithStackByArgs(b.funcName) } return nil } +// VerifyArgsWrapper verifies a function by its name and the count of the arguments. +// Note that this function assumes that the function is supported. +func VerifyArgsWrapper(name string, l int) error { + f, ok := funcs[name] + if !ok { + return nil + } + return f.verifyArgsByCount(l) +} + // functionClass is the interface for a function which may contains multiple functions. type functionClass interface { // getFunction gets a function signature by the types and the counts of given arguments. getFunction(ctx sessionctx.Context, args []Expression) (builtinFunc, error) + // verifyArgsByCount verifies the count of parameters. + verifyArgsByCount(l int) error } // funcs holds all registered builtin functions. When new function is added, From 4d19101c630ee5e18c1a82712d2c3b959a0cfe58 Mon Sep 17 00:00:00 2001 From: Yuanjia Zhang Date: Tue, 26 Jan 2021 19:52:53 +0800 Subject: [PATCH 2/4] fix CI --- ddl/generated_column.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/ddl/generated_column.go b/ddl/generated_column.go index a5ce193c16a1e..76cb8af5dea2f 100644 --- a/ddl/generated_column.go +++ b/ddl/generated_column.go @@ -291,18 +291,6 @@ func checkIllegalFn4GeneratedColumn(colName string, expr ast.ExprNode) error { return nil } -// Check whether newColumnDef refers to any auto-increment columns. -func checkGeneratedWithAutoInc(tableInfo *model.TableInfo, newColumnDef *ast.ColumnDef) error { - _, dependColNames := findDependedColumnNames(newColumnDef) - if err := checkAutoIncrementRef(newColumnDef.Name.Name.L, dependColNames, tableInfo); err != nil { - return errors.Trace(err) - } - if c.otherErr != nil { - return c.otherErr - } - return nil -} - func checkIndexOrStored(tbl table.Table, oldCol, newCol *table.Column) error { if oldCol.GeneratedExprString == newCol.GeneratedExprString { return nil From 863e9bb09283dea03eed8bcd1f81bc71ad6a9218 Mon Sep 17 00:00:00 2001 From: Yuanjia Zhang Date: Wed, 27 Jan 2021 11:13:26 +0800 Subject: [PATCH 3/4] fix CI --- ddl/db_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 125f65da3e715..dff64760cbcf6 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -2953,8 +2953,8 @@ func (s *testDBSuite3) TestGeneratedColumnDDL(c *C) { ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) // Test incorrect parameter count. - tk.MustGetErrCode("create table test_gv_incorrect_pc(a double, b int as (lower(a, 2)))", errno.ErrWrongParamcountToNativeFct) - tk.MustGetErrCode("create table test_gv_incorrect_pc(a double, b int as (lower(a, 2)) stored)", errno.ErrWrongParamcountToNativeFct) + s.tk.MustGetErrCode("create table test_gv_incorrect_pc(a double, b int as (lower(a, 2)))", errno.ErrWrongParamcountToNativeFct) + s.tk.MustGetErrCode("create table test_gv_incorrect_pc(a double, b int as (lower(a, 2)) stored)", errno.ErrWrongParamcountToNativeFct) genExprTests := []struct { stmt string From 30b41e4a88f75c615fe4dff3a06ac27b7eb289d3 Mon Sep 17 00:00:00 2001 From: Yuanjia Zhang Date: Wed, 27 Jan 2021 13:31:55 +0800 Subject: [PATCH 4/4] fix CI --- ddl/generated_column.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ddl/generated_column.go b/ddl/generated_column.go index 76cb8af5dea2f..0c779f769e53c 100644 --- a/ddl/generated_column.go +++ b/ddl/generated_column.go @@ -288,6 +288,9 @@ func checkIllegalFn4GeneratedColumn(colName string, expr ast.ExprNode) error { if c.hasWindowFunc { return errWindowInvalidWindowFuncUse } + if c.otherErr != nil { + return c.otherErr + } return nil }