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
[multistage][bugfix] improve sort copy rule #12237
Conversation
logical sort should not be copied when there's no fetch or fetch limit is too large
@@ -42,6 +42,7 @@ public class PinotSortExchangeCopyRule extends RelRule<RelRule.Config> { | |||
|
|||
public static final PinotSortExchangeCopyRule SORT_EXCHANGE_COPY = | |||
PinotSortExchangeCopyRule.Config.DEFAULT.toRule(); | |||
private static final int DEFAULT_SORT_EXCHANGE_COPY_THRESHOLD = 10_000; |
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.
10_000 is a reasonable heuristic until we get a config/hint.
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.
is it worth to make this configurable as a query param?
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.
probably. but i haven't decided whether to use a hint or use a query param. so for now i will keep this as a default and if we need to support configuration we will do that then.
241918e
to
9f8b95b
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #12237 +/- ##
============================================
- Coverage 61.53% 61.52% -0.02%
Complexity 207 207
============================================
Files 2416 2416
Lines 131177 131181 +4
Branches 20245 20246 +1
============================================
- Hits 80717 80704 -13
- Misses 44570 44585 +15
- Partials 5890 5892 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Co-authored-by: Rong Rong <rongr@startree.ai>
logical sort should not be copied when there's no fetch or fetch limit is too large.
this solves part 1 of #12228
follow up separately later