morrySnow commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1401606207
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/NamedExpression.java:
##########
@@ -33,6 +33,7 @@ protected NamedExpression(List<Expression> children) {
}
public Slot toSlot() throws UnboundException {
+
Review Comment:
remove this blank line
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/FunctionalDependencies.java:
##########
@@ -0,0 +1,174 @@
+// 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.properties;
+
+import org.apache.doris.nereids.trees.expressions.Slot;
+
+import com.google.common.collect.ImmutableSet;
+
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Record functional dependencies
Review Comment:
add more comment to explain what types of FDS do we have
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/FunctionalDependencies.java:
##########
@@ -0,0 +1,174 @@
+// 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.properties;
+
+import org.apache.doris.nereids.trees.expressions.Slot;
+
+import com.google.common.collect.ImmutableSet;
+
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Record functional dependencies
+ */
+public class FunctionalDependencies {
+ NestedSet uniqueSet = new NestedSet();
+ NestedSet uniformSet = new NestedSet();
Review Comment:
private final?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalExcept.java:
##########
@@ -34,7 +35,7 @@
/**
* Logical Except.
*/
-public class LogicalExcept extends LogicalSetOperation {
+public class LogicalExcept extends LogicalSetOperation implements PropagateFD {
Review Comment:
why not add all columns as a unique cluster into FD?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/FunctionalDependencies.java:
##########
@@ -0,0 +1,174 @@
+// 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.properties;
+
+import org.apache.doris.nereids.trees.expressions.Slot;
+
+import com.google.common.collect.ImmutableSet;
+
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Record functional dependencies
+ */
+public class FunctionalDependencies {
+ NestedSet uniqueSet = new NestedSet();
+ NestedSet uniformSet = new NestedSet();
+
+ public FunctionalDependencies() {}
+
+ public FunctionalDependencies(FunctionalDependencies other) {
+ this.uniformSet = new NestedSet(other.uniformSet);
+ this.uniqueSet = new NestedSet(other.uniqueSet);
+ }
+
+ public void addUniformSlot(Slot slot) {
+ uniformSet.add(slot);
+ }
+
+ public void addUniformSlot(FunctionalDependencies functionalDependencies) {
+ uniformSet.add(functionalDependencies.uniformSet);
+ }
+
+ public void addUniformSlot(ImmutableSet<Slot> slotSet) {
+ uniformSet.add(slotSet);
+ }
+
+ public boolean isEmpty() {
+ return uniformSet.isEmpty() && uniqueSet.isEmpty();
+ }
+
+ public void pruneSlots(Set<Slot> outputSlots) {
+ uniformSet.removeNotContain(outputSlots);
+ uniqueSet.removeNotContain(outputSlots);
+ }
+
+ public void addUniqueSlot(Slot slot) {
+ uniqueSet.add(slot);
+ }
+
+ public void addUniqueSlot(ImmutableSet<Slot> slotSet) {
+ uniqueSet.add(slotSet);
+ }
+
+ public boolean isUnique(Slot slot) {
+ return uniqueSet.contains(slot);
+ }
+
+ public boolean isUnique(Set<Slot> slotSet) {
+ if (slotSet.isEmpty()) {
+ return false;
+ }
+ return uniqueSet.containsAnySub(slotSet);
+ }
+
+ public boolean isUniform(Slot slot) {
+ return uniformSet.contains(slot);
+ }
+
+ public boolean isUniform(ImmutableSet<Slot> slotSet) {
+ return uniformSet.contains(slotSet) ||
slotSet.stream().allMatch(uniformSet::contains);
+ }
+
+ public void addFunctionalDependencies(FunctionalDependencies fd) {
+ uniformSet.add(fd.uniformSet);
+ uniqueSet.add(fd.uniqueSet);
+ }
+
+ @Override
+ public String toString() {
+ return "uniform:" + uniformSet.toString()
+ + "\t unique:" + uniqueSet.toString();
+ }
+
+ class NestedSet {
+ Set<Slot> slots = new HashSet<>();
+ Set<ImmutableSet<Slot>> slotSets = new HashSet<>();
Review Comment:
it it better to use immutable data structure
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalRepeat.java:
##########
@@ -41,7 +42,7 @@
* LogicalRepeat.
*/
public class LogicalRepeat<CHILD_TYPE extends Plan> extends
LogicalUnary<CHILD_TYPE>
- implements Repeat<CHILD_TYPE> {
+ implements Repeat<CHILD_TYPE>, BlockFD {
Review Comment:
some uniform could be reserved
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/BlockFD.java:
##########
@@ -0,0 +1,34 @@
+// 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;
+
+import org.apache.doris.nereids.properties.FunctionalDependencies;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
+
+import java.util.List;
+
+/**
+ * Block fd
+ */
+public interface BlockFD extends LogicalPlan {
Review Comment:
add more comment on this class to explain where the name `block` come from
and what's mean
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalAggregate.java:
##########
@@ -282,4 +290,69 @@ public LogicalAggregate<Plan>
withNormalized(List<Expression> normalizedGroupBy,
return new LogicalAggregate<>(normalizedGroupBy, normalizedOutput,
true, ordinalIsResolved, generated,
hasPushed, sourceRepeat, Optional.empty(), Optional.empty(),
normalizedChild);
}
+
+ private void updateByAggregateFunctionn(NamedExpression namedExpression,
FunctionalDependencies fd) {
Review Comment:
typo
```suggestion
private void updateByAggregateFunction(NamedExpression namedExpression,
FunctionalDependencies fd) {
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/FunctionalDependencies.java:
##########
@@ -0,0 +1,174 @@
+// 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.properties;
+
+import org.apache.doris.nereids.trees.expressions.Slot;
+
+import com.google.common.collect.ImmutableSet;
+
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Record functional dependencies
+ */
+public class FunctionalDependencies {
Review Comment:
should override hashCode and equals?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/PropagateFD.java:
##########
@@ -0,0 +1,43 @@
+// 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;
+
+import org.apache.doris.nereids.properties.FunctionalDependencies;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
+
+import java.util.List;
+
+/**
+ * Propage fd
+ */
+public interface PropagateFD extends LogicalPlan {
Review Comment:
same as blockFD
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalAggregate.java:
##########
@@ -282,4 +290,69 @@ public LogicalAggregate<Plan>
withNormalized(List<Expression> normalizedGroupBy,
return new LogicalAggregate<>(normalizedGroupBy, normalizedOutput,
true, ordinalIsResolved, generated,
hasPushed, sourceRepeat, Optional.empty(), Optional.empty(),
normalizedChild);
}
+
+ private void updateByAggregateFunctionn(NamedExpression namedExpression,
FunctionalDependencies fd) {
+ if (namedExpression instanceof Slot) {
+ fd.addUniqueSlot(namedExpression.toSlot());
+ return;
+ }
+
+ if (!(namedExpression instanceof Alias && namedExpression.child(0)
instanceof AggregateFunction)) {
+ return;
+ }
+
+ AggregateFunction agg = (AggregateFunction) namedExpression.child(0);
+ if (agg instanceof Count || agg instanceof Ndv) {
+ fd.addUniformSlot(namedExpression.toSlot());
+ return;
+ }
+
+ if (ExpressionUtils.isInjectiveAgg(agg)
+ &&
child().getLogicalProperties().getFunctionalDependencies().isUnique(agg.getInputSlots()))
{
+ fd.addUniqueSlot(namedExpression.toSlot());
+ }
+ }
+
+ @Override
+ public FunctionalDependencies computeFD(List<Slot> outputs) {
Review Comment:
i think it is better use full name rather than acronym
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalJoin.java:
##########
@@ -357,6 +362,90 @@ public LogicalJoin<Plan, Plan>
withOtherJoinConjuncts(List<Expression> otherJoin
markJoinSlotReference, children);
}
+ private @Nullable Pair<Set<Slot>, Set<Slot>> extractHashKeys() {
+ Set<Slot> leftKeys = new HashSet<>();
+ Set<Slot> rightKeys = new HashSet<>();
+ for (Expression expression : hashJoinConjuncts) {
+ if (!(expression instanceof EqualTo
Review Comment:
should we need to think about null safe equal when left and right are both
not null?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalGenerate.java:
##########
@@ -39,7 +40,7 @@
/**
* plan for table generator, the statement like: SELECT * FROM tbl LATERAL
VIEW EXPLODE(c1) g as (gc1);
*/
-public class LogicalGenerate<CHILD_TYPE extends Plan> extends
LogicalUnary<CHILD_TYPE> implements Generate {
+public class LogicalGenerate<CHILD_TYPE extends Plan> extends
LogicalUnary<CHILD_TYPE> implements Generate, BlockFD {
Review Comment:
i think uniform still working after generate
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalLimit.java:
##########
@@ -140,4 +141,14 @@ public LogicalLimit<Plan> withChildren(List<Plan>
children) {
Preconditions.checkArgument(children.size() == 1);
return new LogicalLimit<>(limit, offset, phase, children.get(0));
}
+
+ @Override
+ public FunctionalDependencies computeFD(List<Slot> outputs) {
+ FunctionalDependencies functionalDependencies = new
FunctionalDependencies(
+ child(0).getLogicalProperties().getFunctionalDependencies());
+ if (getLimit() == 1) {
+ outputs.forEach(functionalDependencies::addUniformSlot);
Review Comment:
need to add to unique?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalSubQueryAlias.java:
##########
@@ -39,7 +40,7 @@
*
* @param <CHILD_TYPE> param
*/
-public class LogicalSubQueryAlias<CHILD_TYPE extends Plan> extends
LogicalUnary<CHILD_TYPE> {
+public class LogicalSubQueryAlias<CHILD_TYPE extends Plan> extends
LogicalUnary<CHILD_TYPE> implements PropagateFD {
Review Comment:
after LogicalSubQueryAlias, the slot expr id of output slots changed, so
need computeFD?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalTopN.java:
##########
@@ -148,4 +149,14 @@ public Plan
withGroupExprLogicalPropChildren(Optional<GroupExpression> groupExpr
Preconditions.checkArgument(children.size() == 1);
return new LogicalTopN<>(orderKeys, limit, offset, groupExpression,
logicalProperties, children.get(0));
}
+
+ @Override
+ public FunctionalDependencies computeFD(List<Slot> outputs) {
+ FunctionalDependencies functionalDependencies = new
FunctionalDependencies(
+ child(0).getLogicalProperties().getFunctionalDependencies());
+ if (getLimit() == 1) {
+ outputs.forEach(functionalDependencies::addUniformSlot);
Review Comment:
same as limit
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalProject.java:
##########
@@ -200,4 +203,17 @@ public JSONObject toJson() {
logicalProject.put("Properties", properties);
return logicalProject;
}
+
+ @Override
+ public FunctionalDependencies computeFD(List<Slot> outputs) {
+ FunctionalDependencies fd = new FunctionalDependencies(
+ child().getLogicalProperties().getFunctionalDependencies());
+ fd.pruneSlots(new HashSet<>(outputs));
+ for (NamedExpression proj : projects) {
+ if (proj instanceof Alias && proj.child(0).isConstant()) {
+ fd.addUniformSlot(proj.toSlot());
+ }
+ }
Review Comment:
maybe we should consider `uuid()` as unique?
--
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
