ddl: enhance validation of column names when creating table #6349

ciscoxll · 2018-04-23T12:39:01Z

Check the table name that are included in the field is it consistent with the create table name
respond issues #6330

mysql:
mysql> drop table if exists t; create table t(c1.c2 blob default null); insert into t values(); select * from t;
Query OK, 0 rows affected, 1 warning (0.00 sec)

ERROR 1103 (42000): Incorrect table name 'c1'
ERROR 1146 (42S02): Table 'test.t' doesn't exist
ERROR 1146 (42S02): Table 'test.t' doesn't exist

tidb:
tidb> drop table if exists t; create table t(c1.c2 blob default null); insert into t values(); select * from t;
Query OK, 0 rows affected (0.00 sec)

ERROR 1103 (42000): Incorrect table name 'c1'
ERROR 1146 (42S02): Table 'test.t' doesn't exist
ERROR 1146 (42S02): Table 'test.t' doesn't exist

…ment

ciscoxll · 2018-04-23T14:20:11Z

@winkyao @zimulala @XuHuaiyu PTAL

shenli · 2018-04-23T17:30:34Z

/run-all-tests

shenli · 2018-04-23T17:31:35Z

ddl/ddl_api.go

@@ -536,6 +536,15 @@ func checkTooManyColumns(colDefs []*ast.ColumnDef) error {
 	return nil
 }

+func checkContainDotColumn(tableName string, colDefs []*ast.ColumnDef) error {


Add comments for the function.

zimulala · 2018-04-24T03:42:21Z

ddl/ddl_api.go

@@ -536,6 +536,16 @@ func checkTooManyColumns(colDefs []*ast.ColumnDef) error {
 	return nil
 }

+// checkContainDotColumns check field contains the table name.


s/check/checks

zimulala · 2018-04-24T04:46:33Z

ddl/ddl_api.go

@@ -536,6 +536,16 @@ func checkTooManyColumns(colDefs []*ast.ColumnDef) error {
 	return nil
 }

+// checkContainDotColumns check field contains the table name.
+func checkContainDotColumns(tableName string, colDefs []*ast.ColumnDef) error {


Could we check it in the function of Preprocess?

ciscoxll · 2018-04-24T11:48:25Z

/run-all-tests

ciscoxll · 2018-04-25T02:23:08Z

@winkyao @zimulala @XuHuaiyu PTAL

zz-jason · 2018-04-25T02:23:29Z

plan/preprocess.go

@@ -462,6 +467,16 @@ func isIncorrectName(name string) bool {
 	return false
 }

+// checksContainDotColumns check field contains the table name.
+func checksContainDotColumns(tableName string, colDefs []*ast.ColumnDef) (string, bool) {


s/checksContainDotColumns/checkContainDotColumn/

make this function as a member function of preprocessor and we can set p.err inside this function, thus we can only return a bool value to indicate whether there is a column whose name contains a dot symbol.

XuHuaiyu · 2018-04-25T02:40:54Z

plan/preprocess.go

@@ -462,6 +467,16 @@ func isIncorrectName(name string) bool {
 	return false
 }

+// checksContainDotColumns check field contains the table name.


This comment is not that explicit, better put an example here?

XuHuaiyu · 2018-04-25T02:47:22Z

executor/executor_test.go

+	tk.MustExec("drop table if exists test.t1")
+	tk.MustExec("create table test.t1(t1.a char)")
+	tk.MustExec("drop table if exists t2")
+	tk.MustExec("create table t2(a char, t2.b int)")


add test for
create table t(s.a char);

Already add.

winkyao · 2018-04-25T02:55:20Z

plan/preprocess.go

@@ -224,6 +224,11 @@ func (p *preprocessor) checkCreateTableGrammar(stmt *ast.CreateTableStmt) {
 		return
 	}

+	if tableName, ok := checksContainDotColumns(tName, stmt.Cols); ok {


add some comments

winkyao · 2018-04-25T02:56:06Z

Please address comments

zz-jason

LGTM

winkyao · 2018-04-25T08:43:21Z

plan/preprocess.go

@@ -52,6 +52,7 @@ func (p *preprocessor) Enter(in ast.Node) (out ast.Node, skipChildren bool) {
 	case *ast.CreateTableStmt:
 		p.inCreateOrDropTable = true
 		p.checkCreateTableGrammar(node)
+		p.checkContainDotColumn(node)


call it in p.checkCreateTableGrammar is better.

ciscoxll · 2018-04-25T09:24:59Z

@winkyao @zimulala @XuHuaiyu PTAL

ciscoxll · 2018-04-25T09:25:37Z

/run-all-tests

XuHuaiyu

LGTM

ciscoxll · 2018-04-25T10:33:09Z

/run-all-tests

zimulala · 2018-04-25T10:36:32Z

plan/preprocess.go

+	for _, colDef := range stmt.Cols {
+		// check table name.
+		if colDef.Name.Table.O != tName && len(colDef.Name.Table.O) != 0 {
+			p.err = ddl.ErrWrongTableName.GenByArgs(colDef.Name.Table.O)


I think we can break here.

ciscoxll · 2018-04-25T15:28:33Z

@winkyao @zimulala PTAL

ddl:Missed extra validation for column with dot in create table state…

15ae503

…ment

ciscoxll changed the title ~~ddl:Missed extra validation for column with dot in create table state…~~ ddl:missed extra validation for column with dot in create table state… Apr 23, 2018

shenli added the DDL label Apr 23, 2018

shenli reviewed Apr 23, 2018

View reviewed changes

add comments for the function

0415a4c

ciscoxll force-pushed the check-column branch from 97c074b to 0415a4c Compare April 24, 2018 03:25

zimulala reviewed Apr 24, 2018

View reviewed changes

Address comments

6af644b

ciscoxll force-pushed the check-column branch from e4e464a to 6af644b Compare April 24, 2018 07:45

ciscoxll added the all-tests-passed label Apr 24, 2018

zz-jason reviewed Apr 25, 2018

View reviewed changes

XuHuaiyu reviewed Apr 25, 2018

View reviewed changes

winkyao reviewed Apr 25, 2018

View reviewed changes

ciscoxll removed the all-tests-passed label Apr 25, 2018

Address comments

6ff30ae

ciscoxll force-pushed the check-column branch from 82e601c to 6ff30ae Compare April 25, 2018 08:30

zz-jason reviewed Apr 25, 2018

View reviewed changes

winkyao reviewed Apr 25, 2018

View reviewed changes

move function

1897d89

ciscoxll added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 25, 2018

XuHuaiyu reviewed Apr 25, 2018

View reviewed changes

ciscoxll changed the title ~~ddl:missed extra validation for column with dot in create table state…~~ ddl: enhance validation of column names when creating table Apr 25, 2018

ciscoxll added the status/LGT2 Indicates that a PR has LGTM 2. label Apr 25, 2018

ciscoxll removed the status/LGT1 Indicates that a PR has LGTM 1. label Apr 25, 2018

zimulala reviewed Apr 25, 2018

View reviewed changes

add return

c341358

ciscoxll force-pushed the check-column branch from 6d7b36b to c341358 Compare April 25, 2018 11:25

ciscoxll added the all-tests-passed label Apr 25, 2018

coocood approved these changes Apr 26, 2018

View reviewed changes

coocood and others added 3 commits April 26, 2018 14:59

Merge branch 'master' into check-column

51863b8

Merge branch 'master' into check-column

3c4c4bc

Merge branch 'master' into check-column

62a825e

ciscoxll merged commit 284e70d into pingcap:master Apr 26, 2018

ciscoxll deleted the check-column branch April 26, 2018 11:00

sre-bot added the contribution This PR is from a community contributor. label Dec 18, 2019

you06 added the sig/sql-infra SIG: SQL Infra label Mar 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: enhance validation of column names when creating table #6349

ddl: enhance validation of column names when creating table #6349

ciscoxll commented Apr 23, 2018 •

edited

Loading

ciscoxll commented Apr 23, 2018 •

edited

Loading

shenli commented Apr 23, 2018

shenli Apr 23, 2018

zimulala Apr 24, 2018

zimulala Apr 24, 2018

ciscoxll commented Apr 24, 2018

ciscoxll commented Apr 25, 2018

zz-jason Apr 25, 2018

XuHuaiyu Apr 25, 2018

XuHuaiyu Apr 25, 2018

ciscoxll Apr 25, 2018

winkyao Apr 25, 2018

winkyao commented Apr 25, 2018

zz-jason left a comment

winkyao Apr 25, 2018

ciscoxll commented Apr 25, 2018

ciscoxll commented Apr 25, 2018

XuHuaiyu left a comment

ciscoxll commented Apr 25, 2018

zimulala Apr 25, 2018

ciscoxll commented Apr 25, 2018

ddl: enhance validation of column names when creating table #6349

ddl: enhance validation of column names when creating table #6349

Conversation

ciscoxll commented Apr 23, 2018 • edited Loading

ciscoxll commented Apr 23, 2018 • edited Loading

shenli commented Apr 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ciscoxll commented Apr 24, 2018

ciscoxll commented Apr 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winkyao commented Apr 25, 2018

zz-jason left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ciscoxll commented Apr 25, 2018

ciscoxll commented Apr 25, 2018

XuHuaiyu left a comment

Choose a reason for hiding this comment

ciscoxll commented Apr 25, 2018

Choose a reason for hiding this comment

ciscoxll commented Apr 25, 2018

ciscoxll commented Apr 23, 2018 •

edited

Loading

ciscoxll commented Apr 23, 2018 •

edited

Loading