xiangfu0 commented on code in PR #19988:
URL: https://github.com/apache/pinot/pull/19988#discussion_r4407458336


##########
pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java:
##########
@@ -1308,6 +1308,16 @@ private static Expression 
compileFunctionExpression(SqlBasicCall functionNode) {
         operands.add(toExpression(childNode));
       }
     }
+    if (functionKind == SqlKind.ARRAY_AGG && 
functionNode.getFunctionQuantifier() != null
+        && "DISTINCT".equals(functionNode.getFunctionQuantifier().toString())) 
{
+      Expression distinct = RequestUtils.getLiteralExpression(true);
+      if (operands.size() == 1 || (operands.size() == 2 && 
operands.get(1).isSetLiteral()
+          && operands.get(1).getLiteral().isSetStringValue())) {
+        operands.add(distinct);
+      } else if (operands.size() == 2 || operands.size() == 3) {
+        operands.set(operands.size() - 1, distinct);

Review Comment:
   Fixed in 7ce4dcb5dc: replace only boolean literals, otherwise append true 
for two operands and preserve three-operand calls. Added parser regressions for 
all four examples; 137 focused tests pass.



##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotApproximateAggregateRewriteRule.java:
##########
@@ -101,29 +100,22 @@ public void onMatch(RelOptRuleCall call) {
     // indices of every call stay valid. With no parameters configured the 
plan keeps the shape it had before, with no
     // extra project.
     //
-    // Invariant: this project has to stay directly beneath the aggregate. 
Both PinotAggregateExchangeNodeInsertRule
-    // and AggregatePushdownRule inline the literal only through 
findImmediateProjects(input), so anything that puts a
-    // node between them would leave the leaf with a column reference where 
the function expects a parameter string.
-    // Nothing in the later phases does: AggregateProjectMergeRule needs an 
all-input-ref project and the literals
-    // block it, and ProjectMergeRule collapses into a single project that 
stays adjacent.
+    // Keep existing literal arguments visible to the aggregate split rules, 
including those projected in union inputs.

Review Comment:
   Updated in 7ce4dcb5dc: the comment now refers only to literals in the 
immediate project.



-- 
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]

Reply via email to