Jackie-Jiang commented on code in PR #19866:
URL: https://github.com/apache/pinot/pull/19866#discussion_r4300022651
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/SelectionPlanNode.java:
##########
@@ -169,7 +169,10 @@ private int
getSortedColumnsPrefix(List<OrderByExpressionContext> orderByExpress
private boolean isSorted(OrderByExpressionContext orderByExpression, boolean
asc, boolean isNullHandlingEnabled) {
switch (orderByExpression.getExpression().getType()) {
case LITERAL: {
- return true;
+ // LITERAL expressions (e.g. unresolved positional ORDER BY 2, or
ORDER BY 1) are not backed by a data source
+ // column and therefore cannot rely on the segment's natural sort
order. Returning false ensures these queries
+ // fall through to the generic SelectionOrderByOperator rather than
taking the partial-order optimized path.
+ return false;
Review Comment:
**MAJOR — Preserve early termination when a literal follows a sorted
identifier.** For `SELECT payload FROM t ORDER BY sorted_col, 'x' LIMIT 10`,
this unconditional `false` shortens the sorted prefix from two expressions to
one. Previously SelectionPlanNode capped projection at 10 documents and
TotallySortedListBuilder stopped at 10 rows. Now it uses
PartiallySortedListBuilder, which continues until sorted_col changes; on a
segment where the sorted column is constant, it reads and materializes all N
rows. The literal cannot distinguish those rows, so this introduces unnecessary
scanning for a previously working query. String literals survive ordinal
rewriting and query-context conversion, making this path reachable. Please
preserve literal suffixes once a sorted identifier has established the prefix,
or normalize constant ordering keys before selecting the operator. A regression
test with a large tied group should assert that the scan remains bounded by
LIMIT.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
