xiangfu0 commented on code in PR #19988:
URL: https://github.com/apache/pinot/pull/19988#discussion_r4393863746
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java:
##########
@@ -610,10 +610,14 @@ private static HepProgram getOptProgram(PinotRuleSet
ruleSet, Set<String> skipRu
// Run the Calcite CORE rules using 1 HepInstruction per rule. We use 1
HepInstruction per rule for simplicity:
// the rules used here can rest assured that they are the only ones
evaluated in a dedicated graph-traversal.
for (RelOptRule relOptRule : basicRules) {
+ // Supplies the data type argument before AggregateCaseToFilter, which
rewrites only one-argument calls: its
+ // ARRAY_AGG(expression) FILTER (WHERE condition) form of ARRAY_AGG(CASE
WHEN ...) fails at runtime.
+ if
(relOptRule.toString().equals(CommonConstants.Broker.PlannerRuleNames.AGGREGATE_CASE_TO_FILTER))
{
+
hepProgramBuilder.addRuleInstance(PinotAggregateTypeArgumentRewriteRule.WITHOUT_ANY_VALUE);
Review Comment:
Fixed in 1a5b828a80: the early instance now rewrites only ARRAY_AGG, leaving
MODE and FIRST/LAST to the normal pass. Runtime tests cover MODE(CASE ...) with
and without skip-leaf grouping.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotRuleUtils.java:
##########
@@ -85,6 +89,45 @@ public static RelNode unboxRel(RelNode rel) {
}
}
+ /// Appends aggregate literal arguments without hiding literals already
projected beneath a union. If a union has
+ /// no projected literals, keeps one project above it; otherwise recurses
through nested unions to their projects.
+ public static RelNode appendAggregateLiterals(RelNode input, List<RexNode>
literals) {
+ return appendAggregateLiterals(input, literals, false);
+ }
+
+ private static RelNode appendAggregateLiterals(RelNode input, List<RexNode>
literals, boolean insideUnion) {
+ RelNode rel = unboxRel(input);
+ if (rel instanceof Union && (insideUnion || hasProjectedLiterals(rel))) {
Review Comment:
Fixed in 1a5b828a80 using the second approach: keep one project above the
union and lift a column to a literal only when every branch supplies the same
literal. Added tagged ANY_VALUE, branch-specific LAST_WITH_TIME, skip-leaf MODE
and approximate DISTINCTCOUNT/COVARPOP regression coverage; the full runtime
resource suite passes.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/AggregateFunctionRewriteOptimizer.java:
##########
@@ -112,6 +113,10 @@ private static void rewrite(@Nullable Expression
expression, Schema schema, bool
}
return;
}
+ if (normalizedName.equals(AggregationFunctionType.ARRAYAGG.name()) &&
isArrayAggWithoutDataType(function)) {
Review Comment:
Fixed in 1a5b828a80: SSE explicitly rejects DISTINCT on ARRAY_AGG rather
than dropping the quantifier. The optimizer test covers the one-argument,
false-flag and explicit-type forms, and all parser tests pass.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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]
