Jackie-Jiang commented on code in PR #20134:
URL: https://github.com/apache/pinot/pull/20134#discussion_r4450655555
##########
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:
Addressed in d3704c38df. Removed the constructor checks in both partially
ordered selection operators: the planner already establishes the sorted prefix
and compatible execution path, and a literal-only prefix does not require an
identifier or impose a direction. The original `now()`/non-null-literal failure
is therefore fixed for custom pipelines and requests from older brokers even
without `OrderByLiteralRemover`.
Added regressions that bypass literal removal and exercise both operators,
mixed literal/column/function keys, reverse scans, null-handling options, and
retained ordered `OFFSET` handling. The default rewrite still removes
unnecessary keys. Custom lists need to include the new rewriter to get that
normalization; I documented this in the description, along with the remaining
result-merging failure for retained `ORDER BY NULL` when null handling is
disabled.
--
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]
