[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
|
|
||
| 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.
10_000 is a reasonable heuristic until we get a config/hint.
There was a problem hiding this comment.
is it worth to make this configurable as a query param?
There was a problem hiding this comment.
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.
walterddr
force-pushed
the
bugfix_skip_local_sort
branch
from
241918e to
9f8b95b
Compare
Codecov Report✅ All 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. 🚀 New features to boost your workflow:
|
Recently, we found cases where users were using very large limit, and this change backfired us. What is the advantage of removing the sort when the limit is larger than 10k? |
4 participants
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