github-actions[bot] commented on code in PR #65264:
URL: https://github.com/apache/doris/pull/65264#discussion_r3528093992


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/AggregateFunction.java:
##########
@@ -153,6 +153,22 @@ public String computeToSql() throws UnboundException {
         return sql.append(")").toString();
     }
 
+    @Override
+    public String shapeInfo() {
+        StringBuilder sql = new StringBuilder(getName()).append("(");
+        if (distinct) {
+            sql.append("DISTINCT ");
+        }
+        int arity = arity();
+        for (int i = 0; i < arity; i++) {
+            sql.append(child(i).shapeInfo());

Review Comment:
   This only fixes qualifier preservation when the aggregate is reached 
directly by `shapeInfo()`. In the same shape-plan predicate paths, wrappers 
still fall back to SQL rendering: `Cast` inherits `Expression.shapeInfo()` 
(`toSql()`), and `Cast.computeToSql()` calls `child().toSql()`, which reaches 
`AggregateFunction.computeToSql()` and uses each argument's `toSql()`. So a 
predicate like `cast(avg(t1.c11) as DOUBLE) > 0` still prints `cast(avg(c11) as 
DOUBLE)` and remains unstable in the way this PR is trying to fix for direct 
`avg(t1.c11)`. Please make the wrapper rendering recurse through child 
`shapeInfo()` where these expressions can appear in shape-plan predicates, and 
add a regression with a wrapped aggregate.



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/AggregateFunction.java:
##########
@@ -153,6 +153,22 @@ public String computeToSql() throws UnboundException {
         return sql.append(")").toString();
     }
 
+    @Override
+    public String shapeInfo() {

Review Comment:
   This base implementation loses the special `Count(*)` rendering. `Count` 
stores star-ness in the `isStar` flag and has zero children, then overrides 
`computeToSql()`, `toString()`, and `toDigest()` to print `count(*)`. With this 
new base `shapeInfo()`, a `Count()` star aggregate never reaches those 
overrides: `arity()` is 0, the loop emits no arguments, and expression shape 
output such as `PhysicalFilter.shapeInfo()` for HAVING predicates prints 
`count()` instead of the canonical `count(*)`. Please preserve the `Count` 
special case, for example by overriding `Count.shapeInfo()` or by reusing 
`computeToSql()` for the function wrapper while still calling child 
`shapeInfo()` for non-star arguments.



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