You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/07/07 04:22:04 UTC

[GitHub] [doris] 924060929 commented on a diff in pull request #10659: [enhancement](nereids) make aggregate works

924060929 commented on code in PR #10659:
URL: https://github.com/apache/doris/pull/10659#discussion_r915441815


##########
fe/fe-core/src/test/java/org/apache/doris/nereids/AnalyzeSSBTest.java:
##########
@@ -161,10 +161,7 @@ private void executeRewriteBottomUpJob(PlannerContext plannerContext, RuleFactor
     }
 
     private boolean checkBound(LogicalPlan root) {
-        if (!checkPlanBound(root))  {
-            return false;
-        }
-        return true;
+        return checkPlanBound(root);

Review Comment:
   I think we can remove checkBound or checkPlanBound  function



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/TreeNode.java:
##########
@@ -91,4 +91,22 @@ default <T> T collect(Predicate<TreeNode<NODE_TYPE>> predicate) {
         return (T) result.build();
     }
 
+    /**
+     * Test whether this tree satisfied predicate.
+     *
+     * @param predicate test condition
+     * @return true if satisfied
+     */
+    default boolean contains(Predicate<TreeNode<NODE_TYPE>> predicate) {

Review Comment:
   contains function repeated with anyMatch function
   
   



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Add.java:
##########
@@ -57,6 +57,6 @@ public <R, C> R accept(ExpressionVisitor<R, C> visitor, C context) {
 
 
     public String toString() {
-        return sql();
+        return left().toString() + ' ' + getArithmeticOperator().toString() + ' ' + right().sql();

Review Comment:
   why left use toString(), but right use sql()



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/Sum.java:
##########
@@ -51,7 +51,7 @@ public DataType getDataType() {
 
     @Override
     public boolean nullable() {
-        return false;
+        return child().nullable();

Review Comment:
   sum aggregate function can return null value?



-- 
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: commits-unsubscribe@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org