-
Notifications
You must be signed in to change notification settings - Fork 914
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
[KYUUBI #6562] Support zorder clause syntax to use brackets #6563
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6563 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 677 677
Lines 41907 41907
Branches 5721 5721
======================================
Misses 41907 41907 ☔ View full report in Codecov by Sentry. |
@@ -746,14 +746,25 @@ trait ZorderSuiteBase extends KyuubiSparkSQLExtensionTest with ExpressionEvalHel | |||
UnresolvedFunction("current_date", Seq.empty, false)), | |||
UnresolvedRelation(TableIdentifier("p"))))))) | |||
|
|||
// TODO: add following case support | |||
intercept[ParseException] { | |||
parser.parsePlan("OPTIMIZE p zorder by (c1)") |
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.
Does Spark SQL support ORDER BY
with parentheses
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.
No, Spark does not support it.
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.
then I think we don't need to support that too
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.
Got it, I just saw this todo in the comment.
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.
I think we can delete those code and comments, unless @ulysses-you and @cfmcgrady have other ideas
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.
Seems added by @cfmcgrady , I'm fine to just remove the comment
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.
Just following the deltalake. https://docs.delta.io/latest/optimizations-oss.html#z-ordering-multi-dimensional-clustering
🔍 Description
Issue References 🔗
This pull request fixes #6562
Describe Your Solution 🔧
KyuubiSparkSQL.g4
to support the zorder clause syntax to use bracketsTypes of changes 🔖
Test Plan 🧪
Behavior Without This Pull Request ⚰️
zorder clause syntax can't use brackets
Behavior With This Pull Request 🎉
zorder clause syntax can use brackets
Related Unit Tests
Checklist 📝
Be nice. Be informative.