-
Notifications
You must be signed in to change notification settings - Fork 18
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.
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
adding support for PARTITION
syntax for table creation
#193
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, nice work. Just a couple comments
} | ||
|
||
any_keyword: | ||
ID |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ID doesn't belong here, it's not a keyword
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed to be any_identifier
instead
{ | ||
$$ = $2 | ||
$$.Options = $4 | ||
$$.Options = $4 + $5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need a space between these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a space here makes it so that tests that don't have a partition option have a dangling space afterwards, which causes a bunch of test failures.
I could add another member variable PartitionOptions
to separate the two, and make the Format()
output look better.
I don't think this matters too much though, since we're not using Options
anywhere outside of just testing
|
||
// partition options | ||
{ | ||
input: "create table t (\n" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need some tests that combine partitions with other table options
Parsing
table_options
for statements likeCREATE TABLE (column_definitions) table_options
better matches MySQLCan parse
partition_options
based off of https://dev.mysql.com/doc/refman/8.0/en/create-table.htmlFix for: dolthub/dolt#4358