You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/06/17 15:07:41 UTC
[GitHub] [doris] Kikyou1997 opened a new pull request, #10240: [Refactor](nereids) Abstract interface of statistics framework for new optimizer reuse
Kikyou1997 opened a new pull request, #10240:
URL: https://github.com/apache/doris/pull/10240
# Proposed changes
As the statistics framework could not reuse by new optmizer before, so I abstract some interface to make it reusable.
1. Make Slot extends the Id
2. Add new interface:ExprStats,PlanStats
3. Move definition of PlanNode.NodeType to statistics sub-directory
## Checklist(Required)
1. Does it affect the original behavior: (No)
5. Has unit tests been added: (No Need)
6. Has document been added or modified: (No)
7. Does it need to update dependencies: (No)
8. Are there any changes that cannot be rolled back: (No)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morrySnow commented on a diff in pull request #10240: [Refactor](nereids) Abstract interface of statistics framework for new optimizer reuse
Posted by GitBox <gi...@apache.org>.
morrySnow commented on code in PR #10240:
URL: https://github.com/apache/doris/pull/10240#discussion_r901213903
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/PlanStats.java:
##########
@@ -0,0 +1,39 @@
+// 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.statistics;
+
+import java.util.List;
+
+/**
+ * Used to abstract a common operator interface for statistics deduction to fit both optimizers.
+ */
+public interface PlanStats {
+
+ List<? extends PlanStats> getChildrenStats();
Review Comment:
this should return children's StatsDeriveResult
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] 924060929 commented on a diff in pull request #10240: [Refactor](nereids) Abstract interface of statistics framework for new optimizer reuse
Posted by GitBox <gi...@apache.org>.
924060929 commented on code in PR #10240:
URL: https://github.com/apache/doris/pull/10240#discussion_r900267355
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/PlaceHolderPlan.java:
##########
@@ -81,4 +85,34 @@ public PlaceHolderPlan withChildren(List<Plan> children) {
Preconditions.checkArgument(children.size() == 0);
return new PlaceHolderPlan(logicalProperties);
}
+
+ @Override
+ public List<? extends PlanStats> getChildrenStats() {
+ return null;
+ }
+
+ @Override
+ public StatsDeriveResult getStatsDeriveResult() {
+ return null;
+ }
+
+ @Override
+ public StatisticalType getNodeType() {
Review Comment:
```suggestion
public StatisticalType getStatisticalType() {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morrySnow commented on a diff in pull request #10240: [Refactor](nereids) Abstract interface of statistics framework for new optimizer reuse
Posted by GitBox <gi...@apache.org>.
morrySnow commented on code in PR #10240:
URL: https://github.com/apache/doris/pull/10240#discussion_r900261396
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/PlaceHolderPlan.java:
##########
@@ -81,4 +85,34 @@ public PlaceHolderPlan withChildren(List<Plan> children) {
Preconditions.checkArgument(children.size() == 0);
return new PlaceHolderPlan(logicalProperties);
}
+
+ @Override
+ public List<? extends PlanStats> getChildrenStats() {
+ return null;
Review Comment:
I haven't thought about it very carefully, but IMO, throw exception in these interface in PlaceHolderPlan may be better
##########
fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java:
##########
@@ -353,6 +327,11 @@ public List<Expr> getConjuncts() {
return conjuncts;
}
+ @Override
+ public List<? extends PlanStats> getChildrenStats() {
+ return children;
Review Comment:
why return children here? i think this interface should be used to return children's StatsDeriveResult
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/ExprId.java:
##########
@@ -17,13 +17,15 @@
package org.apache.doris.nereids.trees.expressions;
+import org.apache.doris.common.Id;
+
import java.util.Objects;
import java.util.UUID;
/**
* UUID for Expression in Nereids.
*/
-public class ExprId {
+public class ExprId extends Id<ExprId> {
private final long id;
private final UUID jvmId;
Review Comment:
remove these lines instead with parent class's attribute: id
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/StatsRecursiveDerive.java:
##########
@@ -38,11 +36,11 @@ private static class Inner {
* This parameter is an input and output parameter,
* which will store the derivation result of statistical information in the corresponding node
*/
- public void statsRecursiveDerive(PlanNode node) throws UserException {
Review Comment:
statsRecursiveDerive's parameter should not change to PlanStats, since Nereids will never use this to do stats Derive
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #10240: [Refactor](nereids) Abstract interface of statistics framework for new optimizer reuse
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #10240:
URL: https://github.com/apache/doris/pull/10240#discussion_r900735365
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/ExprId.java:
##########
@@ -17,13 +17,15 @@
package org.apache.doris.nereids.trees.expressions;
+import org.apache.doris.common.Id;
+
import java.util.Objects;
import java.util.UUID;
/**
* UUID for Expression in Nereids.
*/
-public class ExprId {
+public class ExprId extends Id<ExprId> {
private final long id;
private final UUID jvmId;
Review Comment:
And removed the jvmId by the way
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/PlaceHolderPlan.java:
##########
@@ -81,4 +85,34 @@ public PlaceHolderPlan withChildren(List<Plan> children) {
Preconditions.checkArgument(children.size() == 0);
return new PlaceHolderPlan(logicalProperties);
}
+
+ @Override
+ public List<? extends PlanStats> getChildrenStats() {
+ return null;
+ }
+
+ @Override
+ public StatsDeriveResult getStatsDeriveResult() {
+ return null;
+ }
+
+ @Override
+ public StatisticalType getNodeType() {
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #10240: [Refactor](nereids) Abstract interface of statistics framework for new optimizer reuse
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10240:
URL: https://github.com/apache/doris/pull/10240#issuecomment-1159998289
PR approved by anyone and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #10240: [Refactor](nereids) Abstract interface of statistics framework for new optimizer reuse
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #10240:
URL: https://github.com/apache/doris/pull/10240#discussion_r900735138
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/ExprId.java:
##########
@@ -17,13 +17,15 @@
package org.apache.doris.nereids.trees.expressions;
+import org.apache.doris.common.Id;
+
import java.util.Objects;
import java.util.UUID;
/**
* UUID for Expression in Nereids.
*/
-public class ExprId {
+public class ExprId extends Id<ExprId> {
private final long id;
private final UUID jvmId;
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #10240: [Refactor](nereids) Abstract interface of statistics framework for new optimizer reuse
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #10240:
URL: https://github.com/apache/doris/pull/10240#discussion_r900717225
##########
fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java:
##########
@@ -353,6 +327,11 @@ public List<Expr> getConjuncts() {
return conjuncts;
}
+ @Override
+ public List<? extends PlanStats> getChildrenStats() {
+ return children;
Review Comment:
It chould get StatsDeriveResult by `PlanStats::getStatsDeriveResult`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #10240: [Refactor](nereids) Abstract interface of statistics framework for new optimizer reuse
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #10240:
URL: https://github.com/apache/doris/pull/10240#discussion_r901222467
##########
fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java:
##########
@@ -353,6 +327,11 @@ public List<Expr> getConjuncts() {
return conjuncts;
}
+ @Override
+ public List<? extends PlanStats> getChildrenStats() {
+ return children;
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman merged pull request #10240: [Refactor](nereids) Abstract interface of statistics framework for new optimizer reuse
Posted by GitBox <gi...@apache.org>.
morningman merged PR #10240:
URL: https://github.com/apache/doris/pull/10240
--
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