-
Notifications
You must be signed in to change notification settings - Fork 170
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
fix: Fall back to Spark for unsupported partition or sort expressions in window aggregates #1253
Conversation
"area", | ||
"product") | ||
df.createOrReplaceTempView("windowData") | ||
checkSparkAnswer(sql(""" |
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.
What about using checkSparkAnswerAndOperator
instead?
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.
With auto
and native
we fall back to Spark currently
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.
Let me see if I can conditionally use checkSparkAnswerAndOperator
for the jvm
shuffle case
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 updated the test. I added assertions to check for the expected shuffle exchanges and added some notes.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1253 +/- ##
============================================
- Coverage 34.88% 34.69% -0.19%
- Complexity 990 991 +1
============================================
Files 116 116
Lines 43755 44891 +1136
Branches 9569 9864 +295
============================================
+ Hits 15263 15574 +311
- Misses 25513 26168 +655
- Partials 2979 3149 +170 ☔ View full report in Codecov by Sentry. |
Thanks for the review @kazuyukitanimura |
Which issue does this PR close?
Closes #1249
Rationale for this change
Fix bug that causes Spark SQL tests to fail when we run with jvm shuffle mode.
What changes are included in this PR?
How are these changes tested?