silundong commented on code in PR #4392:
URL: https://github.com/apache/calcite/pull/4392#discussion_r2111463951
##########
core/src/main/java/org/apache/calcite/rel/rules/DpHyp.java:
##########
@@ -173,42 +180,86 @@ private void enumerateCmpRec(long csg, long cmp, long
forbidden) {
private void emitCsgCmp(long csg, long cmp, List<HyperEdge> edges) {
RelNode child1 = dpTable.get(csg);
RelNode child2 = dpTable.get(cmp);
+ ImmutableList csgOrder = resultInputOrder.get(csg);
+ ImmutableList cmpOrder = resultInputOrder.get(cmp);
if (child1 == null || child2 == null) {
- throw new IllegalArgumentException(
+ throw new DphypOrHyperGraphException(
"csg and cmp were not enumerated in the previous dp process");
}
+ if (csgOrder == null || cmpOrder == null) {
+ throw new DphypOrHyperGraphException("Lost the vertex order of csg or
cmp");
+ }
JoinRelType joinType = hyperGraph.extractJoinType(edges);
if (joinType == null) {
return;
}
- RexNode joinCond1 = hyperGraph.extractJoinCond(child1, child2, edges);
+ if (!ConflictDetectionHelper.applicable(csg | cmp, edges)) {
+ return;
+ }
+
+ ImmutableList<Integer> unionOrder = ImmutableList.<Integer>builder()
+ .addAll(csgOrder)
+ .addAll(cmpOrder)
+ .build();
+ RexNode joinCond1 = hyperGraph.extractJoinCond(unionOrder,
csgOrder.size(), edges);
RelNode newPlan1 = builder
.push(child1)
.push(child2)
.join(joinType, joinCond1)
.build();
+ RelNode winPlan = newPlan1;
+ ImmutableList<Integer> winOrder = ImmutableList.copyOf(unionOrder);
- // swap left and right
- RexNode joinCond2 = hyperGraph.extractJoinCond(child2, child1, edges);
- RelNode newPlan2 = builder
- .push(child2)
- .push(child1)
- .join(joinType, joinCond2)
- .build();
- RelNode winPlan = chooseBetterPlan(newPlan1, newPlan2);
+ if (ConflictDetectionHelper.isCommutative(joinType)) {
+ // swap left and right
+ unionOrder = ImmutableList.<Integer>builder()
+ .addAll(cmpOrder)
+ .addAll(csgOrder)
+ .build();
+ RexNode joinCond2 = hyperGraph.extractJoinCond(unionOrder,
cmpOrder.size(), edges);
+ RelNode newPlan2 = builder
+ .push(child2)
+ .push(child1)
+ .join(joinType, joinCond2)
+ .build();
+ winPlan = chooseBetterPlan(winPlan, newPlan2);
+ if (winPlan.equals(newPlan2)) {
+ winOrder = ImmutableList.copyOf(unionOrder);
+ }
+ }
RelNode oriPlan = dpTable.get(csg | cmp);
if (oriPlan != null) {
winPlan = chooseBetterPlan(winPlan, oriPlan);
+ if (winPlan.equals(oriPlan)) {
+ winOrder = resultInputOrder.get(csg | cmp);
+ }
}
+ assert winOrder != null;
dpTable.put(csg | cmp, winPlan);
+ resultInputOrder.put(csg | cmp, winOrder);
}
public @Nullable RelNode getBestPlan() {
int size = hyperGraph.getInputs().size();
long wholeGraph = LongBitmap.newBitmapBetween(0, size);
- return dpTable.get(wholeGraph);
+ RelNode orderedJoin = dpTable.get(wholeGraph);
+ if (orderedJoin == null) {
+ return null;
+ }
+ ImmutableList<Integer> resultOrder = resultInputOrder.get(wholeGraph);
+ if (resultOrder == null) {
Review Comment:
You are right. I have thrown exceptions in several places in the DpHyp
related code, but they should not happen in a legal program. I think I should
change them all to assertion failures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
