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/24 13:51:24 UTC
[GitHub] [doris] Kikyou1997 opened a new pull request, #10412: [Feature] [Draft] Agg rewrite rule of nereids optmizer
Kikyou1997 opened a new pull request, #10412:
URL: https://github.com/apache/doris/pull/10412
# Proposed changes
Issue Number: close #xxx
## Problem Summary:
Describe the overview of changes.
## Checklist(Required)
1. Does it affect the original behavior: (Yes/No/I Don't know)
2. Has unit tests been added: (Yes/No/No Need)
3. Has document been added or modified: (Yes/No/No Need)
4. Does it need to update dependencies: (Yes/No)
5. Are there any changes that cannot be rolled back: (Yes/No)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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
[GitHub] [doris] EmmyMiao87 merged pull request #10412: [Feature] [nereids] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
EmmyMiao87 merged PR #10412:
URL: https://github.com/apache/doris/pull/10412
--
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
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #10412: [Feature] [nereids] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #10412:
URL: https://github.com/apache/doris/pull/10412#discussion_r912709466
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java:
##########
@@ -38,6 +38,7 @@ public enum RuleType {
PROJECT_TO_GLOBAL_AGGREGATE(RuleTypeClass.REWRITE),
// rewrite rules
+ AGGREGATE_DISASSEMBLE(RuleTypeClass.REWRITE),
Review Comment:
added TODO
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateDisassemble.java:
##########
@@ -0,0 +1,151 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregate;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.expressions.functions.AggregateFunction;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.clearspring.analytics.util.Lists;
+import com.google.common.base.Preconditions;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ * Do this in following steps:
+ * 1. clone output expr list, find all agg function
+ * 2. set found agg function intermediaType
+ * 3. create new child plan rooted at new local agg
+ * 4. update the slot referenced by expr of merge agg
+ * 5. create plan rooted at merge agg, return it.
+ */
+public class AggregateDisassemble extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregate().when(p -> {
+ LogicalAggregate logicalAggregation = p.getOperator();
+ return !logicalAggregation.isDisassembled();
+ }).thenApply(ctx -> {
+ Plan plan = ctx.root;
+ Operator operator = plan.getOperator();
+ LogicalAggregate agg = (LogicalAggregate) operator;
+ List<NamedExpression> outputExpressionList = agg.getOutputExpressionList();
+ List<NamedExpression> intermediateAggExpressionList = Lists.newArrayList();
+ for (NamedExpression namedExpression : outputExpressionList) {
+ namedExpression = (NamedExpression) namedExpression.clone();
+ List<AggregateFunction> functionCallList =
+ namedExpression.collect(org.apache.doris.catalog.AggregateFunction.class::isInstance);
+ for (AggregateFunction functionCall : functionCallList) {
+ FunctionName functionName = new FunctionName(functionCall.getName());
+ List<Expression> expressionList = functionCall.getArguments();
+ List<Type> staleTypeList = expressionList.stream().map(Expression::getDataType)
+ .map(DataType::toCatalogDataType).collect(Collectors.toList());
+ Function staleFuncDesc = new Function(functionName, staleTypeList,
+ functionCall.getDataType().toCatalogDataType(),
+ // I think an aggregate function will never have a variable length parameters
+ false);
+ Function staleFunc = Catalog.getCurrentCatalog()
+ .getFunction(staleFuncDesc, CompareMode.IS_IDENTICAL);
+ Preconditions.checkArgument(staleFunc instanceof org.apache.doris.catalog.AggregateFunction);
+ org.apache.doris.catalog.AggregateFunction
+ staleAggFunc = (org.apache.doris.catalog.AggregateFunction) staleFunc;
+ Type staleIntermediateType = staleAggFunc.getIntermediateType();
+ Type staleRetType = staleAggFunc.getReturnType();
+ if (staleIntermediateType != null && !staleIntermediateType.equals(staleRetType)) {
+ functionCall.setIntermediate(DataType.convertFromCatalogDataType(staleIntermediateType));
+ }
+ }
+ intermediateAggExpressionList.add(namedExpression);
+ }
+ LogicalAggregate localAgg = new LogicalAggregate(
+ agg.getGroupByExprList().stream().map(Expression::clone).collect(Collectors.toList()),
+ intermediateAggExpressionList,
+ true
+ );
+
+ Plan childPlan = plan(localAgg, plan.child(0));
+ List<Slot> stalePlanOutputSlotList = plan.getOutput();
+ List<Slot> childOutputSlotList = childPlan.getOutput();
+ int childOutputSize = stalePlanOutputSlotList.size();
+ Preconditions.checkState(childOutputSize == childOutputSlotList.size());
+ Map<Slot, Slot> staleToNew = new HashMap<>();
+ for (int i = 0; i < stalePlanOutputSlotList.size(); i++) {
+ staleToNew.put(stalePlanOutputSlotList.get(i), childOutputSlotList.get(i));
+ }
+ List<Expression> groupByExpressionList = agg.getGroupByExprList();
+ for (int i = 0; i < groupByExpressionList.size(); i++) {
+ replaceSlot(staleToNew, groupByExpressionList, groupByExpressionList.get(i), i);
+ }
+ List<NamedExpression> mergeOutputExpressionList = agg.getOutputExpressionList();
+ for (int i = 0; i < mergeOutputExpressionList.size(); i++) {
+ replaceSlot(staleToNew, mergeOutputExpressionList, mergeOutputExpressionList.get(i), i);
+ }
+ LogicalAggregate mergeAgg = new LogicalAggregate(
Review Comment:
done
--
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
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #10412: [Feature] [Draft] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #10412:
URL: https://github.com/apache/doris/pull/10412#discussion_r906749484
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateRewrite.java:
##########
@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.AggregateFunction;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregation;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.analysis.FunctionParams;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.FunctionCall;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ */
+public class AggregateRewrite extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregation().thenApply(ctx -> {
Review Comment:
I agree, should we add some method like `condition` in the Rule definition to determine whether a rule is truly applicable as the cascades paper described ?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateRewrite.java:
##########
@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.AggregateFunction;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregation;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.analysis.FunctionParams;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.FunctionCall;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ */
+public class AggregateRewrite extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregation().thenApply(ctx -> {
Review Comment:
I agree, should we add some method like `check` in the Rule definition to determine whether a rule is truly applicable as the cascades paper described ?
--
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
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #10412: [Feature] [nereids] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #10412:
URL: https://github.com/apache/doris/pull/10412#discussion_r912709220
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateDisassemble.java:
##########
@@ -0,0 +1,151 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregate;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.expressions.functions.AggregateFunction;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.clearspring.analytics.util.Lists;
+import com.google.common.base.Preconditions;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ * Do this in following steps:
+ * 1. clone output expr list, find all agg function
+ * 2. set found agg function intermediaType
+ * 3. create new child plan rooted at new local agg
+ * 4. update the slot referenced by expr of merge agg
+ * 5. create plan rooted at merge agg, return it.
+ */
+public class AggregateDisassemble extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregate().when(p -> {
+ LogicalAggregate logicalAggregation = p.getOperator();
+ return !logicalAggregation.isDisassembled();
+ }).thenApply(ctx -> {
+ Plan plan = ctx.root;
+ Operator operator = plan.getOperator();
+ LogicalAggregate agg = (LogicalAggregate) operator;
+ List<NamedExpression> outputExpressionList = agg.getOutputExpressionList();
+ List<NamedExpression> intermediateAggExpressionList = Lists.newArrayList();
+ for (NamedExpression namedExpression : outputExpressionList) {
Review Comment:
added TODO
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateDisassemble.java:
##########
@@ -0,0 +1,151 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregate;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.expressions.functions.AggregateFunction;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.clearspring.analytics.util.Lists;
+import com.google.common.base.Preconditions;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ * Do this in following steps:
+ * 1. clone output expr list, find all agg function
+ * 2. set found agg function intermediaType
+ * 3. create new child plan rooted at new local agg
+ * 4. update the slot referenced by expr of merge agg
+ * 5. create plan rooted at merge agg, return it.
+ */
+public class AggregateDisassemble extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregate().when(p -> {
+ LogicalAggregate logicalAggregation = p.getOperator();
+ return !logicalAggregation.isDisassembled();
+ }).thenApply(ctx -> {
+ Plan plan = ctx.root;
+ Operator operator = plan.getOperator();
+ LogicalAggregate agg = (LogicalAggregate) operator;
+ List<NamedExpression> outputExpressionList = agg.getOutputExpressionList();
+ List<NamedExpression> intermediateAggExpressionList = Lists.newArrayList();
+ for (NamedExpression namedExpression : outputExpressionList) {
+ namedExpression = (NamedExpression) namedExpression.clone();
+ List<AggregateFunction> functionCallList =
+ namedExpression.collect(org.apache.doris.catalog.AggregateFunction.class::isInstance);
+ for (AggregateFunction functionCall : functionCallList) {
+ FunctionName functionName = new FunctionName(functionCall.getName());
+ List<Expression> expressionList = functionCall.getArguments();
+ List<Type> staleTypeList = expressionList.stream().map(Expression::getDataType)
+ .map(DataType::toCatalogDataType).collect(Collectors.toList());
+ Function staleFuncDesc = new Function(functionName, staleTypeList,
Review Comment:
done
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #10412: [Feature] [nereids] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10412:
URL: https://github.com/apache/doris/pull/10412#issuecomment-1174573940
PR approved by anyone and no changes requested.
--
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
[GitHub] [doris] EmmyMiao87 commented on a diff in pull request #10412: [Feature] [nereids] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
EmmyMiao87 commented on code in PR #10412:
URL: https://github.com/apache/doris/pull/10412#discussion_r913337306
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateDisassemble.java:
##########
@@ -0,0 +1,151 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregate;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.expressions.functions.AggregateFunction;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.clearspring.analytics.util.Lists;
+import com.google.common.base.Preconditions;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ * Do this in following steps:
+ * 1. clone output expr list, find all agg function
+ * 2. set found agg function intermediaType
+ * 3. create new child plan rooted at new local agg
+ * 4. update the slot referenced by expr of merge agg
+ * 5. create plan rooted at merge agg, return it.
+ */
+public class AggregateDisassemble extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregate().when(p -> {
+ LogicalAggregate logicalAggregation = p.getOperator();
+ return !logicalAggregation.isDisassembled();
+ }).thenApply(ctx -> {
+ Plan plan = ctx.root;
+ Operator operator = plan.getOperator();
+ LogicalAggregate agg = (LogicalAggregate) operator;
+ List<NamedExpression> outputExpressionList = agg.getOutputExpressionList();
+ List<NamedExpression> intermediateAggExpressionList = Lists.newArrayList();
+ for (NamedExpression namedExpression : outputExpressionList) {
Review Comment:
This TODO should actually be added to the logical aggregate.
--
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
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #10412: [Feature] [Draft] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #10412:
URL: https://github.com/apache/doris/pull/10412#discussion_r906746432
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateRewrite.java:
##########
@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.AggregateFunction;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregation;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.analysis.FunctionParams;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.FunctionCall;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ */
+public class AggregateRewrite extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregation().thenApply(ctx -> {
+ Plan plan = ctx.root;
+ Operator operator = plan.getOperator();
+ LogicalAggregation agg = (LogicalAggregation) operator;
+ List<NamedExpression> namedExpressionList = agg.getOutputExpressions();
+ List<NamedExpression> intermediateAggExpressionList = agg.getOutputExpressions();
+ for (NamedExpression namedExpression : namedExpressionList) {
Review Comment:
I thought so, but I haven't figured out how to get the slotRef yet.....
--
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
[GitHub] [doris] morrySnow commented on a diff in pull request #10412: [Feature] [nereids] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
morrySnow commented on code in PR #10412:
URL: https://github.com/apache/doris/pull/10412#discussion_r913352685
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateDisassemble.java:
##########
@@ -0,0 +1,151 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregate;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.expressions.functions.AggregateFunction;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.clearspring.analytics.util.Lists;
+import com.google.common.base.Preconditions;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ * Do this in following steps:
+ * 1. clone output expr list, find all agg function
+ * 2. set found agg function intermediaType
+ * 3. create new child plan rooted at new local agg
+ * 4. update the slot referenced by expr of merge agg
+ * 5. create plan rooted at merge agg, return it.
+ */
+public class AggregateDisassemble extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregate().when(p -> {
+ LogicalAggregate logicalAggregation = p.getOperator();
+ return !logicalAggregation.isDisassembled();
+ }).thenApply(ctx -> {
+ Plan plan = ctx.root;
+ Operator operator = plan.getOperator();
+ LogicalAggregate agg = (LogicalAggregate) operator;
+ List<NamedExpression> outputExpressionList = agg.getOutputExpressionList();
+ List<NamedExpression> intermediateAggExpressionList = Lists.newArrayList();
+ for (NamedExpression namedExpression : outputExpressionList) {
Review Comment:
for aggregate function in having, we should push them to aggregate output expression and add generate a project(filter) as its parent, after binding, the whole plan will be project(filter(aggregate)). If we do that, the output expression is all we need and process aggregate operator will more easily
--
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
[GitHub] [doris] morrySnow commented on a diff in pull request #10412: [Feature] [Draft] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
morrySnow commented on code in PR #10412:
URL: https://github.com/apache/doris/pull/10412#discussion_r907411869
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleSet.java:
##########
@@ -40,6 +41,7 @@ public class RuleSet {
.build();
public static final List<Rule<Plan>> EXPLORATION_RULES = planRuleFactories()
+ .add(new AggregateBreakUp())
Review Comment:
it is a rewrite rule
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/operators/plans/logical/LogicalAggregation.java:
##########
@@ -95,4 +104,12 @@ public List<Slot> computeOutput(Plan input) {
public List<Expression> getExpressions() {
return new ImmutableList.Builder<Expression>().addAll(groupByExpressions).addAll(outputExpressions).build();
}
+
+ public boolean isBrokeUp() {
+ return brokeUp;
+ }
+
+ public void setBrokeUp(boolean brokeUp) {
Review Comment:
operator is immutable, should not have any setter
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateBreakUp.java:
##########
@@ -0,0 +1,153 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.AggregateFunction;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregation;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.analysis.FunctionParams;
+import org.apache.doris.nereids.trees.expressions.Alias;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.FunctionCall;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ */
+public class AggregateBreakUp extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregation().when(p -> {
+ LogicalAggregation logicalAggregation = p.getOperator();
+ return !logicalAggregation.isBrokeUp();
+ }).thenApply(ctx -> {
+ Plan plan = ctx.root;
+ Operator operator = plan.getOperator();
+ LogicalAggregation agg = (LogicalAggregation) operator;
+ List<NamedExpression> outputExpressionList = agg.getOutputExpressions();
+ List<NamedExpression> intermediateAggExpressionList = agg.getOutputExpressions();
+ for (NamedExpression namedExpression : outputExpressionList) {
+ namedExpression = (NamedExpression) namedExpression.clone();
Review Comment:
u need use an expression visitor to do that. For example
```
select a + 1, sum(b) + 2 from t group by a;
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java:
##########
@@ -32,6 +32,7 @@ public enum RuleType {
BINDING_SENTINEL(RuleTypeClass.REWRITE),
// rewrite rules
+ REWRITE_AGG(RuleTypeClass.REWRITE),
Review Comment:
```suggestion
REWRITE_AGGREGATE(RuleTypeClass.REWRITE),
```
--
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
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #10412: [Feature] [nereids] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #10412:
URL: https://github.com/apache/doris/pull/10412#discussion_r912704846
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/operators/plans/logical/LogicalAggregate.java:
##########
@@ -56,6 +57,16 @@ public LogicalAggregate(List<Expression> groupByExprList, List<NamedExpression>
super(OperatorType.LOGICAL_AGGREGATION);
this.groupByExprList = groupByExprList;
this.outputExpressionList = outputExpressionList;
+ this.disassembled = false;
Review Comment:
It's a `final` field, so we must init it explicitly in the constructor
--
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
[GitHub] [doris] morrySnow commented on a diff in pull request #10412: [Feature] [Draft] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
morrySnow commented on code in PR #10412:
URL: https://github.com/apache/doris/pull/10412#discussion_r907968917
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateBreakUp.java:
##########
@@ -0,0 +1,153 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.AggregateFunction;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregation;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.analysis.FunctionParams;
+import org.apache.doris.nereids.trees.expressions.Alias;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.FunctionCall;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ */
+public class AggregateBreakUp extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregation().when(p -> {
+ LogicalAggregation logicalAggregation = p.getOperator();
+ return !logicalAggregation.isBrokeUp();
+ }).thenApply(ctx -> {
+ Plan plan = ctx.root;
+ Operator operator = plan.getOperator();
+ LogicalAggregation agg = (LogicalAggregation) operator;
+ List<NamedExpression> outputExpressionList = agg.getOutputExpressions();
+ List<NamedExpression> intermediateAggExpressionList = agg.getOutputExpressions();
+ for (NamedExpression namedExpression : outputExpressionList) {
+ namedExpression = (NamedExpression) namedExpression.clone();
Review Comment:
u need use an expression visitor to do that. For example
```sql
select a + 1, sum(b) + 2 from t group by a;
```
--
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
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #10412: [Feature] [Draft] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #10412:
URL: https://github.com/apache/doris/pull/10412#discussion_r906747888
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateRewrite.java:
##########
@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.AggregateFunction;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregation;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.analysis.FunctionParams;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.FunctionCall;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ */
+public class AggregateRewrite extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregation().thenApply(ctx -> {
+ Plan plan = ctx.root;
+ Operator operator = plan.getOperator();
+ LogicalAggregation agg = (LogicalAggregation) operator;
+ List<NamedExpression> namedExpressionList = agg.getOutputExpressions();
+ List<NamedExpression> intermediateAggExpressionList = agg.getOutputExpressions();
+ for (NamedExpression namedExpression : namedExpressionList) {
Review Comment:
I think so, but I haven't figured out how to get the `SlotReference` properly yet. As `SlotReference` is a LeafExpression, it can't have any child which means If the namedExpression is a `SlotReference` type, then it shouldn' have any child as `FunctionCall`.
If the slotRef is calculated by `computeOutput`, the update of `FunctionCall` would be enough, I think the invocation chain is like below:
```
LogicalAgg::computeOutput ->
Alias::computeOutput ->
FunctionCall::getDataType
```
--
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
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #10412: [Feature] [Draft] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #10412:
URL: https://github.com/apache/doris/pull/10412#discussion_r906746432
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateRewrite.java:
##########
@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.AggregateFunction;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregation;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.analysis.FunctionParams;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.FunctionCall;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ */
+public class AggregateRewrite extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregation().thenApply(ctx -> {
+ Plan plan = ctx.root;
+ Operator operator = plan.getOperator();
+ LogicalAggregation agg = (LogicalAggregation) operator;
+ List<NamedExpression> namedExpressionList = agg.getOutputExpressions();
+ List<NamedExpression> intermediateAggExpressionList = agg.getOutputExpressions();
+ for (NamedExpression namedExpression : namedExpressionList) {
Review Comment:
I thought so, but I haven't figured out how to get the slotRef yet.....
--
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
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #10412: [Feature] [Draft] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #10412:
URL: https://github.com/apache/doris/pull/10412#discussion_r906746480
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleSet.java:
##########
@@ -37,6 +38,7 @@
public class RuleSet {
public static final List<Rule<Plan>> ANALYSIS_RULES = planRuleFactories()
.add(new BindRelation())
+ .add(new AggregateRewrite())
Review Comment:
why
--
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
[GitHub] [doris] morrySnow commented on a diff in pull request #10412: [Feature] [nereids] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
morrySnow commented on code in PR #10412:
URL: https://github.com/apache/doris/pull/10412#discussion_r913352685
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateDisassemble.java:
##########
@@ -0,0 +1,151 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregate;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.expressions.functions.AggregateFunction;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.clearspring.analytics.util.Lists;
+import com.google.common.base.Preconditions;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ * Do this in following steps:
+ * 1. clone output expr list, find all agg function
+ * 2. set found agg function intermediaType
+ * 3. create new child plan rooted at new local agg
+ * 4. update the slot referenced by expr of merge agg
+ * 5. create plan rooted at merge agg, return it.
+ */
+public class AggregateDisassemble extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregate().when(p -> {
+ LogicalAggregate logicalAggregation = p.getOperator();
+ return !logicalAggregation.isDisassembled();
+ }).thenApply(ctx -> {
+ Plan plan = ctx.root;
+ Operator operator = plan.getOperator();
+ LogicalAggregate agg = (LogicalAggregate) operator;
+ List<NamedExpression> outputExpressionList = agg.getOutputExpressionList();
+ List<NamedExpression> intermediateAggExpressionList = Lists.newArrayList();
+ for (NamedExpression namedExpression : outputExpressionList) {
Review Comment:
for aggregate function in having, we should push them to aggregate output expression and add generate a project(filter) as its parent, after binding, the whole plan will be project(filter(aggregate))
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #10412: [Feature] [nereids] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10412:
URL: https://github.com/apache/doris/pull/10412#issuecomment-1174573925
PR approved by at least one committer and no changes requested.
--
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
[GitHub] [doris] EmmyMiao87 commented on a diff in pull request #10412: [Feature] [nereids] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
EmmyMiao87 commented on code in PR #10412:
URL: https://github.com/apache/doris/pull/10412#discussion_r912664745
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/operators/plans/logical/LogicalAggregate.java:
##########
@@ -56,6 +57,16 @@ public LogicalAggregate(List<Expression> groupByExprList, List<NamedExpression>
super(OperatorType.LOGICAL_AGGREGATION);
this.groupByExprList = groupByExprList;
this.outputExpressionList = outputExpressionList;
+ this.disassembled = false;
Review Comment:
No need this line. The default value of disassembled is false
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/operators/plans/logical/LogicalAggregate.java:
##########
@@ -43,6 +43,7 @@
*/
public class LogicalAggregate extends LogicalUnaryOperator {
+ private final boolean disassembled;
Review Comment:
Maybe it would be more appropriate to change the name to two-stage
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java:
##########
@@ -38,6 +38,7 @@ public enum RuleType {
PROJECT_TO_GLOBAL_AGGREGATE(RuleTypeClass.REWRITE),
// rewrite rules
+ AGGREGATE_DISASSEMBLE(RuleTypeClass.REWRITE),
Review Comment:
It seems that it is now split into two-stage aggregation by default? Should a todo be added, indicating that whether to apply the rule or not will be determined according to the cost situation in the future.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/operators/plans/logical/LogicalAggregate.java:
##########
@@ -56,6 +57,16 @@ public LogicalAggregate(List<Expression> groupByExprList, List<NamedExpression>
super(OperatorType.LOGICAL_AGGREGATION);
this.groupByExprList = groupByExprList;
this.outputExpressionList = outputExpressionList;
+ this.disassembled = false;
+ }
+
+ public LogicalAggregate(List<Expression> groupByExprList,
+ List<NamedExpression> outputExpressionList,
+ boolean disassembled) {
+ super(OperatorType.LOGICAL_AGGREGATION);
+ this.groupByExprList = groupByExprList;
+ this.outputExpressionList = outputExpressionList;
+ this.disassembled = false;
Review Comment:
The parameter is passed in, the result is still set to FALSE directly?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleSet.java:
##########
@@ -39,6 +40,10 @@ public class RuleSet {
.add(new JoinLeftAssociative())
.build();
+ public static final List<Rule<Plan>> REWRITE_RULES = planRuleFactories()
Review Comment:
Is it appropriate to put aggregation and splitting at the current stage?
In theory almost all rewrite rules don't care if agg is one or two.
If the agg split is performed too early, it will affect the writing of some agg related rules.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateDisassemble.java:
##########
@@ -0,0 +1,151 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregate;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.expressions.functions.AggregateFunction;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.clearspring.analytics.util.Lists;
+import com.google.common.base.Preconditions;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ * Do this in following steps:
+ * 1. clone output expr list, find all agg function
+ * 2. set found agg function intermediaType
+ * 3. create new child plan rooted at new local agg
+ * 4. update the slot referenced by expr of merge agg
+ * 5. create plan rooted at merge agg, return it.
+ */
+public class AggregateDisassemble extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregate().when(p -> {
+ LogicalAggregate logicalAggregation = p.getOperator();
+ return !logicalAggregation.isDisassembled();
+ }).thenApply(ctx -> {
+ Plan plan = ctx.root;
+ Operator operator = plan.getOperator();
+ LogicalAggregate agg = (LogicalAggregate) operator;
+ List<NamedExpression> outputExpressionList = agg.getOutputExpressionList();
+ List<NamedExpression> intermediateAggExpressionList = Lists.newArrayList();
+ for (NamedExpression namedExpression : outputExpressionList) {
Review Comment:
Don't try to extract the agg function from the output, but read the agg function directly from the operator
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateDisassemble.java:
##########
@@ -0,0 +1,151 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregate;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.expressions.functions.AggregateFunction;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.clearspring.analytics.util.Lists;
+import com.google.common.base.Preconditions;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ * Do this in following steps:
+ * 1. clone output expr list, find all agg function
+ * 2. set found agg function intermediaType
+ * 3. create new child plan rooted at new local agg
+ * 4. update the slot referenced by expr of merge agg
+ * 5. create plan rooted at merge agg, return it.
+ */
+public class AggregateDisassemble extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregate().when(p -> {
+ LogicalAggregate logicalAggregation = p.getOperator();
+ return !logicalAggregation.isDisassembled();
Review Comment:
It seems that we should use the framework mechanism of rule apply to ensure that an operator should not apply repeated rules again, rather than judging here.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateDisassemble.java:
##########
@@ -0,0 +1,151 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregate;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.expressions.functions.AggregateFunction;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.clearspring.analytics.util.Lists;
+import com.google.common.base.Preconditions;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ * Do this in following steps:
+ * 1. clone output expr list, find all agg function
+ * 2. set found agg function intermediaType
+ * 3. create new child plan rooted at new local agg
+ * 4. update the slot referenced by expr of merge agg
+ * 5. create plan rooted at merge agg, return it.
+ */
+public class AggregateDisassemble extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregate().when(p -> {
+ LogicalAggregate logicalAggregation = p.getOperator();
+ return !logicalAggregation.isDisassembled();
+ }).thenApply(ctx -> {
+ Plan plan = ctx.root;
+ Operator operator = plan.getOperator();
+ LogicalAggregate agg = (LogicalAggregate) operator;
+ List<NamedExpression> outputExpressionList = agg.getOutputExpressionList();
+ List<NamedExpression> intermediateAggExpressionList = Lists.newArrayList();
+ for (NamedExpression namedExpression : outputExpressionList) {
+ namedExpression = (NamedExpression) namedExpression.clone();
+ List<AggregateFunction> functionCallList =
+ namedExpression.collect(org.apache.doris.catalog.AggregateFunction.class::isInstance);
+ for (AggregateFunction functionCall : functionCallList) {
+ FunctionName functionName = new FunctionName(functionCall.getName());
+ List<Expression> expressionList = functionCall.getArguments();
+ List<Type> staleTypeList = expressionList.stream().map(Expression::getDataType)
+ .map(DataType::toCatalogDataType).collect(Collectors.toList());
+ Function staleFuncDesc = new Function(functionName, staleTypeList,
Review Comment:
Wrap these 73~83 lines of getting functions from the old framework in a function. At the same time, with TODO, the acquisition and registration of subsequent lines will be modified uniformly.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateDisassemble.java:
##########
@@ -0,0 +1,151 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregate;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.expressions.functions.AggregateFunction;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.clearspring.analytics.util.Lists;
+import com.google.common.base.Preconditions;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ * Do this in following steps:
+ * 1. clone output expr list, find all agg function
+ * 2. set found agg function intermediaType
+ * 3. create new child plan rooted at new local agg
+ * 4. update the slot referenced by expr of merge agg
+ * 5. create plan rooted at merge agg, return it.
+ */
+public class AggregateDisassemble extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregate().when(p -> {
+ LogicalAggregate logicalAggregation = p.getOperator();
+ return !logicalAggregation.isDisassembled();
+ }).thenApply(ctx -> {
+ Plan plan = ctx.root;
+ Operator operator = plan.getOperator();
+ LogicalAggregate agg = (LogicalAggregate) operator;
+ List<NamedExpression> outputExpressionList = agg.getOutputExpressionList();
+ List<NamedExpression> intermediateAggExpressionList = Lists.newArrayList();
+ for (NamedExpression namedExpression : outputExpressionList) {
+ namedExpression = (NamedExpression) namedExpression.clone();
+ List<AggregateFunction> functionCallList =
+ namedExpression.collect(org.apache.doris.catalog.AggregateFunction.class::isInstance);
+ for (AggregateFunction functionCall : functionCallList) {
+ FunctionName functionName = new FunctionName(functionCall.getName());
+ List<Expression> expressionList = functionCall.getArguments();
+ List<Type> staleTypeList = expressionList.stream().map(Expression::getDataType)
+ .map(DataType::toCatalogDataType).collect(Collectors.toList());
+ Function staleFuncDesc = new Function(functionName, staleTypeList,
+ functionCall.getDataType().toCatalogDataType(),
+ // I think an aggregate function will never have a variable length parameters
+ false);
+ Function staleFunc = Catalog.getCurrentCatalog()
+ .getFunction(staleFuncDesc, CompareMode.IS_IDENTICAL);
+ Preconditions.checkArgument(staleFunc instanceof org.apache.doris.catalog.AggregateFunction);
+ org.apache.doris.catalog.AggregateFunction
+ staleAggFunc = (org.apache.doris.catalog.AggregateFunction) staleFunc;
+ Type staleIntermediateType = staleAggFunc.getIntermediateType();
+ Type staleRetType = staleAggFunc.getReturnType();
+ if (staleIntermediateType != null && !staleIntermediateType.equals(staleRetType)) {
+ functionCall.setIntermediate(DataType.convertFromCatalogDataType(staleIntermediateType));
+ }
+ }
+ intermediateAggExpressionList.add(namedExpression);
+ }
+ LogicalAggregate localAgg = new LogicalAggregate(
+ agg.getGroupByExprList().stream().map(Expression::clone).collect(Collectors.toList()),
+ intermediateAggExpressionList,
+ true
+ );
+
+ Plan childPlan = plan(localAgg, plan.child(0));
+ List<Slot> stalePlanOutputSlotList = plan.getOutput();
+ List<Slot> childOutputSlotList = childPlan.getOutput();
+ int childOutputSize = stalePlanOutputSlotList.size();
+ Preconditions.checkState(childOutputSize == childOutputSlotList.size());
+ Map<Slot, Slot> staleToNew = new HashMap<>();
+ for (int i = 0; i < stalePlanOutputSlotList.size(); i++) {
+ staleToNew.put(stalePlanOutputSlotList.get(i), childOutputSlotList.get(i));
+ }
+ List<Expression> groupByExpressionList = agg.getGroupByExprList();
+ for (int i = 0; i < groupByExpressionList.size(); i++) {
+ replaceSlot(staleToNew, groupByExpressionList, groupByExpressionList.get(i), i);
+ }
+ List<NamedExpression> mergeOutputExpressionList = agg.getOutputExpressionList();
+ for (int i = 0; i < mergeOutputExpressionList.size(); i++) {
+ replaceSlot(staleToNew, mergeOutputExpressionList, mergeOutputExpressionList.get(i), i);
+ }
+ LogicalAggregate mergeAgg = new LogicalAggregate(
Review Comment:
You need a representation to determine whether the current agg is a lower agg or an upper one.
--
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
[GitHub] [doris] morrySnow commented on a diff in pull request #10412: [Feature] [Draft] Agg rewrite rule of nereids optmizer
Posted by GitBox <gi...@apache.org>.
morrySnow commented on code in PR #10412:
URL: https://github.com/apache/doris/pull/10412#discussion_r906694231
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleSet.java:
##########
@@ -37,6 +38,7 @@
public class RuleSet {
public static final List<Rule<Plan>> ANALYSIS_RULES = planRuleFactories()
.add(new BindRelation())
+ .add(new AggregateRewrite())
Review Comment:
maybe it is better to use two rule set, one for bind, another for rewrite
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateRewrite.java:
##########
@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.AggregateFunction;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregation;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.analysis.FunctionParams;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.FunctionCall;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ */
+public class AggregateRewrite extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregation().thenApply(ctx -> {
Review Comment:
we need to check whether this aggregate plan has been split or generated by this rule. If that, we should not apply this rule again.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateRewrite.java:
##########
@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.AggregateFunction;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregation;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.analysis.FunctionParams;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.FunctionCall;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ */
+public class AggregateRewrite extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregation().thenApply(ctx -> {
+ Plan plan = ctx.root;
+ Operator operator = plan.getOperator();
+ LogicalAggregation agg = (LogicalAggregation) operator;
+ List<NamedExpression> namedExpressionList = agg.getOutputExpressions();
+ List<NamedExpression> intermediateAggExpressionList = agg.getOutputExpressions();
+ for (NamedExpression namedExpression : namedExpressionList) {
Review Comment:
the slot reference in upper node's expression always refer to it children's output. So we should not only update function call's type, but also update all slot references.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/operators/plans/logical/LogicalAggregation.java:
##########
@@ -42,18 +42,27 @@
public class LogicalAggregation extends LogicalUnaryOperator {
private final List<Expression> groupByExpressions;
- private final List<? extends NamedExpression> outputExpressions;
+ private List<NamedExpression> outputExpressions;
Review Comment:
all operator should immutable, always generate new operator if u want to change attributes in it
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java:
##########
@@ -34,6 +34,7 @@ public enum RuleType {
// rewrite rules
COLUMN_PRUNE_PROJECTION(RuleTypeClass.REWRITE),
REWRITE_SENTINEL(RuleTypeClass.REWRITE),
+ REWRITE_AGG(RuleTypeClass.REWRITE),
Review Comment:
should be move above REWRITE_SENTINEL
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateRewrite.java:
##########
@@ -0,0 +1,93 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.AggregateFunction;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
+import org.apache.doris.catalog.Function.CompareMode;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.operators.Operator;
+import org.apache.doris.nereids.operators.plans.logical.LogicalAggregation;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.analysis.FunctionParams;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.FunctionCall;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.types.DataType;
+
+import com.google.common.base.Preconditions;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Used to generate the merge agg node for distributed execution.
+ */
+public class AggregateRewrite extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule<Plan> build() {
+ return logicalAggregation().thenApply(ctx -> {
+ Plan plan = ctx.root;
+ Operator operator = plan.getOperator();
+ LogicalAggregation agg = (LogicalAggregation) operator;
+ List<NamedExpression> namedExpressionList = agg.getOutputExpressions();
Review Comment:
```suggestion
List<NamedExpression> outputExpressionList = agg.getOutputExpressions();
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/FunctionCall.java:
##########
@@ -30,7 +35,14 @@ public class FunctionCall extends Expression {
private FunctionParams fnParams;
- private FunctionCall(FunctionName functionName, FunctionParams functionParams) {
+ private DataType retType;
+
+ // Used to construct output, this type may differ from above retType
+ // when the intermediate type of aggregate function is not same
+ // as its return type
+ private DataType type;
Review Comment:
do not understand why introduce these two type
--
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