-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
parser: use alter table remove ttl spec #39341
parser: use alter table remove ttl spec #39341
Conversation
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
108a90a
to
cd6fe00
Compare
Signed-off-by: YangKeao <[email protected]>
cd6fe00
to
885cfa5
Compare
@lcwangchao @hawkingrei Could you help me to merge this PR? |
/merge |
This pull request has been accepted and is ready to merge. Commit hash: 9cc5dc0
|
TiDB MergeCI notify✅ Well Done! New fixed [1] after this pr merged.
|
Signed-off-by: YangKeao [email protected]
What problem does this PR solve?
Issue Number: close #39340
Problem Summary:
Use
alter table remove ttl
instead ofalter table no_ttl
. And remove the support of arbitrary expression, but only support valueExpr from Literal.What is changed and how it works?
It changed three things:
AlterTablePartitionOpt
toAlterTableSingleSpecOpt
and add a"REMOVE" "TTL"
branch to it. We cannot add"REMOVE" "TTL"
to other rules orAlterTableSpec
becauseyacc
has to know whether to reduce theAlterTableSpec
and moving forward toAlterTablePartitionOpt
, or try to"REMOVE" "TTL"
valueExpr
. Maybe we could modify it toDefaultValueExpr
or other wider range grammar later, but supporing number / string literal is enough for today.Check List
Tests
Release note