morrySnow commented on code in PR #13046:
URL: https://github.com/apache/doris/pull/13046#discussion_r1011467239
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/memo/GroupExpression.java:
##########
@@ -109,6 +114,14 @@ public void setChildren(ImmutableList<Group> children) {
this.children = children;
}
+ public boolean isHasCalculateCost() {
+ return hasCalculateCost;
+ }
+
+ public void setHasCalculateCost(boolean hasCalculateCost) {
+ this.hasCalculateCost = hasCalculateCost;
+ }
+
Review Comment:
remove it in this PR
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/cascades/CostAndEnforcerJob.java:
##########
@@ -164,74 +167,96 @@ public void execute() {
if (curTotalCost > context.getCostUpperBound()) {
break;
}
+ // the request child properties will be covered by the output
properties
+ // that corresponding to the request properties. so if we run
a costAndEnforceJob of the same
+ // group expression, that request child properties will be
different of this.
}
// This mean that we successfully optimize all child groups.
+ // if break when running the loop above, the condition must be
false.
if (curChildIndex == groupExpression.arity()) {
-
- // to ensure distributionSpec has been added sufficiently.
- ChildrenPropertiesRegulator regulator = new
ChildrenPropertiesRegulator(groupExpression,
- lowestCostChildren, requestChildrenProperties,
requestChildrenProperties, context);
- double enforceCost = regulator.adjustChildrenProperties();
- if (enforceCost < 0) {
- // invalid enforce, return.
- return;
- }
- curTotalCost += enforceCost;
-
- // Not need to do pruning here because it has been done when
we get the
- // best expr from the child group
- ChildOutputPropertyDeriver childOutputPropertyDeriver
- = new
ChildOutputPropertyDeriver(requestChildrenProperties);
- PhysicalProperties outputProperty =
childOutputPropertyDeriver.getOutputProperties(groupExpression);
-
- // update current group statistics and re-compute costs.
- if (groupExpression.children().stream().anyMatch(group ->
group.getStatistics() == null)) {
- // if we come here, mean that we have some error in stats
calculator and should fix it.
+ if (!calculateEnforce(requestChildrenProperties)) {
return;
}
- StatsCalculator.estimate(groupExpression);
- curTotalCost -= curNodeCost;
- curNodeCost = CostCalculator.calculateCost(groupExpression);
- groupExpression.setCost(curNodeCost);
- curTotalCost += curNodeCost;
-
- // record map { outputProperty -> outputProperty }, { ANY ->
outputProperty },
- recordPropertyAndCost(groupExpression, outputProperty,
PhysicalProperties.ANY,
- requestChildrenProperties);
- recordPropertyAndCost(groupExpression, outputProperty,
outputProperty, requestChildrenProperties);
- enforce(outputProperty, requestChildrenProperties);
-
if (curTotalCost < context.getCostUpperBound()) {
context.setCostUpperBound(curTotalCost);
}
}
-
clear();
}
}
+ private boolean calculateEnforce(List<PhysicalProperties>
requestChildrenProperties) {
Review Comment:
add some comment to explain return value
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/memo/GroupExpression.java:
##########
@@ -168,25 +182,24 @@ public List<PhysicalProperties>
getInputPropertiesList(PhysicalProperties requir
/**
* Add a (outputProperties) -> (cost, childrenInputProperties) in
lowestCostTable.
+ * if the outputProperties exists, will be covered.
+ * @return true if lowest cost table change.
*/
public boolean updateLowestCostTable(PhysicalProperties outputProperties,
List<PhysicalProperties> childrenInputProperties, double cost) {
if (lowestCostTable.containsKey(outputProperties)) {
if (lowestCostTable.get(outputProperties).first > cost) {
lowestCostTable.put(outputProperties, Pair.of(cost,
childrenInputProperties));
return true;
- } else {
- return false;
}
- } else {
- lowestCostTable.put(outputProperties, Pair.of(cost,
childrenInputProperties));
- return true;
+ return false;
Review Comment:
do not remove else block. it is easy to do a accidentally modifying with out
it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
