Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

parser: support 'CREATE TABLE ... SELECT' syntax (#4754) #6851

Merged
merged 8 commits into from
Jun 22, 2018
Merged

parser: support 'CREATE TABLE ... SELECT' syntax (#4754) #6851

merged 8 commits into from
Jun 22, 2018

Conversation

bb7133
Copy link
Member

@bb7133 bb7133 commented Jun 16, 2018 •
edited
Loading

What have you changed? (mandatory)

This is the first part of PRs for issue #4754:

  • updated 'parser/parser.y'(as well as some parser related go code like ddl.go/preprocess.go...) to support 'CREATE TABLE ... SELECT' SQL syntax
  • the further implementations(plan/runtime) will be in next PRs, for now, a 'CREATE TABLE ... SELECT is not implemented' error will be returned in preprocess.go

What are the type of the changes (mandatory)?

  • Bug fix

How has this PR been tested (mandatory)?

Some new SQL statements are added in the tests to verify the parsed result

Does this PR affect documentation (docs/docs-cn) update? (optional)

Nope

Refer to a related PR or issue link (optional)

Issue 4754: #4754

Benchmark result if necessary (optional)

Add a few positive/negative examples (optional)

@sre-bot
Copy link
Contributor

sre-bot commented Jun 16, 2018

Hi contributor, thanks for your PR.

This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically.

@zz-jason zz-jason added the contribution This PR is from a community contributor. label Jun 19, 2018
@shenli
Copy link
Member

shenli commented Jun 19, 2018

/add-to-whitelist

@shenli
Copy link
Member

shenli commented Jun 19, 2018

/run-all-tests

@winkyao
Copy link
Contributor

winkyao commented Jun 19, 2018

@bb7133 Thanks to your contribution, I'll look into the failed test case.

winkyao
winkyao reviewed Jun 19, 2018
View reviewed changes
parser/parser.y Outdated
@@ -600,6 +600,7 @@ import (
ConstraintKeywordOpt "Constraint Keyword or empty"
CreateIndexStmtUnique "CREATE INDEX optional UNIQUE clause"
CreateTableOptionListOpt "create table option list opt"
CreateTableSelectOpt "Select/Union statement in CREATE TABLE ... SELECT"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep the indent.

parser/parser.y Outdated
@@ -653,6 +654,7 @@ import (
JoinType "join type"
KillOrKillTiDB "Kill or Kill TiDB"
LikeEscapeOpt "like escape option"
LikeTableWithOrWithoutParen "LIKE table_name or ( LIKE table_name )"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

winkyao
winkyao reviewed Jun 19, 2018
View reviewed changes
parser/parser.y
}
| "CREATE" "TABLE" IfNotExists TableName "LIKE" TableName
| "CREATE" "TABLE" IfNotExists TableName LikeTableWithOrWithoutParen
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add some test to LikeTableWithOrWithoutParen?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK~

@winkyao
Copy link
Contributor

winkyao commented Jun 20, 2018

/run-integration-common-test -tidb-test=pr/553

@winkyao
Copy link
Contributor

winkyao commented Jun 20, 2018

/run-common-test -tidb-test=pr/553

@winkyao
Copy link
Contributor

winkyao commented Jun 20, 2018

The failed internal tidb test case was fixed by tidb-test/pr/553, please address comments. @bb7133

winkyao
winkyao reviewed Jun 21, 2018
View reviewed changes
parser/parser.y Outdated
@@ -1876,6 +1868,57 @@ PartDefStorageOpt:
| "ENGINE" eq Identifier
{}

OnDuplicateCreateTableOpt:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/OnDuplicateCreateTableOpt/DuplicateOpt/g, MySQL name it opt_duplicate here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thanks!

winkyao
winkyao reviewed Jun 21, 2018
View reviewed changes
parser/parser.y
| "AS"
{}

CreateTableSelectOpt:
Copy link
Contributor

@winkyao winkyao Jun 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Miss '(' SelectStmt ')' ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is included in 'SubSelect', PTAL thanks

winkyao
winkyao reviewed Jun 21, 2018
View reviewed changes
Copy link
Contributor

@winkyao winkyao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reset LGTM

@winkyao
Copy link
Contributor

winkyao commented Jun 21, 2018

/run-all-tests -tidb-test=pr/553

1 similar comment
@winkyao
Copy link
Contributor

winkyao commented Jun 21, 2018

/run-all-tests -tidb-test=pr/553

@bb7133
parser: support 'CREATE TABLE ... SELECT' syntax (pingcap#4754)
8897eb9
@bb7133
fix golint complaint
09f95c0
@bb7133
fix a bug & add more tests
aa47231 
1. added tests for 'create table ... (like ...)' syntax
2. added more other tests for 'create table ... select' syntax
3. fixed a bug on 'TableMustHaveColumn'(MySQL errcode 1113) error
@bb7133
rename OnDuplicateCreateTableOpt to DuplicateOpt
8f4d0d2
@winkyao
Copy link
Contributor

winkyao commented Jun 22, 2018

/run-all-tests -tidb-test=pr/553

winkyao
winkyao reviewed Jun 22, 2018
View reviewed changes
Copy link
Contributor

@winkyao winkyao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM @coocood @zz-jason PTAL

@winkyao winkyao added the status/LGT1 Indicates that a PR has LGTM 1. label Jun 22, 2018
zz-jason
zz-jason approved these changes Jun 22, 2018
View reviewed changes
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zz-jason zz-jason added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jun 22, 2018
@zz-jason zz-jason merged commit 20c5675 into pingcap:master Jun 22, 2018
@bb7133 bb7133 deleted the bb7133/create_select branch September 8, 2018 09:26
@bb7133 bb7133 mentioned this pull request Sep 26, 2018
Closed
@bb7133 bb7133 mentioned this pull request Dec 13, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@winkyao winkyao winkyao left review comments

@zz-jason zz-jason zz-jason approved these changes

Assignees
No one assigned
Labels
component/parser contribution This PR is from a community contributor. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@bb7133 @sre-bot @shenli @winkyao @zz-jason @coocood