Copilot commented on code in PR #59872: URL: https://github.com/apache/doris/pull/59872#discussion_r2692952919
########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalWorkTableReference.java: ########## @@ -0,0 +1,152 @@ +// 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.physical; + +import org.apache.doris.nereids.memo.GroupExpression; +import org.apache.doris.nereids.properties.DataTrait; +import org.apache.doris.nereids.properties.LogicalProperties; +import org.apache.doris.nereids.properties.PhysicalProperties; +import org.apache.doris.nereids.trees.expressions.CTEId; +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.RelationId; +import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor; +import org.apache.doris.nereids.util.Utils; +import org.apache.doris.statistics.Statistics; + +import java.util.List; +import java.util.Objects; +import java.util.Optional; + +/** + * PhysicalRecursiveCteScan. + */ +public class PhysicalWorkTableReference extends PhysicalRelation { + private CTEId cteId; + private final List<Slot> outputs; + private final List<String> nameParts; + + public PhysicalWorkTableReference(RelationId relationId, CTEId cteId, List<Slot> outputs, List<String> nameParts, + Optional<GroupExpression> groupExpression, LogicalProperties logicalProperties) { + this(relationId, cteId, outputs, nameParts, groupExpression, logicalProperties, PhysicalProperties.ANY, null); + } + + public PhysicalWorkTableReference(RelationId relationId, CTEId cteId, List<Slot> outputs, List<String> nameParts, + Optional<GroupExpression> groupExpression, LogicalProperties logicalProperties, + PhysicalProperties physicalProperties, Statistics statistics) { + super(relationId, PlanType.PHYSICAL_WORK_TABLE_REFERENCE, groupExpression, logicalProperties, + physicalProperties, statistics); + this.cteId = cteId; + this.outputs = Objects.requireNonNull(outputs); + this.nameParts = Objects.requireNonNull(nameParts); + } + + public CTEId getCteId() { + return cteId; + } + + public List<String> getNameParts() { + return nameParts; + } + + public String getTableName() { + return nameParts.stream().map(Utils::quoteIfNeeded) + .reduce((left, right) -> left + "." + right).orElse(""); + } + + @Override + public <R, C> R accept(PlanVisitor<R, C> visitor, C context) { + return visitor.visitPhysicalWorkTableReference(this, context); + } + + @Override + public Plan withGroupExpression(Optional<GroupExpression> groupExpression) { + return new PhysicalWorkTableReference(relationId, cteId, outputs, nameParts, groupExpression, + getLogicalProperties(), + physicalProperties, statistics); + } + + @Override + public Plan withGroupExprLogicalPropChildren(Optional<GroupExpression> groupExpression, + Optional<LogicalProperties> logicalProperties, List<Plan> children) { + return new PhysicalWorkTableReference(relationId, cteId, outputs, nameParts, groupExpression, + getLogicalProperties(), + physicalProperties, statistics); + } + + @Override + public void computeUnique(DataTrait.Builder builder) { + + } + + @Override + public void computeUniform(DataTrait.Builder builder) { + + } + + @Override + public void computeEqualSet(DataTrait.Builder builder) { + + } + + @Override + public void computeFd(DataTrait.Builder builder) { + + } + + @Override + public PhysicalPlan withPhysicalPropertiesAndStats(PhysicalProperties physicalProperties, Statistics statistics) { + return new PhysicalWorkTableReference(relationId, cteId, outputs, nameParts, groupExpression, + getLogicalProperties(), + physicalProperties, statistics); + } + + @Override + public List<Slot> getOutput() { + return outputs; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + if (!super.equals(o)) { + return false; + } + PhysicalWorkTableReference that = (PhysicalWorkTableReference) o; + return cteId.equals(that.cteId) && nameParts.equals(that.nameParts) && outputs.equals(outputs); Review Comment: In the equals method, `outputs.equals(outputs)` compares outputs to itself instead of comparing to `that.outputs`. This should be `outputs.equals(that.outputs)`. ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalRecursiveUnionAnchor.java: ########## @@ -0,0 +1,144 @@ +// 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.physical; + +import org.apache.doris.nereids.memo.GroupExpression; +import org.apache.doris.nereids.properties.DataTrait; +import org.apache.doris.nereids.properties.LogicalProperties; +import org.apache.doris.nereids.properties.PhysicalProperties; +import org.apache.doris.nereids.trees.expressions.CTEId; +import org.apache.doris.nereids.trees.expressions.Expression; +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 org.apache.doris.nereids.util.Utils; +import org.apache.doris.statistics.Statistics; + +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; +import org.jetbrains.annotations.Nullable; + +import java.util.List; +import java.util.Objects; +import java.util.Optional; + +/** + * PhysicalRecursiveUnionAnchor is sentinel plan for must_shuffle + */ +public class PhysicalRecursiveUnionAnchor<CHILD_TYPE extends Plan> extends PhysicalUnary<CHILD_TYPE> { + private final CTEId cteId; + + public PhysicalRecursiveUnionAnchor(CTEId cteId, LogicalProperties logicalProperties, CHILD_TYPE child) { + this(cteId, Optional.empty(), logicalProperties, child); + } + + public PhysicalRecursiveUnionAnchor(CTEId cteId, Optional<GroupExpression> groupExpression, + LogicalProperties logicalProperties, CHILD_TYPE child) { + this(cteId, groupExpression, logicalProperties, PhysicalProperties.ANY, null, child); + } + + public PhysicalRecursiveUnionAnchor(CTEId cteId, Optional<GroupExpression> groupExpression, + LogicalProperties logicalProperties, @Nullable PhysicalProperties physicalProperties, Statistics statistics, + CHILD_TYPE child) { + super(PlanType.PHYSICAL_RECURSIVE_CTE_RECURSIVE_CHILD, groupExpression, logicalProperties, physicalProperties, + statistics, child); + this.cteId = cteId; + } + + public CTEId getCteId() { + return cteId; + } + + @Override + public String toString() { + return Utils.toSqlStringSkipNull("PhysicalRecursiveCteRecursiveChild", Review Comment: The toString() method uses an outdated class name 'PhysicalRecursiveCteRecursiveChild' instead of the current class name 'PhysicalRecursiveUnionAnchor'. ########## fe/fe-core/src/main/java/org/apache/doris/nereids/stats/StatsCalculator.java: ########## @@ -183,6 +185,7 @@ public class StatsCalculator extends DefaultPlanVisitor<Statistics, Void> { public static double DEFAULT_AGGREGATE_RATIO = 1 / 3.0; public static double AGGREGATE_COLUMN_CORRELATION_COEFFICIENT = 0.75; public static double DEFAULT_COLUMN_NDV_RATIO = 0.5; + public static double RECURSIVE_CTE_EXPAND_RATION = 5.0; Review Comment: Corrected spelling of 'RATION' to 'RATIO'. ```suggestion public static double RECURSIVE_CTE_EXPAND_RATIO = 5.0; @Deprecated public static double RECURSIVE_CTE_EXPAND_RATION = RECURSIVE_CTE_EXPAND_RATIO; ``` ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalWorkTableReference.java: ########## @@ -0,0 +1,149 @@ +// 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.DataTrait; +import org.apache.doris.nereids.properties.LogicalProperties; +import org.apache.doris.nereids.trees.expressions.CTEId; +import org.apache.doris.nereids.trees.expressions.Slot; +import org.apache.doris.nereids.trees.expressions.SlotReference; +import org.apache.doris.nereids.trees.plans.Plan; +import org.apache.doris.nereids.trees.plans.PlanType; +import org.apache.doris.nereids.trees.plans.RelationId; +import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor; +import org.apache.doris.nereids.util.Utils; + +import com.google.common.collect.ImmutableList; + +import java.util.List; +import java.util.Objects; +import java.util.Optional; + +/** + * LogicalWorkTableReference, recursive union's producer child will scan WorkTableReference in every iteration. + */ +public class LogicalWorkTableReference extends LogicalRelation { + private CTEId cteId; + private final List<Slot> outputs; + private final List<String> nameParts; + + public LogicalWorkTableReference(RelationId relationId, CTEId cteId, List<Slot> outputs, List<String> nameParts) { + this(relationId, cteId, outputs, nameParts, Optional.empty(), Optional.empty()); + } + + private LogicalWorkTableReference(RelationId relationId, CTEId cteId, List<Slot> outputs, List<String> nameParts, + Optional<GroupExpression> groupExpression, Optional<LogicalProperties> logicalProperties) { + super(relationId, PlanType.LOGICAL_WORK_TABLE_REFERENCE, groupExpression, logicalProperties); + this.cteId = cteId; + this.outputs = Objects.requireNonNull(outputs); + this.nameParts = Objects.requireNonNull(nameParts); + } + + public List<String> getNameParts() { + return nameParts; + } + + public String getTableName() { + return nameParts.stream().map(Utils::quoteIfNeeded) + .reduce((left, right) -> left + "." + right).orElse(""); + } + + public CTEId getCteId() { + return cteId; + } + + @Override + public String toString() { + return Utils.toSqlString("LogicalWorkTableReference", + "cteId", cteId, + "cteName", getTableName()); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + if (!super.equals(o)) { + return false; + } + LogicalWorkTableReference that = (LogicalWorkTableReference) o; + return cteId.equals(that.cteId) && nameParts.equals(that.nameParts) && outputs.equals(outputs); Review Comment: In the equals method, `outputs.equals(outputs)` compares outputs to itself instead of comparing to `that.outputs`. This should be `outputs.equals(that.outputs)`. ########## fe/fe-core/src/main/java/org/apache/doris/nereids/stats/StatsCalculator.java: ########## @@ -1519,7 +1578,7 @@ public Statistics computeEmptyRelation(EmptyRelation emptyRelation) { * computeRecursiveCte */ public Statistics computeRecursiveCte(RecursiveCte recursiveCte, List<Statistics> childStats) { - // TODO: refactor this for one row relation + // simliar as computeUnion Review Comment: Corrected spelling of 'simliar' to 'similar'. ```suggestion // similar as computeUnion ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
