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/06/22 09:15:59 UTC
[GitHub] [doris] morrySnow commented on a diff in pull request #10335: [enhancement](Nereids): add more implmentation rules.
morrySnow commented on code in PR #10335:
URL: https://github.com/apache/doris/pull/10335#discussion_r903492537
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalProjectionToPhysicalProjection.java:
##########
@@ -23,7 +23,7 @@
import org.apache.doris.nereids.trees.plans.Plan;
/**
- * Implementation rule that convert logical join to physical hash join.
+ * Implementation rule that convert logical projection to physical projection.
Review Comment:
```suggestion
* Implementation rule that convert logical project to physical project.
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/operators/plans/logical/LogicalAggregation.java:
##########
@@ -31,68 +32,82 @@
/**
* Logical Aggregation plan operator.
- *
- *eg:select a, sum(b), c from table group by a, c;
+ * <p>
+ * eg:select a, sum(b), c from table group by a, c;
* groupByExpressions: Column field after group by. eg: a, c;
* outputExpressions: Column field after select. eg: a, sum(b), c;
- *
+ * <p>
* Each agg node only contains the select statement field of the same layer,
* and other agg nodes in the subquery contain.
*/
public class LogicalAggregation extends LogicalUnaryOperator {
- private final List<Expression> groupByExpressions;
- private final List<? extends NamedExpression> outputExpressions;
+ private final List<Expression> groupByExprList;
+ private final List<? extends NamedExpression> aggExprList;
+ private List<Expression> partitionExprList;
+ private AggPhase aggPhase;
Review Comment:
i think use different agg and group by expr is enough to distinguish different agg phase. We need to remove it later
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/operators/AbstractOperator.java:
##########
@@ -27,16 +27,16 @@
*/
public abstract class AbstractOperator implements Operator {
protected final OperatorType type;
- protected final long limited;
+ protected long limit;
Review Comment:
all operator should immutable, if need to change limit, we should add a function named `withLimit` or use full parameter constructor
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/operators/plans/logical/LogicalAggregation.java:
##########
@@ -31,68 +32,82 @@
/**
* Logical Aggregation plan operator.
- *
- *eg:select a, sum(b), c from table group by a, c;
+ * <p>
+ * eg:select a, sum(b), c from table group by a, c;
* groupByExpressions: Column field after group by. eg: a, c;
* outputExpressions: Column field after select. eg: a, sum(b), c;
- *
+ * <p>
* Each agg node only contains the select statement field of the same layer,
* and other agg nodes in the subquery contain.
*/
public class LogicalAggregation extends LogicalUnaryOperator {
- private final List<Expression> groupByExpressions;
- private final List<? extends NamedExpression> outputExpressions;
+ private final List<Expression> groupByExprList;
+ private final List<? extends NamedExpression> aggExprList;
+ private List<Expression> partitionExprList;
+ private AggPhase aggPhase;
+ private boolean usingStream;
Review Comment:
logical don't need this
--
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