You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "XieJiann (via GitHub)" <gi...@apache.org> on 2023/11/15 10:12:24 UTC
[PR] [feature](Nereids) add functional dependencies [doris]
XieJiann opened a new pull request, #27051:
URL: https://github.com/apache/doris/pull/27051
## Proposed changes
Issue Number: close #xxx
<!--Describe your changes.-->
## 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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1396732287
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/Command.java:
##########
@@ -36,7 +37,7 @@
/**
* All DDL and DML commands' super class.
*/
-public abstract class Command extends AbstractPlan implements LogicalPlan {
+public abstract class Command extends AbstractPlan implements LogicalPlan, BlockFD {
Review Comment:
No, Implicit implementation is always bad
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1396731134
##########
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:
blocking means blocking propagate fd
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1822376586
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1401910430
##########
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:
It shouldn't be necessary. Functional Dependencies shouldn't be hashed or compared.
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "xzj7019 (via GitHub)" <gi...@apache.org>.
xzj7019 commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1406083746
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalWindow.java:
##########
@@ -224,4 +227,46 @@ public Optional<Plan> pushPartitionLimitThroughWindow(long partitionLimit, boole
return Optional.ofNullable(window);
}
+
+ private void updateFuncDepsByWindowExpr(NamedExpression namedExpression, FunctionalDependencies.Builder builder) {
+ if (namedExpression.children().size() != 1 || !(namedExpression.child(0) instanceof WindowExpression)) {
+ return;
+ }
+ WindowExpression windowExpr = (WindowExpression) namedExpression.child(0);
+ List<Expression> partitionKeys = windowExpr.getPartitionKeys();
+
+ // window func should return unique results for different rows
+ if (!(windowExpr.getFunction() instanceof RowNumber
+ || windowExpr.getFunction() instanceof Rank)) {
Review Comment:
rank doesn't meet the requirement.
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1829297779
<details>
<summary>TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'</summary>
```
Tpch sf100 test result on commit 5378641e12bd50eddb508b457a9a7070bd90f7f0, data reload: false
run tpch-sf100 query with default conf and session variables
q1 4919 4640 4641 4640
q2 363 159 157 157
q3 2037 1950 1871 1871
q4 1393 1263 1248 1248
q5 3944 3933 4016 3933
q6 257 130 135 130
q7 1362 842 836 836
q8 2814 2789 2771 2771
q9 10129 9510 9413 9413
q10 3464 3536 3509 3509
q11 387 250 254 250
q12 439 296 295 295
q13 4592 3847 3852 3847
q14 326 288 292 288
q15 591 543 525 525
q16 495 436 436 436
q17 1148 977 921 921
q18 7903 7490 7543 7490
q19 1675 1670 1670 1670
q20 546 307 302 302
q21 4384 4043 4043 4043
q22 464 368 386 368
Total cold run time: 53632 ms
Total hot run time: 48943 ms
run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1 4601 4596 4586 4586
q2 340 226 259 226
q3 4032 4028 3994 3994
q4 2733 2707 2708 2707
q5 9646 9654 9605 9605
q6 252 124 123 123
q7 2990 2436 2476 2436
q8 4424 4447 4463 4447
q9 13085 12761 12965 12761
q10 4072 4157 4181 4157
q11 778 654 680 654
q12 985 831 836 831
q13 4313 3570 3558 3558
q14 383 356 349 349
q15 582 512 519 512
q16 601 528 600 528
q17 3906 3804 3870 3804
q18 9715 9033 9230 9033
q19 1821 1779 1796 1779
q20 2391 2069 2027 2027
q21 8748 8658 8591 8591
q22 861 827 784 784
Total cold run time: 81259 ms
Total hot run time: 77492 ms
```
</details>
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "morrySnow (via GitHub)" <gi...@apache.org>.
morrySnow commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1408668354
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraph/receiver/PlanReceiver.java:
##########
@@ -378,7 +379,9 @@ private List<Plan> proposeProject(List<Plan> allChild, List<Edge> edges, long le
.collect(Collectors.toList());
if (!outputSet.equals(new HashSet<>(projects))) {
LogicalProperties projectProperties = new LogicalProperties(
- () -> projects.stream().map(p -> p.toSlot()).collect(Collectors.toList()));
+ () -> projects.stream()
+ .map(p -> p.toSlot())
+ .collect(Collectors.toList()), () -> FunctionalDependencies.EMPTY_FUNC_DEPS);
Review Comment:
NIT: use ImmutableList collector
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/FunctionalDependencies.java:
##########
@@ -0,0 +1,232 @@
+// 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.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Record functional dependencies, aka func deps, including
+ * 1. unique slot: it means the column that has ndv = row count, can be null
+ * 2. uniform slotL it means the column that has ndv = 1, can be null
+ */
+public class FunctionalDependencies {
+ public static final FunctionalDependencies EMPTY_FUNC_DEPS
+ = new FunctionalDependencies(new NestedSet().toImmutable(), new NestedSet().toImmutable());
Review Comment:
add a blank between public and private static var
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCatalogRelation.java:
##########
@@ -113,4 +118,18 @@ public String qualifiedName() {
}
return Utils.qualifiedName(qualifier, table.getName());
}
+
+ @Override
+ public FunctionalDependencies computeFuncDeps(Supplier<List<Slot>> outputSupplier) {
+ Builder fdBuilder = new Builder();
+ if (table instanceof OlapTable && ((OlapTable) table).getKeysType().isAggregationFamily()) {
+ ImmutableSet<Slot> slotSet = computeOutput().stream()
+ .filter(SlotReference.class::isInstance)
Review Comment:
nit: add a map to cast to SlotReference
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCheckPolicy.java:
##########
@@ -46,7 +47,8 @@
/**
* Logical Check Policy
*/
-public class LogicalCheckPolicy<CHILD_TYPE extends Plan> extends LogicalUnary<CHILD_TYPE> {
+public class LogicalCheckPolicy<CHILD_TYPE extends Plan> extends LogicalUnary<CHILD_TYPE> implements
Review Comment:
nit: wrap before implements is better
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraph/receiver/PlanReceiver.java:
##########
@@ -378,7 +379,9 @@ private List<Plan> proposeProject(List<Plan> allChild, List<Edge> edges, long le
.collect(Collectors.toList());
if (!outputSet.equals(new HashSet<>(projects))) {
LogicalProperties projectProperties = new LogicalProperties(
- () -> projects.stream().map(p -> p.toSlot()).collect(Collectors.toList()));
+ () -> projects.stream()
+ .map(p -> p.toSlot())
Review Comment:
nit: use lambda style
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/FunctionalDependencies.java:
##########
@@ -0,0 +1,232 @@
+// 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.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Record functional dependencies, aka func deps, including
+ * 1. unique slot: it means the column that has ndv = row count, can be null
+ * 2. uniform slotL it means the column that has ndv = 1, can be null
+ */
+public class FunctionalDependencies {
+ public static final FunctionalDependencies EMPTY_FUNC_DEPS
+ = new FunctionalDependencies(new NestedSet().toImmutable(), new NestedSet().toImmutable());
+ private final NestedSet uniqueSet;
+ private final NestedSet uniformSet;
+
+ private FunctionalDependencies(NestedSet uniqueSet, NestedSet uniformSet) {
+ this.uniqueSet = uniqueSet;
+ this.uniformSet = uniformSet;
+ }
+
+ public boolean isEmpty() {
+ return uniformSet.isEmpty() && uniqueSet.isEmpty();
+ }
+
+ public boolean isUnique(Slot slot) {
+ return uniqueSet.contains(slot);
+ }
+
+ public boolean isUnique(Set<Slot> slotSet) {
+ return !slotSet.isEmpty() && uniqueSet.containsAnySub(slotSet);
+ }
+
+ public boolean isUniform(Slot slot) {
+ return uniformSet.contains(slot);
+ }
+
+ public boolean isUniform(ImmutableSet<Slot> slotSet) {
+ return !slotSet.isEmpty()
+ && (uniformSet.contains(slotSet) || slotSet.stream().allMatch(uniformSet::contains));
+ }
+
+ public boolean isUniqueAndNotNull(Slot slot) {
+ return !slot.nullable() && isUnique(slot);
+ }
+
+ public boolean isUniqueAndNotNull(Set<Slot> slotSet) {
+ return slotSet.stream().noneMatch(Slot::nullable) && isUnique(slotSet);
+ }
+
+ public boolean isUniformAndNotNull(Slot slot) {
+ return !slot.nullable() && isUniform(slot);
+ }
+
+ public boolean isUniformAndNotNull(ImmutableSet<Slot> slotSet) {
+ return slotSet.stream().noneMatch(Slot::nullable) && isUniform(slotSet);
+ }
+
+ @Override
+ public String toString() {
+ return "uniform:" + uniformSet
Review Comment:
nit: use uniform style toString as Plan is better
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1814672951
(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 45.38 seconds
stream load tsv: 569 seconds loaded 74807831229 Bytes, about 125 MB/s
stream load json: 18 seconds loaded 2358488459 Bytes, about 124 MB/s
stream load orc: 65 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 28.6 seconds inserted 10000000 Rows, about 349K ops/s
storage size: 17099365037 Bytes
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1813740461
run buildall
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "xzj7019 (via GitHub)" <gi...@apache.org>.
xzj7019 commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1406079017
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalPartitionTopN.java:
##########
@@ -200,4 +204,24 @@ public Plan withGroupExprLogicalPropChildren(Optional<GroupExpression> groupExpr
return new LogicalPartitionTopN<>(function, partitionKeys, orderKeys, hasGlobalLimit, partitionLimit,
groupExpression, logicalProperties, children.get(0));
}
+
+ @Override
+ public FunctionalDependencies computeFuncDeps(Supplier<List<Slot>> outputSupplier) {
Review Comment:
should the same as filter.
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1829273547
(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 46.63 seconds
stream load tsv: 568 seconds loaded 74807831229 Bytes, about 125 MB/s
stream load json: 27 seconds loaded 2358488459 Bytes, about 83 MB/s
stream load orc: 67 seconds loaded 1101869774 Bytes, about 15 MB/s
stream load parquet: 33 seconds loaded 861443392 Bytes, about 24 MB/s
insert into select: 28.2 seconds inserted 10000000 Rows, about 354K ops/s
storage size: 17098601985 Bytes
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "xzj7019 (via GitHub)" <gi...@apache.org>.
xzj7019 commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1407389082
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalWindow.java:
##########
@@ -224,4 +227,52 @@ public Optional<Plan> pushPartitionLimitThroughWindow(long partitionLimit, boole
return Optional.ofNullable(window);
}
+
+ private void updateFuncDepsByWindowExpr(NamedExpression namedExpression, FunctionalDependencies.Builder builder) {
+ if (namedExpression.children().size() != 1 || !(namedExpression.child(0) instanceof WindowExpression)) {
+ return;
+ }
+ WindowExpression windowExpr = (WindowExpression) namedExpression.child(0);
+ List<Expression> partitionKeys = windowExpr.getPartitionKeys();
+
+ // window func should return unique results for different rows
+ if (!(windowExpr.getFunction() instanceof RowNumber
+ || windowExpr.getFunction() instanceof Rank)) {
+ return;
+ }
+
+ // Now we only support slot type keys
+ if (!partitionKeys.stream().allMatch(Slot.class::isInstance)) {
+ return;
+ }
+ ImmutableSet<Slot> slotSet = partitionKeys.stream()
+ .map(s -> (Slot) s)
+ .collect(ImmutableSet.toImmutableSet());
+
+ // if partition by keys are unique, output is uniform
+ if (child(0).getLogicalProperties().getFunctionalDependencies().isUniqueAndNotNull(slotSet)) {
+ if (windowExpr.getFunction() instanceof RowNumber
+ || windowExpr.getFunction() instanceof Rank
+ || windowExpr.getFunction() instanceof DenseRank) {
Review Comment:
1. for unique and not null, i think the function list is not limited for those three, may be more...
2. DenseRank is not allowed at line 240 before.
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "morrySnow (via GitHub)" <gi...@apache.org>.
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-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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1402079299
##########
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:
Null safe equal will produce more rows when there are multiple nulls. It depends whether the left/right can be null
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1823815477
(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 44.13 seconds
stream load tsv: 585 seconds loaded 74807831229 Bytes, about 121 MB/s
stream load json: 18 seconds loaded 2358488459 Bytes, about 124 MB/s
stream load orc: 67 seconds loaded 1101869774 Bytes, about 15 MB/s
stream load parquet: 33 seconds loaded 861443392 Bytes, about 24 MB/s
insert into select: 28.8 seconds inserted 10000000 Rows, about 347K ops/s
storage size: 17100640422 Bytes
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1407395359
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalWindow.java:
##########
@@ -224,4 +227,52 @@ public Optional<Plan> pushPartitionLimitThroughWindow(long partitionLimit, boole
return Optional.ofNullable(window);
}
+
+ private void updateFuncDepsByWindowExpr(NamedExpression namedExpression, FunctionalDependencies.Builder builder) {
+ if (namedExpression.children().size() != 1 || !(namedExpression.child(0) instanceof WindowExpression)) {
+ return;
+ }
+ WindowExpression windowExpr = (WindowExpression) namedExpression.child(0);
+ List<Expression> partitionKeys = windowExpr.getPartitionKeys();
+
+ // window func should return unique results for different rows
+ if (!(windowExpr.getFunction() instanceof RowNumber
+ || windowExpr.getFunction() instanceof Rank)) {
+ return;
+ }
+
+ // Now we only support slot type keys
+ if (!partitionKeys.stream().allMatch(Slot.class::isInstance)) {
+ return;
+ }
+ ImmutableSet<Slot> slotSet = partitionKeys.stream()
+ .map(s -> (Slot) s)
+ .collect(ImmutableSet.toImmutableSet());
+
+ // if partition by keys are unique, output is uniform
+ if (child(0).getLogicalProperties().getFunctionalDependencies().isUniqueAndNotNull(slotSet)) {
+ if (windowExpr.getFunction() instanceof RowNumber
+ || windowExpr.getFunction() instanceof Rank
+ || windowExpr.getFunction() instanceof DenseRank) {
Review Comment:
Yeah, I forgot to delete loc 240.
For 1, there may be a lot functions, but I have not handled them systematically. May be we can do it later
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "xzj7019 (via GitHub)" <gi...@apache.org>.
xzj7019 commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1407383734
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalIntersect.java:
##########
@@ -98,4 +104,23 @@ public LogicalIntersect withNewOutputs(List<NamedExpression> newOutputs) {
return new LogicalIntersect(qualifier, newOutputs, regularChildrenOutputs,
Optional.empty(), Optional.empty(), children);
}
+
+ @Override
+ public FunctionalDependencies computeFuncDeps(Supplier<List<Slot>> outputSupplier) {
+ FunctionalDependencies.Builder builder = new FunctionalDependencies
+ .Builder(child(0).getLogicalProperties().getFunctionalDependencies());
Review Comment:
for intersect, right side fd also can be propagated.
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1396731837
##########
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 {
+ @Override
+ default FunctionalDependencies computeFD(List<Slot> outputs) {
+ return new FunctionalDependencies();
Review Comment:
最好不要隐式作为默认的方法，这样之后新写一个plan会忽略到底需不需要传递。现在只是implement 一个借口，也比较好写
##########
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 {
+ @Override
+ default FunctionalDependencies computeFD(List<Slot> outputs) {
+ return new FunctionalDependencies();
Review Comment:
最好不要隐式作为默认的方法，这样之后新写一个plan会忽略到底需不需要传递。现在只是implement 一个借口，也比较好写
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "xzj7019 (via GitHub)" <gi...@apache.org>.
xzj7019 commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1406084761
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalWindow.java:
##########
@@ -224,4 +227,46 @@ public Optional<Plan> pushPartitionLimitThroughWindow(long partitionLimit, boole
return Optional.ofNullable(window);
}
+
+ private void updateFuncDepsByWindowExpr(NamedExpression namedExpression, FunctionalDependencies.Builder builder) {
+ if (namedExpression.children().size() != 1 || !(namedExpression.child(0) instanceof WindowExpression)) {
+ return;
+ }
+ WindowExpression windowExpr = (WindowExpression) namedExpression.child(0);
+ List<Expression> partitionKeys = windowExpr.getPartitionKeys();
+
+ // window func should return unique results for different rows
+ if (!(windowExpr.getFunction() instanceof RowNumber
+ || windowExpr.getFunction() instanceof Rank)) {
+ return;
+ }
+
+ // Now we only support slot type keys
+ if (!partitionKeys.stream().allMatch(Slot.class::isInstance)) {
+ return;
+ }
+ ImmutableSet<Slot> slotSet = partitionKeys.stream()
+ .map(s -> (Slot) s)
+ .collect(ImmutableSet.toImmutableSet());
+
+ // if partition by keys are unique, output is uniform
+ if (child(0).getLogicalProperties().getFunctionalDependencies().isUniqueAndNotNull(slotSet)) {
Review Comment:
this is ok for rank
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "xzj7019 (via GitHub)" <gi...@apache.org>.
xzj7019 commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1406065279
##########
fe/fe-core/src/test/java/org/apache/doris/nereids/properties/FunctionalDependenciesTest.java:
##########
@@ -0,0 +1,383 @@
+// 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.properties.FunctionalDependencies.Builder;
+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.logical.LogicalPartitionTopN;
+import org.apache.doris.nereids.types.IntegerType;
+import org.apache.doris.nereids.util.PlanChecker;
+import org.apache.doris.utframe.TestWithFeService;
+
+import com.google.common.collect.ImmutableSet;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+class FunctionalDependenciesTest extends TestWithFeService {
+ Slot slot1 = new SlotReference("1", IntegerType.INSTANCE, false);
+ Slot slot2 = new SlotReference("2", IntegerType.INSTANCE, false);
+ Slot slot3 = new SlotReference("1", IntegerType.INSTANCE, false);
+ Slot slot4 = new SlotReference("1", IntegerType.INSTANCE, false);
+
+ @Override
+ protected void runBeforeAll() throws Exception {
+ createDatabase("test");
+ createTable("create table test.agg (\n"
+ + "id int not null,\n"
+ + "name varchar(128) replace not null )\n"
+ + "AGGREGATE KEY(id)\n"
+ + "distributed by hash(id) buckets 10\n"
+ + "properties('replication_num' = '1');");
+ createTable("create table test.uni (\n"
+ + "id int not null,\n"
+ + "name varchar(128) not null)\n"
+ + "UNIQUE KEY(id)\n"
+ + "distributed by hash(id) buckets 10\n"
+ + "properties('replication_num' = '1');");
+ connectContext.setDatabase("default_cluster:test");
+ }
+
+ @Test
+ void testUniform() {
+ Builder fdBuilder = new Builder();
+ fdBuilder.addUniformSlot(slot1);
+ FunctionalDependencies fd = fdBuilder.build();
+ Assertions.assertTrue(fd.isUniformAndNotNull(slot1));
+ Assertions.assertFalse(fd.isUniformAndNotNull(slot2));
+ fdBuilder.addUniformSlot(ImmutableSet.of(slot2));
+ fd = fdBuilder.build();
+ Assertions.assertTrue(fd.isUniformAndNotNull(slot2));
+ ImmutableSet<Slot> slotSet = ImmutableSet.of(slot1, slot2, slot3);
+ fdBuilder.addUniformSlot(slotSet);
+ fd = fdBuilder.build();
+ Assertions.assertTrue(fd.isUniformAndNotNull(slotSet));
+ Assertions.assertFalse(fd.isUniformAndNotNull(ImmutableSet.of(slot1, slot2, slot3, slot4)));
+ Assertions.assertTrue(fd.isUniformAndNotNull(ImmutableSet.of(slot1, slot2)));
+ Assertions.assertFalse(fd.isUniformAndNotNull(ImmutableSet.of(slot3, slot2)));
+ }
+
+ @Test
+ void testUnique() {
+ Builder fdBuilder = new Builder();
+ fdBuilder.addUniqueSlot(slot1);
+ FunctionalDependencies fd = fdBuilder.build();
+ Assertions.assertTrue(fd.isUniqueAndNotNull(slot1));
+ Assertions.assertFalse(fd.isUniqueAndNotNull(slot2));
+ fdBuilder.addUniqueSlot(slot2);
+ fd = fdBuilder.build();
+ Assertions.assertTrue(fd.isUniqueAndNotNull(slot2));
+ ImmutableSet<Slot> slotSet = ImmutableSet.of(slot1, slot2, slot3);
+ fdBuilder.addUniqueSlot(slotSet);
+ fd = fdBuilder.build();
+ Assertions.assertTrue(fd.isUniqueAndNotNull(slotSet));
+ Assertions.assertTrue(fd.isUniqueAndNotNull(ImmutableSet.of(slot1, slot2, slot3, slot4)));
+ Assertions.assertFalse(fd.isUniqueAndNotNull(ImmutableSet.of(slot3, slot4)));
+ }
+
+ @Test
+ void testMergeFD() {
+ Builder fdBuilder1 = new Builder();
+ fdBuilder1.addUniformSlot(slot1);
+ Builder fdBuilder2 = new Builder();
+ fdBuilder2.addUniformSlot(slot2);
+
+ fdBuilder1.addFunctionalDependencies(fdBuilder2.build());
+ FunctionalDependencies fd = fdBuilder1.build();
+ Assertions.assertTrue(fd.isUniformAndNotNull(slot1));
+ Assertions.assertTrue(fd.isUniformAndNotNull(slot2));
+ }
+
+ @Test
+ void testScan() {
+ Plan plan = PlanChecker.from(connectContext)
+ .analyze("select id from agg")
+ .rewrite()
+ .getPlan();
+ Assertions.assertTrue(plan.getLogicalProperties().getFunctionalDependencies()
+ .isUniqueAndNotNull(plan.getOutput().get(0)));
+ plan = PlanChecker.from(connectContext)
+ .analyze("select id from uni")
+ .rewrite()
+ .getPlan();
+ Assertions.assertTrue(plan.getLogicalProperties().getFunctionalDependencies()
+ .isUniqueAndNotNull(plan.getOutput().get(0)));
+ }
+
+ @Test
+ void testFilter() {
+ Plan plan = PlanChecker.from(connectContext)
+ .analyze("select name from agg where name = \"1\"")
+ .rewrite()
+ .getPlan();
+ Assertions.assertTrue(plan.getLogicalProperties().getFunctionalDependencies()
+ .isUniformAndNotNull(plan.getOutput().get(0)));
+ }
+
+ @Test
+ void testJoin() {
+ Plan plan = PlanChecker.from(connectContext)
+ .analyze("select * from agg full outer join uni "
+ + "on agg.id = uni.id")
+ .rewrite()
+ .getPlan();
+ Assertions.assertTrue(plan.getLogicalProperties().getFunctionalDependencies().isEmpty());
+
+ plan = PlanChecker.from(connectContext)
+ .analyze("select agg.id, uni.id from agg full outer join uni "
+ + "on agg.id = uni.id")
+ .rewrite()
+ .getPlan();
+ Assertions.assertTrue(plan.getLogicalProperties().getFunctionalDependencies().isEmpty());
+
+ plan = PlanChecker.from(connectContext)
+ .analyze("select agg.id from agg left outer join uni "
+ + "on agg.id = uni.id")
+ .rewrite()
+ .getPlan();
+ System.out.println(plan.getLogicalProperties().getFunctionalDependencies());
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniqueAndNotNull(plan.getOutput().get(0)));
+
+ plan = PlanChecker.from(connectContext)
+ .analyze("select agg.id from agg left semi join uni "
+ + "on agg.id = uni.id")
+ .rewrite()
+ .getPlan();
+ System.out.println(plan.getLogicalProperties().getFunctionalDependencies());
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniqueAndNotNull(plan.getOutput().get(0)));
+
+ plan = PlanChecker.from(connectContext)
+ .analyze("select agg.id from agg left anti join uni "
+ + "on agg.id = uni.id")
+ .rewrite()
+ .getPlan();
+ System.out.println(plan.getLogicalProperties().getFunctionalDependencies());
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniqueAndNotNull(plan.getOutput().get(0)));
+
+ plan = PlanChecker.from(connectContext)
+ .analyze("select uni.id from agg right outer join uni "
+ + "on agg.id = uni.id")
+ .rewrite()
+ .getPlan();
+ System.out.println(plan.getLogicalProperties().getFunctionalDependencies());
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniqueAndNotNull(plan.getOutput().get(0)));
+
+ plan = PlanChecker.from(connectContext)
+ .analyze("select uni.id, agg.id from agg inner join uni "
+ + "on agg.id = uni.id")
+ .rewrite()
+ .getPlan();
+ System.out.println(plan.getLogicalProperties().getFunctionalDependencies());
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniqueAndNotNull(plan.getOutput().get(0)));
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniqueAndNotNull(plan.getOutput().get(1)));
+ }
+
+ @Test
+ void testAgg() {
+ Plan plan = PlanChecker.from(connectContext)
+ .analyze("select count(1), name from agg group by name")
+ .rewrite()
+ .getPlan();
+ System.out.println(plan.getLogicalProperties().getFunctionalDependencies());
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniqueAndNotNull(plan.getOutput().get(1)));
+
+ plan = PlanChecker.from(connectContext)
+ .analyze("select count(1), max(id), id from agg group by id")
+ .rewrite()
+ .getPlan();
+ System.out.println(plan.getLogicalProperties().getFunctionalDependencies());
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniqueAndNotNull(plan.getOutput().get(1)));
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniqueAndNotNull(plan.getOutput().get(2)));
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniformAndNotNull(plan.getOutput().get(0)));
+
+ plan = PlanChecker.from(connectContext)
+ .analyze("select count(1), id from agg where id = 1 group by id")
+ .rewrite()
+ .getPlan();
+ System.out.println(plan.getLogicalProperties().getFunctionalDependencies());
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniformAndNotNull(plan.getOutput().get(1)));
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniformAndNotNull(plan.getOutput().get(0)));
+
+ plan = PlanChecker.from(connectContext)
+ .analyze("select count(name) from agg")
+ .rewrite()
+ .getPlan();
+ System.out.println(plan.getLogicalProperties().getFunctionalDependencies());
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniformAndNotNull(plan.getOutput().get(0)));
+
+ plan = PlanChecker.from(connectContext)
+ .analyze("select id from agg where id = 1 group by cube(id, name)")
+ .rewrite()
+ .getPlan();
+ System.out.println(plan.getLogicalProperties().getFunctionalDependencies());
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniform(plan.getOutput().get(0)));
+ }
+
+ @Test
+ void testGenerate() {
+ Plan plan = PlanChecker.from(connectContext)
+ .analyze("select k1 from (select 1 k1) as t lateral view explode([1,2,3]) tmp1 as e1")
+ .rewrite()
+ .getPlan();
+ System.out.println(plan.getLogicalProperties().getFunctionalDependencies());
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniform(plan.getOutput().get(0)));
+ }
+
+ @Test
+ void testWindow() {
+ Plan plan = PlanChecker.from(connectContext)
+ .analyze("select rank() over(partition by id) from agg")
+ .rewrite()
+ .getPlan();
+ System.out.println(plan.getLogicalProperties().getFunctionalDependencies());
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniformAndNotNull(plan.getOutput().get(0)));
+
+ plan = PlanChecker.from(connectContext)
+ .analyze("select rank() over(partition by name) from agg where name = '1'")
+ .rewrite()
+ .getPlan();
+ System.out.println(plan.getLogicalProperties().getFunctionalDependencies());
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniqueAndNotNull(plan.getOutput().get(0)));
+
+ plan = PlanChecker.from(connectContext)
+ .analyze("select rank() over(partition by name) from agg where name = '1' limit 1")
+ .rewrite()
+ .getPlan();
+ LogicalPartitionTopN<?> ptopn = (LogicalPartitionTopN<?>) plan.child(0).child(0).child(0).child(0).child(0);
+ System.out.println(ptopn.getLogicalProperties().getFunctionalDependencies());
+ System.out.println(ptopn.getOutput());
+ Assertions.assertTrue(ptopn.getLogicalProperties()
+ .getFunctionalDependencies().isUniformAndNotNull(ImmutableSet.copyOf(ptopn.getOutputSet())));
+
+ plan = PlanChecker.from(connectContext)
+ .analyze("select rank() over(partition by name) from agg limit 1")
+ .rewrite()
+ .getPlan();
+ ptopn = (LogicalPartitionTopN<?>) plan.child(0).child(0).child(0).child(0).child(0);
+ Assertions.assertTrue(ptopn.getLogicalProperties()
+ .getFunctionalDependencies().isEmpty());
+ }
+
+ @Test
+ void testProject() {
+ Plan plan = PlanChecker.from(connectContext)
+ .analyze("select name from agg where id = 1")
+ .rewrite()
+ .getPlan();
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isEmpty());
+ }
+
+ @Test
+ void testLimitAndTopN() {
+ Plan plan = PlanChecker.from(connectContext)
+ .analyze("select name from agg limit 1")
+ .rewrite()
+ .getPlan();
+ System.out.println(plan.getLogicalProperties().getFunctionalDependencies());
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniformAndNotNull(plan.getOutput().get(0)));
+
+ plan = PlanChecker.from(connectContext)
+ .analyze("select name from agg order by id limit 1")
+ .rewrite()
+ .getPlan();
+ System.out.println(plan.getLogicalProperties().getFunctionalDependencies());
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniformAndNotNull(plan.getOutput().get(0)));
+ }
+
+ @Test
+ void testSubQuery() {
+ Plan plan = PlanChecker.from(connectContext)
+ .analyze("select id from (select * from agg) t")
+ .rewrite()
+ .getPlan();
+ System.out.println(plan.getLogicalProperties().getFunctionalDependencies());
+ Assertions.assertTrue(plan.getLogicalProperties()
+ .getFunctionalDependencies().isUniqueAndNotNull(plan.getOutput().get(0)));
+ }
+
+ @Test
+ void testCTE() {
+ Plan plan = PlanChecker.from(connectContext)
+ .analyze("with t as (select * from agg) select id from t")
Review Comment:
add cases for materialized cte
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1407386680
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalIntersect.java:
##########
@@ -98,4 +104,23 @@ public LogicalIntersect withNewOutputs(List<NamedExpression> newOutputs) {
return new LogicalIntersect(qualifier, newOutputs, regularChildrenOutputs,
Optional.empty(), Optional.empty(), children);
}
+
+ @Override
+ public FunctionalDependencies computeFuncDeps(Supplier<List<Slot>> outputSupplier) {
+ FunctionalDependencies.Builder builder = new FunctionalDependencies
+ .Builder(child(0).getLogicalProperties().getFunctionalDependencies());
Review Comment:
Yeah, you are right
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1829571629
(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 45.31 seconds
stream load tsv: 565 seconds loaded 74807831229 Bytes, about 126 MB/s
stream load json: 26 seconds loaded 2358488459 Bytes, about 86 MB/s
stream load orc: 72 seconds loaded 1101869774 Bytes, about 14 MB/s
stream load parquet: 34 seconds loaded 861443392 Bytes, about 24 MB/s
insert into select: 28.6 seconds inserted 10000000 Rows, about 349K ops/s
storage size: 17097957688 Bytes
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1812916029
run buildall
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1813761482
(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 44.75 seconds
stream load tsv: 563 seconds loaded 74807831229 Bytes, about 126 MB/s
stream load json: 18 seconds loaded 2358488459 Bytes, about 124 MB/s
stream load orc: 66 seconds loaded 1101869774 Bytes, about 15 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 27.9 seconds inserted 10000000 Rows, about 358K ops/s
storage size: 17099128077 Bytes
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1402079299
##########
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:
Null safe equal will produce more rows when there are multiple nulls. Therefore rejecting it is better
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "xzj7019 (via GitHub)" <gi...@apache.org>.
xzj7019 commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1406087947
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalUnion.java:
##########
@@ -170,4 +174,14 @@ public LogicalUnion withHasPushedFilter() {
public LogicalUnion pruneOutputs(List<NamedExpression> prunedOutputs) {
return withNewOutputs(prunedOutputs);
}
+
Review Comment:
add case for adding replaceMap
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1831172060
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1823799866
run buildall
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1814647420
<details>
<summary>TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'</summary>
```
Tpch sf100 test result on commit 8a0a9bbd18cca3462130d4dac373311416430e2b, data reload: false
run tpch-sf100 query with default conf and session variables
q1 4962 4704 4665 4665
q2 354 172 165 165
q3 2052 1903 1884 1884
q4 1398 1268 1239 1239
q5 3974 3959 4065 3959
q6 253 131 132 131
q7 1443 874 888 874
q8 2765 2785 2782 2782
q9 9684 9607 9614 9607
q10 3464 3545 3513 3513
q11 376 251 245 245
q12 446 293 297 293
q13 4575 3807 3764 3764
q14 322 283 293 283
q15 587 526 525 525
q16 662 580 582 580
q17 1150 955 956 955
q18 7835 7322 7415 7322
q19 1672 1692 1681 1681
q20 529 297 319 297
q21 4432 3951 3949 3949
q22 478 377 376 376
Total cold run time: 53413 ms
Total hot run time: 49089 ms
run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1 4604 4553 4593 4553
q2 346 232 256 232
q3 4018 4036 4017 4017
q4 2735 2691 2699 2691
q5 9648 9678 9715 9678
q6 250 124 124 124
q7 2604 2299 2288 2288
q8 4471 4460 4451 4451
q9 13251 13086 13127 13086
q10 4087 4158 4175 4158
q11 774 629 686 629
q12 974 811 801 801
q13 4263 3545 3579 3545
q14 387 348 350 348
q15 584 523 526 523
q16 735 657 687 657
q17 3886 3873 3847 3847
q18 9482 9058 9055 9055
q19 1824 1813 1769 1769
q20 2387 2070 2044 2044
q21 8808 8465 8681 8465
q22 863 807 771 771
Total cold run time: 80981 ms
Total hot run time: 77732 ms
```
</details>
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1812964471
<details>
<summary>TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'</summary>
```
Tpch sf100 test result on commit 425bb645d2c0c7b7654851c80233be75a8b0eb62, data reload: false
run tpch-sf100 query with default conf and session variables
q1 4923 4658 4666 4658
q2 359 152 153 152
q3 2055 1901 1848 1848
q4 1398 1255 1240 1240
q5 3956 3952 3966 3952
q6 250 132 134 132
q7 1412 872 888 872
q8 2750 2783 2772 2772
q9 9819 9644 9543 9543
q10 3454 3515 3516 3515
q11 384 240 248 240
q12 438 289 291 289
q13 4572 3824 3826 3824
q14 317 294 286 286
q15 600 548 533 533
q16 662 576 601 576
q17 1133 959 931 931
q18 7923 7436 7373 7373
q19 1678 1674 1686 1674
q20 553 312 293 293
q21 4355 3963 3984 3963
q22 490 371 381 371
Total cold run time: 53481 ms
Total hot run time: 49037 ms
run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1 4582 4555 4572 4555
q2 345 223 251 223
q3 4010 3985 3979 3979
q4 2705 2702 2693 2693
q5 9735 9641 9689 9641
q6 248 123 125 123
q7 2611 2224 2276 2224
q8 4450 4464 4460 4460
q9 13216 13095 13146 13095
q10 4058 4176 4171 4171
q11 751 631 683 631
q12 984 808 802 802
q13 4303 3536 3539 3536
q14 388 343 340 340
q15 580 528 527 527
q16 742 679 671 671
q17 3851 3841 3833 3833
q18 9462 8943 8943 8943
q19 1807 1759 1780 1759
q20 2385 2062 2038 2038
q21 8823 8476 8265 8265
q22 854 775 814 775
Total cold run time: 80890 ms
Total hot run time: 77284 ms
```
</details>
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "xzj7019 (via GitHub)" <gi...@apache.org>.
xzj7019 commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1407380381
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalUnion.java:
##########
@@ -170,4 +174,14 @@ public LogicalUnion withHasPushedFilter() {
public LogicalUnion pruneOutputs(List<NamedExpression> prunedOutputs) {
return withNewOutputs(prunedOutputs);
}
+
Review Comment:
that's right.
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1407395359
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalWindow.java:
##########
@@ -224,4 +227,52 @@ public Optional<Plan> pushPartitionLimitThroughWindow(long partitionLimit, boole
return Optional.ofNullable(window);
}
+
+ private void updateFuncDepsByWindowExpr(NamedExpression namedExpression, FunctionalDependencies.Builder builder) {
+ if (namedExpression.children().size() != 1 || !(namedExpression.child(0) instanceof WindowExpression)) {
+ return;
+ }
+ WindowExpression windowExpr = (WindowExpression) namedExpression.child(0);
+ List<Expression> partitionKeys = windowExpr.getPartitionKeys();
+
+ // window func should return unique results for different rows
+ if (!(windowExpr.getFunction() instanceof RowNumber
+ || windowExpr.getFunction() instanceof Rank)) {
+ return;
+ }
+
+ // Now we only support slot type keys
+ if (!partitionKeys.stream().allMatch(Slot.class::isInstance)) {
+ return;
+ }
+ ImmutableSet<Slot> slotSet = partitionKeys.stream()
+ .map(s -> (Slot) s)
+ .collect(ImmutableSet.toImmutableSet());
+
+ // if partition by keys are unique, output is uniform
+ if (child(0).getLogicalProperties().getFunctionalDependencies().isUniqueAndNotNull(slotSet)) {
+ if (windowExpr.getFunction() instanceof RowNumber
+ || windowExpr.getFunction() instanceof Rank
+ || windowExpr.getFunction() instanceof DenseRank) {
Review Comment:
Yeah, I forgot to delete loc 240.
For 1, there may be a lot of functions, but I have not handled them systematically. May be we can do it later
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1829140390
run buildall
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1812917222
run buildall
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1812976791
(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 45.19 seconds
stream load tsv: 563 seconds loaded 74807831229 Bytes, about 126 MB/s
stream load json: 18 seconds loaded 2358488459 Bytes, about 124 MB/s
stream load orc: 65 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 28.7 seconds inserted 10000000 Rows, about 348K ops/s
storage size: 17097997833 Bytes
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1813767438
<details>
<summary>TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'</summary>
```
Tpch sf100 test result on commit 21d7d6c433535447c91207b475d6b600558c2ecd, data reload: false
run tpch-sf100 query with default conf and session variables
q1 4989 4718 4678 4678
q2 360 166 158 158
q3 2002 1931 1920 1920
q4 1401 1268 1218 1218
q5 3947 3946 4012 3946
q6 254 128 132 128
q7 1375 872 898 872
q8 2769 2789 2764 2764
q9 9742 9574 9539 9539
q10 3442 3534 3510 3510
q11 383 248 249 248
q12 442 294 297 294
q13 4601 3823 3778 3778
q14 321 282 304 282
q15 590 531 530 530
q16 666 578 584 578
q17 1134 948 927 927
q18 7767 7226 7449 7226
q19 1667 1675 1668 1668
q20 577 317 316 316
q21 4394 3971 4002 3971
q22 480 373 380 373
Total cold run time: 53303 ms
Total hot run time: 48924 ms
run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1 4587 4574 4563 4563
q2 349 227 270 227
q3 4022 3970 3999 3970
q4 2690 2685 2696 2685
q5 9602 9508 9578 9508
q6 242 127 126 126
q7 2588 2240 2274 2240
q8 4438 4415 4468 4415
q9 13239 13110 13042 13042
q10 4084 4172 4185 4172
q11 778 624 669 624
q12 987 827 825 825
q13 4304 3567 3540 3540
q14 385 364 351 351
q15 577 522 519 519
q16 738 686 685 685
q17 3878 3847 3872 3847
q18 9523 8886 9148 8886
q19 1769 1767 1764 1764
q20 2400 2098 2040 2040
q21 8817 8520 8611 8520
q22 880 798 825 798
Total cold run time: 80877 ms
Total hot run time: 77347 ms
```
</details>
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "englefly (via GitHub)" <gi...@apache.org>.
englefly commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1396725499
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/Command.java:
##########
@@ -36,7 +37,7 @@
/**
* All DDL and DML commands' super class.
*/
-public abstract class Command extends AbstractPlan implements LogicalPlan {
+public abstract class Command extends AbstractPlan implements LogicalPlan, BlockFD {
Review Comment:
It would be better to make LogicalPlan extends BlockFD
##########
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 {
+ @Override
+ default FunctionalDependencies computeFD(List<Slot> outputs) {
+ return new FunctionalDependencies();
Review Comment:
很多节点都是透传孩子的FD，所以透传孩子的FD是否可以作为default？这样大部分LogicalXXX class里都不用重复实现这个函数
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalWindow.java:
##########
@@ -224,4 +226,46 @@ public Optional<Plan> pushPartitionLimitThroughWindow(long partitionLimit, boole
return Optional.ofNullable(window);
}
+
+ private void updateByWindowExpr(NamedExpression namedExpression, FunctionalDependencies functionalDependencies) {
Review Comment:
“udpateFdByWindowExpr” may be a better name
##########
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:
what the meaning of "block" here ?
do you mean fd deriver
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalWindow.java:
##########
@@ -224,4 +226,46 @@ public Optional<Plan> pushPartitionLimitThroughWindow(long partitionLimit, boole
return Optional.ofNullable(window);
}
+
+ private void updateByWindowExpr(NamedExpression namedExpression, FunctionalDependencies functionalDependencies) {
Review Comment:
“udpateFdByWindowExpr” may be a better name
##########
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 {
+ @Override
+ default FunctionalDependencies computeFD(List<Slot> outputs) {
+ return new FunctionalDependencies();
Review Comment:
很多节点都是透传孩子的FD，所以透传孩子的FD是否可以作为default？这样大部分LogicalXXX class里都不用重复实现这个函数
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1814429643
run buildall
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1831172061
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1407078684
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalJoin.java:
##########
@@ -357,6 +364,91 @@ 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 EqualPredicate
+ && ((EqualTo) expression).left() instanceof Slot
+ && ((EqualTo) expression).right() instanceof Slot)) {
+ return null;
+ }
+ Slot leftKey = (Slot) ((EqualTo) expression).left();
+ Slot rightKey = (Slot) ((EqualTo) expression).right();
+ if (left().getOutputSet().contains(leftKey)) {
+ leftKeys.add(leftKey);
+ rightKeys.add(rightKey);
+ } else {
+ leftKeys.add(rightKey);
+ rightKeys.add(leftKey);
+ }
+ }
+ return Pair.of(leftKeys, rightKeys);
+ }
+
+ @Override
+ public FunctionalDependencies computeFuncDeps(Supplier<List<Slot>> outputSupplier) {
+ //1. NALAJ and FOJ block functional dependencies
+ if (joinType.isNullAwareLeftAntiJoin() || joinType.isFullOuterJoin()) {
+ return FunctionalDependencies.EMPTY_FUNC_DEPS;
+ }
+
+ // left/right semi/anti join propagate left/right functional dependencies
+ if (joinType.isLeftAntiJoin() || joinType.isLefSemiJoin()) {
+ return left().getLogicalProperties().getFunctionalDependencies();
+ }
+ if (joinType.isRightSemiJoin() || joinType.isRightAntiJoin()) {
+ return right().getLogicalProperties().getFunctionalDependencies();
+ }
+
+ // if there is non-equal join conditions, block functional dependencies
+ if (!otherJoinConjuncts.isEmpty()) {
+ return FunctionalDependencies.EMPTY_FUNC_DEPS;
+ }
+
+ Pair<Set<Slot>, Set<Slot>> keys = extractHashKeys();
+ if (keys == null) {
+ return FunctionalDependencies.EMPTY_FUNC_DEPS;
+ }
+
+ // Note here we only check whether the left is unique.
+ // So the hash condition can be null-safe
+ // TODO: consider rejecting Null-safe hash condition when left and rigth can be null
+ boolean isLeftUnique = left().getLogicalProperties()
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1822376497
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1401933994
##########
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:
Yes, I forgot about that.
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1402071298
##########
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:
yeah
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1822909931
run buildall
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1812920188
run buildall
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1823817303
<details>
<summary>TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'</summary>
```
Tpch sf100 test result on commit b9ab432b6cee70852d5e73e8ddd636965b11e5ba, data reload: false
run tpch-sf100 query with default conf and session variables
q1 4966 4679 4679 4679
q2 356 161 158 158
q3 2041 1903 1950 1903
q4 1383 1272 1262 1262
q5 4006 3985 4055 3985
q6 251 129 130 129
q7 1429 898 884 884
q8 2797 2803 2788 2788
q9 9746 9501 9473 9473
q10 3469 3532 3511 3511
q11 379 239 247 239
q12 441 286 295 286
q13 4560 3850 3797 3797
q14 336 291 287 287
q15 591 536 527 527
q16 675 591 581 581
q17 1141 963 961 961
q18 7911 7364 7442 7364
q19 1661 1683 1681 1681
q20 575 304 300 300
q21 4457 3981 4020 3981
q22 474 378 368 368
Total cold run time: 53645 ms
Total hot run time: 49144 ms
run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1 4597 4622 4604 4604
q2 345 217 264 217
q3 4015 4005 3994 3994
q4 2717 2688 2707 2688
q5 9571 9562 9607 9562
q6 240 119 127 119
q7 3028 2454 2506 2454
q8 4417 4495 4444 4444
q9 12988 12788 12833 12788
q10 4086 4147 4195 4147
q11 774 631 672 631
q12 986 786 816 786
q13 4335 3568 3568 3568
q14 387 348 342 342
q15 574 521 520 520
q16 741 662 660 660
q17 3818 3844 3818 3818
q18 9721 9045 9196 9045
q19 1818 1783 1767 1767
q20 2389 2073 2039 2039
q21 8799 8729 8638 8638
q22 892 797 805 797
Total cold run time: 81238 ms
Total hot run time: 77628 ms
```
</details>
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1407085060
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalUnion.java:
##########
@@ -170,4 +174,14 @@ public LogicalUnion withHasPushedFilter() {
public LogicalUnion pruneOutputs(List<NamedExpression> prunedOutputs) {
return withNewOutputs(prunedOutputs);
}
+
Review Comment:
union doesn't propagate child's fd. It's ok
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1829600069
<details>
<summary>TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'</summary>
```
Tpch sf100 test result on commit 45fb40637f131826e19404955bcf8d29d12467fd, data reload: false
run tpch-sf100 query with default conf and session variables
q1 4935 4634 4657 4634
q2 361 153 158 153
q3 2021 1916 1851 1851
q4 1391 1283 1240 1240
q5 3929 3939 4009 3939
q6 265 134 131 131
q7 1324 862 873 862
q8 2803 2799 2774 2774
q9 9789 9561 9455 9455
q10 3466 3496 3527 3496
q11 375 252 255 252
q12 441 297 293 293
q13 4599 3841 3814 3814
q14 325 290 295 290
q15 579 539 523 523
q16 494 462 451 451
q17 1143 982 936 936
q18 7950 7508 7511 7508
q19 1680 1694 1681 1681
q20 544 331 312 312
q21 4470 4078 4067 4067
q22 471 375 383 375
Total cold run time: 53355 ms
Total hot run time: 49037 ms
run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1 4564 4563 4561 4561
q2 341 226 235 226
q3 4071 4018 4018 4018
q4 2734 2713 2717 2713
q5 9624 9624 9559 9559
q6 250 125 122 122
q7 3018 2535 2527 2527
q8 4444 4451 4394 4394
q9 12966 12842 12859 12842
q10 4075 4146 4156 4146
q11 784 676 683 676
q12 986 817 798 798
q13 4307 3579 3565 3565
q14 385 360 344 344
q15 593 524 517 517
q16 596 558 574 558
q17 3873 3811 3937 3811
q18 9598 9141 9160 9141
q19 1818 1830 1794 1794
q20 2381 2101 2072 2072
q21 8917 8629 8391 8391
q22 918 786 812 786
Total cold run time: 81243 ms
Total hot run time: 77561 ms
```
</details>
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "xzj7019 (via GitHub)" <gi...@apache.org>.
xzj7019 commented on code in PR #27051:
URL: https://github.com/apache/doris/pull/27051#discussion_r1406073699
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalJoin.java:
##########
@@ -357,6 +364,91 @@ 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 EqualPredicate
+ && ((EqualTo) expression).left() instanceof Slot
+ && ((EqualTo) expression).right() instanceof Slot)) {
+ return null;
+ }
+ Slot leftKey = (Slot) ((EqualTo) expression).left();
+ Slot rightKey = (Slot) ((EqualTo) expression).right();
+ if (left().getOutputSet().contains(leftKey)) {
+ leftKeys.add(leftKey);
+ rightKeys.add(rightKey);
+ } else {
+ leftKeys.add(rightKey);
+ rightKeys.add(leftKey);
+ }
+ }
+ return Pair.of(leftKeys, rightKeys);
+ }
+
+ @Override
+ public FunctionalDependencies computeFuncDeps(Supplier<List<Slot>> outputSupplier) {
+ //1. NALAJ and FOJ block functional dependencies
+ if (joinType.isNullAwareLeftAntiJoin() || joinType.isFullOuterJoin()) {
+ return FunctionalDependencies.EMPTY_FUNC_DEPS;
+ }
+
+ // left/right semi/anti join propagate left/right functional dependencies
+ if (joinType.isLeftAntiJoin() || joinType.isLefSemiJoin()) {
+ return left().getLogicalProperties().getFunctionalDependencies();
+ }
+ if (joinType.isRightSemiJoin() || joinType.isRightAntiJoin()) {
+ return right().getLogicalProperties().getFunctionalDependencies();
+ }
+
+ // if there is non-equal join conditions, block functional dependencies
+ if (!otherJoinConjuncts.isEmpty()) {
+ return FunctionalDependencies.EMPTY_FUNC_DEPS;
+ }
+
+ Pair<Set<Slot>, Set<Slot>> keys = extractHashKeys();
+ if (keys == null) {
+ return FunctionalDependencies.EMPTY_FUNC_DEPS;
+ }
+
+ // Note here we only check whether the left is unique.
+ // So the hash condition can be null-safe
+ // TODO: consider rejecting Null-safe hash condition when left and rigth can be null
+ boolean isLeftUnique = left().getLogicalProperties()
Review Comment:
for non-null-safe equal join, remove the not null restriction.
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "XieJiann (via GitHub)" <gi...@apache.org>.
XieJiann commented on PR #27051:
URL: https://github.com/apache/doris/pull/27051#issuecomment-1829359342
run buildall
--
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
Re: [PR] [feature](Nereids) add functional dependencies [doris]
Posted by "jackwener (via GitHub)" <gi...@apache.org>.
jackwener merged PR #27051:
URL: https://github.com/apache/doris/pull/27051
--
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