-
Notifications
You must be signed in to change notification settings - Fork 919
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 #5377][FOLLOWUP] Get limit from more spark plan and regard result max rows #5963
Conversation
ba264ff
to
367622b
Compare
367622b
to
0f734ee
Compare
.../kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/sql/kyuubi/SparkDatasetHelper.scala
Outdated
Show resolved
Hide resolved
.../kyuubi-spark-sql-engine/src/main/scala/org/apache/spark/sql/kyuubi/SparkDatasetHelper.scala
Outdated
Show resolved
Hide resolved
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, one minor suggestion
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #5963 +/- ##
============================================
- Coverage 61.27% 61.04% -0.23%
Complexity 23 23
============================================
Files 622 623 +1
Lines 36882 37150 +268
Branches 5014 5035 +21
============================================
+ Hits 22599 22679 +80
- Misses 11854 12014 +160
- Partials 2429 2457 +28 ☔ View full report in Codecov by Sentry. |
logical plan can not cover the case likes:
So, I revert to use sparkPlan. |
@turboFei can we use optimizedPlan ? |
c97a72e
to
196845a
Compare
addressed. @ulysses-you |
196845a
to
223d510
Compare
thanks, merged to master |
…ard result max rows # 🔍 Description Followup apache#5591 Support to get existing limit from more plan and regard the result max rows. ## Issue References 🔗 This pull request fixes # ## Describe Your Solution 🔧 Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change. ## Types of changes 🔖 - [ ] Bugfix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 #### Behavior Without This Pull Request ⚰️ #### Behavior With This Pull Request 🎉 #### Related Unit Tests --- # Checklist 📝 - [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) **Be nice. Be informative.** Closes apache#5963 from turboFei/incremental_save. Closes apache#5377 223d510 [Fei Wang] use optimized plan ecefc2a [Fei Wang] use spark plan 57091e5 [Fei Wang] minor 2096144 [Fei Wang] for logical plan 0f734ee [Fei Wang] ut fdc1155 [Fei Wang] save f8e405a [Fei Wang] math.min Authored-by: Fei Wang <[email protected]> Signed-off-by: Fei Wang <[email protected]>
…ard result max rows # 🔍 Description Followup apache#5591 Support to get existing limit from more plan and regard the result max rows. ## Issue References 🔗 This pull request fixes # ## Describe Your Solution 🔧 Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change. ## Types of changes 🔖 - [ ] Bugfix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 #### Behavior Without This Pull Request ⚰️ #### Behavior With This Pull Request 🎉 #### Related Unit Tests --- # Checklist 📝 - [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) **Be nice. Be informative.** Closes apache#5963 from turboFei/incremental_save. Closes apache#5377 223d510 [Fei Wang] use optimized plan ecefc2a [Fei Wang] use spark plan 57091e5 [Fei Wang] minor 2096144 [Fei Wang] for logical plan 0f734ee [Fei Wang] ut fdc1155 [Fei Wang] save f8e405a [Fei Wang] math.min Authored-by: Fei Wang <[email protected]> Signed-off-by: Fei Wang <[email protected]>
🔍 Description
Followup #5591
Support to get existing limit from more plan and regard the result max rows.
Issue References 🔗
This pull request fixes #
Describe Your Solution 🔧
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Types of changes 🔖
Test Plan 🧪
Behavior Without This Pull Request ⚰️
Behavior With This Pull Request 🎉
Related Unit Tests
Checklist 📝
Be nice. Be informative.