-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
VSchema DDL: Add grammar to accept qualified table names in Vindex option values #12577
Conversation
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
If a new flag is being introduced:
If a workflow is added or modified:
Bug fixes
Non-trivial changes
New/Existing features
Backward compatibility
|
👋 This should not impact any reasonable existing usage of the VSchema DDL, and only fix a previously-broken syntax error. As such, I would like to kindly request a backport of this issue (similarly and perhaps bundle with #12519) |
If not the upcoming |
I think this should be part of the upcoming |
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.
Looks good to me, but I'll let @GuptaManan100 manan have a final say on the parser change.
The GH UI is telling me that there are some conflicts though: This branch has conflicts that must be resolved
, would you mind fixing those?
855520d
to
7e23ac0
Compare
Good morning/afternoon, thanks for taking a look and including this in the upcoming release! I have fixed the merge conflict on the codegen file. 🤞 for CI |
Hello @hkdsun, there are some more conflicts in |
Signed-off-by: Hormoz Kheradmand <[email protected]>
7e23ac0
to
cb7173c
Compare
I was unable to backport this Pull Request to the following branches: |
Signed-off-by: Hormoz Kheradmand <[email protected]> Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Hormoz Kheradmand <[email protected]> Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Hormoz Kheradmand <[email protected]> Signed-off-by: Florent Poinsard <[email protected]> Co-authored-by: Hormoz Kheradmand <[email protected]>
Signed-off-by: Hormoz Kheradmand <[email protected]> Signed-off-by: Florent Poinsard <[email protected]> Co-authored-by: Hormoz Kheradmand <[email protected]>
Description
In a similar spirit as #12519, we have run into another bug handling keyspace names with dashes
-
in them.This time, while applying VSchema DDL statements such as:
Currently, the above is a syntax error, since everything after
WITH
is parsed as areserved_sql_id
/STRING
/INTEGRAL
.This PR introduces
table_id '.' reserved_table_id
as valid syntax.I chose this over a blanket
table_name
token since it's a narrower change, but I'm happy to go either way with it. We will be using fully-qualified table names with keyspace names.Checklist
Deployment Notes