[GitHub] [shardingsphere] tuichenchuxin commented on a diff in pull request #18045: parse aggregationProjectionSegment with expressionSegment

[GitBox <gi...@apache.org>]

tuichenchuxin commented on code in PR #18045:
URL: https://github.com/apache/shardingsphere/pull/18045#discussion_r885139882


##########
shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/postgresql/visitor/statement/impl/PostgreSQLStatementSQLVisitor.java:
##########
@@ -504,10 +504,22 @@ public ASTNode visitBExpr(final BExprContext ctx) {
     private ProjectionSegment createAggregationSegment(final FuncApplicationContext ctx, final String aggregationType) {
         AggregationType type = AggregationType.valueOf(aggregationType.toUpperCase());
         String innerExpression = ctx.start.getInputStream().getText(new Interval(ctx.LP_().getSymbol().getStartIndex(), ctx.stop.getStopIndex()));
-        if (null == ctx.DISTINCT()) {
-            return new AggregationProjectionSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), type, innerExpression);
+        if (null != ctx.DISTINCT()) {
+            AggregationDistinctProjectionSegment distinctProjectionSegment = new AggregationDistinctProjectionSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(),
+                    type, innerExpression, getDistinctExpression(ctx));
+            distinctProjectionSegment.getParameters().addAll(getExpressions(ctx));
+            return distinctProjectionSegment;
         }
-        return new AggregationDistinctProjectionSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), type, innerExpression, getDistinctExpression(ctx));
+        AggregationProjectionSegment projectionSegment = new AggregationProjectionSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), type, innerExpression);
+        projectionSegment.getParameters().addAll(getExpressions(ctx));
+        return projectionSegment;
+    }
+    
+    private Collection<ExpressionSegment> getExpressions(final FuncApplicationContext ctx) {
+        if (null == ctx.funcArgList()) {
+            return Collections.emptyList();
+        }
+        return Collections.singletonList((ExpressionSegment) visit(ctx.funcArgList()));

Review Comment:
   argList could return multi expressionSegments. We should implement visitArgList method.



##########
shardingsphere-test/shardingsphere-parser-test/src/main/resources/case/dml/select-aggregate.xml:
##########
@@ -26,6 +26,15 @@
         </projections>
     </select>
 
+    <select sql-case-id="select_sum_column">
+        <from>
+            <simple-table name="t_order" start-index="48" stop-index="54" />
+        </from>
+        <projections start-index="7" stop-index="41">
+            <aggregation-projection type="SUM" inner-expression="(t_order.user_id)" alias="user_id_sum" start-index="7" stop-index="26" />

Review Comment:
   Maybe we need to extract params and judge.



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org