morrySnow commented on code in PR #19934:
URL: https://github.com/apache/doris/pull/19934#discussion_r1206412223
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java:
##########
@@ -262,6 +273,9 @@ public enum RuleType {
LOGICAL_JOIN_TO_NESTED_LOOP_JOIN_RULE(RuleTypeClass.IMPLEMENTATION),
LOGICAL_PROJECT_TO_PHYSICAL_PROJECT_RULE(RuleTypeClass.IMPLEMENTATION),
LOGICAL_FILTER_TO_PHYSICAL_FILTER_RULE(RuleTypeClass.IMPLEMENTATION),
+
LOGICAL_CTE_PRODUCE_TO_PHYSICAL_CTE_PRODUCE_RULE(RuleTypeClass.IMPLEMENTATION),
+
LOGICAL_CTE_CONSUME_TO_PHYSICAL_CTE_CONSUME_RULE(RuleTypeClass.IMPLEMENTATION),
Review Comment:
```suggestion
LOGICAL_CTE_PRODUCER_TO_PHYSICAL_CTE_PRODUCER_RULE(RuleTypeClass.IMPLEMENTATION),
LOGICAL_CTE_CONSUMER_TO_PHYSICAL_CTE_CONSUMER_RULE(RuleTypeClass.IMPLEMENTATION),
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/RuntimeFilterTranslator.java:
##########
@@ -84,6 +84,8 @@ private class RuntimeFilterExpressionTranslator extends
ExpressionTranslator {
@Override
public Expr visitSlotReference(SlotReference slotReference,
PlanTranslatorContext context) {
+ slotReference = context.getRuntimeTranslator().get()
+ .context.getCorrespondingOlapSlotReference(slotReference);
Review Comment:
rebase to newest master in case of repeat these codes twice after merge into
master
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCTE.java:
##########
@@ -116,11 +140,28 @@ public List<Expression> getExpressions() {
@Override
public LogicalCTE<CHILD_TYPE>
withGroupExpression(Optional<GroupExpression> groupExpression) {
- return new LogicalCTE<>(aliasQueries, groupExpression,
Optional.of(getLogicalProperties()), child());
+ return new LogicalCTE<>(aliasQueries, groupExpression,
Optional.of(getLogicalProperties()), child(),
+ registered, cteNameToId);
}
@Override
public LogicalCTE<CHILD_TYPE>
withLogicalProperties(Optional<LogicalProperties> logicalProperties) {
- return new LogicalCTE<>(aliasQueries, Optional.empty(),
logicalProperties, child());
+ return new LogicalCTE<>(aliasQueries, Optional.empty(),
logicalProperties, child(), registered,
+ cteNameToId);
+ }
+
+ public boolean isRegistered() {
+ return registered;
+ }
+
+ public int findUniqueId(String subQueryAlias) {
Review Comment:
not use `uniqueId`, please replace all `uniqueId` with `cteId`
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCTEProducer.java:
##########
@@ -0,0 +1,143 @@
+// 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.trees.plans.logical;
+
+import org.apache.doris.nereids.memo.GroupExpression;
+import org.apache.doris.nereids.properties.LogicalProperties;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.PlanType;
+import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor;
+
+import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+
+/**
+ * LogicalCTEProducer
+ */
+public class LogicalCTEProducer<CHILD_TYPE extends Plan>
+ extends LogicalUnary<CHILD_TYPE> {
+
+ private final int cteId;
+
+ private final List<Slot> projects;
+
+ private final boolean rewritten;
+
+ public LogicalCTEProducer(CHILD_TYPE child, int cteId) {
+ super(PlanType.LOGICAL_CTE_PRODUCER, child);
+ this.cteId = cteId;
+ this.projects = Collections.emptyList();
+ this.rewritten = false;
+ }
+
+ public LogicalCTEProducer(Optional<GroupExpression> groupExpression,
+ Optional<LogicalProperties> logicalProperties,
CHILD_TYPE child, int cteId,
+ List<Slot> projects, boolean rewritten) {
+ super(PlanType.LOGICAL_CTE_PRODUCER, groupExpression,
logicalProperties, child);
+ this.cteId = cteId;
+ this.projects =
ImmutableList.copyOf(java.util.Objects.requireNonNull(projects,
Review Comment:
import java.util.Objects
```suggestion
this.projects = ImmutableList.copyOf(Objects.requireNonNull(projects,
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalSubQueryAlias.java:
##########
@@ -156,4 +169,16 @@ public LogicalSubQueryAlias<CHILD_TYPE>
withLogicalProperties(Optional<LogicalPr
return new LogicalSubQueryAlias<>(qualifier, columnAliases,
Optional.empty(),
logicalProperties, child());
}
+
+ public int cteId() {
+ return CTEIdGenerator.newCTEId();
Review Comment:
u should add a new function `newCteId` in `StatementScopeIdGenerator`, and
generate id from it to avoid overflow.
and do not use primitive int directly, use class `CTEId` instead
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/CTEIdGenerator.java:
##########
@@ -0,0 +1,50 @@
+// 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.util;
+
+import org.apache.doris.nereids.StatementContext;
+import org.apache.doris.qe.ConnectContext;
+
+import com.google.common.annotations.VisibleForTesting;
+
+/**
+ * Used to generate id for CTE.
+ */
+public class CTEIdGenerator {
Review Comment:
remove this generator, use `StatementScopeIdGenerator#newCteId` to generate
cteId
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/logical/CTEProducerRewrite.java:
##########
@@ -0,0 +1,122 @@
+// 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.logical;
+
+import org.apache.doris.nereids.CascadesContext;
+import org.apache.doris.nereids.jobs.batch.NereidsRewriter;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.rules.rewrite.OneRewriteRuleFactory;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.logical.LogicalCTEConsumer;
+import org.apache.doris.nereids.trees.plans.logical.LogicalCTEProducer;
+import org.apache.doris.nereids.trees.plans.logical.LogicalFilter;
+import org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
+import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
+import org.apache.doris.nereids.util.ExpressionUtils;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import org.apache.commons.collections.CollectionUtils;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.Map.Entry;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Rewrite CTE Producer recursively.
+ */
+public class CTEProducerRewrite extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule build() {
+ return logicalCTEProducer().when(p -> !p.isRewritten()).thenApply(ctx
-> {
+ LogicalCTEProducer<? extends Plan> cteProducer = ctx.root;
+ Set<Expression> projects =
ctx.cascadesContext.findProjectForProducer(cteProducer.getCteId());
+ LogicalPlan child = tryToConstructFilter(ctx.cascadesContext,
cteProducer.getCteId(),
+ (LogicalPlan) ctx.root.child());
+ if (CollectionUtils.isNotEmpty(projects)
+ &&
ctx.cascadesContext.couldPruneColumnOnProducer(cteProducer.getCteId())) {
+ child = new LogicalProject(ImmutableList.copyOf(projects),
child);
+ }
+ CascadesContext rewrittenCtx =
ctx.cascadesContext.forkForCTEProducer(child);
+ NereidsRewriter rewriter = new NereidsRewriter(rewrittenCtx);
+ rewriter.execute();
+ return
cteProducer.withChildrenAndProjects(ImmutableList.of(rewrittenCtx.getRewritePlan()),
+ new ArrayList<>(child.getOutput()), true);
+ }).toRule(RuleType.CTE_PRODUCER_REWRITE);
+ }
+
+ /*
+ * An expression can only be pushed down if it has filter expressions on
all consumers that reference the slot.
+ * For example, let's assume a producer has two consumers, consumer1 and
consumer2:
+ *
+ * filter(a > 5 and b < 1) -> consumer1
+ * filter(a < 8) -> consumer2
+ *
+ * In this case, the only expression that can be pushed down to the
producer is filter(a > 5 or a < 8).
+ */
+ private LogicalPlan tryToConstructFilter(CascadesContext cascadesContext,
int cteId, LogicalPlan child) {
Review Comment:
just push `(a > 5 and b < 1) or (a < 8)` is simple and better
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java:
##########
@@ -51,6 +51,8 @@ public class StatementContext {
private final IdGenerator<ObjectId> objectIdGenerator =
ObjectId.createGenerator();
+ private final IdGenerator<ObjectId> cteIdGenerator =
ObjectId.createGenerator();
Review Comment:
u need create a new class named CteId, and use CteId directly, not use
primitive type int
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCTE.java:
##########
@@ -40,14 +43,35 @@
private final List<LogicalSubQueryAlias<Plan>> aliasQueries;
+ private final Map<String, Integer> cteNameToId;
+
+ private final boolean registered;
+
public LogicalCTE(List<LogicalSubQueryAlias<Plan>> aliasQueries,
CHILD_TYPE child) {
- this(aliasQueries, Optional.empty(), Optional.empty(), child);
+ this(aliasQueries, Optional.empty(), Optional.empty(), child, false,
null);
+ }
+
+ public LogicalCTE(List<LogicalSubQueryAlias<Plan>> aliasQueries,
CHILD_TYPE child, boolean registered,
+ Map<String, Integer> cteNameToId) {
+ this(aliasQueries, Optional.empty(), Optional.empty(), child,
registered,
+ cteNameToId);
Review Comment:
```suggestion
public LogicalCTE(List<LogicalSubQueryAlias<Plan>> aliasQueries,
CHILD_TYPE child,
boolean registered, Map<String, Integer> cteNameToId) {
this(aliasQueries, Optional.empty(), Optional.empty(), child,
registered, cteNameToId);
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/StatsCalculator.java:
##########
@@ -131,12 +139,17 @@ public class StatsCalculator extends
DefaultPlanVisitor<Statistics, Void> {
private Map<String, Histogram> totalHistogramMap = new HashMap<>();
+ private Map<Integer, Statistics> cteIdToStats;
+
private StatsCalculator(GroupExpression groupExpression, boolean
forbidUnknownColStats,
- Map<String, ColumnStatistic>
columnStatisticMap, boolean isPlayNereidsDump) {
+ Map<String, ColumnStatistic>
columnStatisticMap, boolean isPlayNereidsDump,
+ Map<Integer, Statistics> cteIdToStats) {
this.groupExpression = groupExpression;
this.forbidUnknownColStats = forbidUnknownColStats;
this.totalColumnStatisticMap = columnStatisticMap;
this.isPlayNereidsDump = isPlayNereidsDump;
+ Preconditions.checkArgument(cteIdToStats != null);
+ this.cteIdToStats = cteIdToStats;
Review Comment:
```suggestion
this.cteIdToStats = Objects.requireNonNul(cteIdToStats, ...);
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCTEProducer.java:
##########
@@ -0,0 +1,143 @@
+// 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.trees.plans.logical;
+
+import org.apache.doris.nereids.memo.GroupExpression;
+import org.apache.doris.nereids.properties.LogicalProperties;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.PlanType;
+import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor;
+
+import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+
+/**
+ * LogicalCTEProducer
+ */
+public class LogicalCTEProducer<CHILD_TYPE extends Plan>
+ extends LogicalUnary<CHILD_TYPE> {
+
+ private final int cteId;
+
+ private final List<Slot> projects;
+
+ private final boolean rewritten;
+
+ public LogicalCTEProducer(CHILD_TYPE child, int cteId) {
+ super(PlanType.LOGICAL_CTE_PRODUCER, child);
+ this.cteId = cteId;
+ this.projects = Collections.emptyList();
Review Comment:
```suggestion
this.projects = ImmutableList.of();
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/visitor/PlanVisitor.java:
##########
@@ -350,6 +356,19 @@ public R visitPhysicalLimit(PhysicalLimit<? extends Plan>
limit, C context) {
return visit(limit, context);
}
+ public R visitPhysicalCTEProducer(PhysicalCTEProducer<? extends Plan>
cteProduceOperator, C context) {
+ return visit(cteProduceOperator, context);
+ }
+
+ public R visitPhysicalCTEConsumer(PhysicalCTEConsumer cteConsumeOperator,
C context) {
+ return visit(cteConsumeOperator, context);
+ }
+
+ public R visitPhysicalCTEAnchor(
+ PhysicalCTEAnchor<? extends Plan, ? extends Plan>
cteAnchorOperator, C context) {
+ return visit(cteAnchorOperator, context);
+ }
+
Review Comment:
```suggestion
public R visitPhysicalCTEProducer(PhysicalCTEProducer<? extends Plan>
cteProducer, C context) {
return visit(cteProducer, context);
}
public R visitPhysicalCTEConsumer(PhysicalCTEConsumer cteConsumer, C
context) {
return visit(cteConsumer, context);
}
public R visitPhysicalCTEAnchor(
PhysicalCTEAnchor<? extends Plan, ? extends Plan> cteAnchor, C
context) {
return visit(cteAnchor, context);
}
```
--
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-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
