xiangfu0 commented on code in PR #20134:
URL: https://github.com/apache/pinot/pull/20134#discussion_r4450813508
##########
pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/QueryRewriterFactory.java:
##########
@@ -38,10 +38,11 @@ private QueryRewriterFactory() {
// (reference) into the group-by list, but the alias should not be applied
to the reference.
// E.g. SELECT a + 1 AS a FROM table GROUP BY 1
// AggregationOptimizer should run early to optimize aggregation patterns
before other rewrites.
+ // OrderByLiteralRemover must run after OrdinalsUpdater so integer
ordinals are resolved before removing literals.
public static final List<String> DEFAULT_QUERY_REWRITERS_CLASS_NAMES =
List.of(CompileTimeFunctionsInvoker.class.getName(),
AggregationOptimizer.class.getName(),
SelectionsRewriter.class.getName(),
PredicateComparisonRewriter.class.getName(),
- AliasApplier.class.getName(), OrdinalsUpdater.class.getName(),
+ AliasApplier.class.getName(), OrdinalsUpdater.class.getName(),
OrderByLiteralRemover.class.getName(),
Review Comment:
Confirmed fixed at d3704c38df. `SelectionPlanNode` already enforces the
direction and sortedness invariant the removed checks guarded, so literal-only
and literal-prefix keys that bypass the rewriter now run correctly; I checked
this locally against normalized results. The retained `ORDER BY NULL` with null
handling off is documented in the description and the default pipeline removes
it, so I have no further concern here.
AI-generated review via IntentLab ยท Model: claude-opus-5-5 (Opus 5.5).
--
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]
