silundong commented on code in PR #4392:
URL: https://github.com/apache/calcite/pull/4392#discussion_r2155942342
##########
core/src/main/java/org/apache/calcite/rel/rules/DpHyp.java:
##########
@@ -173,42 +202,140 @@ 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);
- if (child1 == null || child2 == null) {
- throw new IllegalArgumentException(
- "csg and cmp were not enumerated in the previous dp process");
- }
+ ImmutableList<HyperGraph.NodeState> csgOrder = resultInputOrder.get(csg);
+ ImmutableList<HyperGraph.NodeState> cmpOrder = resultInputOrder.get(cmp);
+ assert child1 != null && child2 != null && csgOrder != null && cmpOrder !=
null;
+ assert Long.bitCount(csg) == csgOrder.size() && Long.bitCount(cmp) ==
cmpOrder.size();
JoinRelType joinType = hyperGraph.extractJoinType(edges);
if (joinType == null) {
return;
}
- RexNode joinCond1 = hyperGraph.extractJoinCond(child1, child2, edges);
+ // verify whether the subgraph is legal by using the conflict rules in
hyperedges
+ if (!hyperGraph.applicable(csg | cmp, edges)) {
+ return;
+ }
+
+ List<HyperGraph.NodeState> unionOrder = new ArrayList<>(csgOrder);
+ unionOrder.addAll(cmpOrder);
+ // build join condition from hyperedges. e.g.
+ // case.1
+ // csg: node0_projected [field0, field1], node1_projected [field0, field1],
+ //
+ // join
+ // / \
+ // node0 node1
+ //
+ // cmp: node2_projected [field0, field1]
+ // hyperedge1: node0.field0 = node2.field0
+ // hyperedge2: node1.field1 = node2.field1
+ // we will get join condition: ($0 = $4) and ($3 = $5)
+ //
+ // new_join(condition=[AND(($0 = $4), ($3 = $5))])
+ // / \
+ // join node2
+ // / \
+ // node0 node1
+ //
+ // case.2
+ // csg: node0_projected [field0, field1], node1_not_projected [field0,
field1],
+ //
+ // join(joinType=semi/anti)
+ // / \
+ // node0 node1
+ //
+ // cmp: node2_projected [field0, field1]
+ // hyperedge1: node0.field0 = node2.field0
+ // hyperedge2: node0.field1 = node2.field1
+ // we will get join condition: ($0 = $2) and ($1 = $3)
+ //
+ // new_join(condition=[AND(($0 = $2), ($1 = $3))])
+ // / \
+ // join(joinType=semi/anti) node2
+ // / \
+ // node0 node1
+ RexNode joinCond1 = hyperGraph.extractJoinCond(unionOrder,
csgOrder.size(), edges, joinType);
RelNode newPlan1 = builder
.push(child1)
.push(child2)
.join(joinType, joinCond1)
.build();
+ RelNode winPlan = newPlan1;
+ ImmutableList<HyperGraph.NodeState> winOrder =
ImmutableList.copyOf(unionOrder);
+ assert verifyDpResultRowType(newPlan1, unionOrder);
Review Comment:
> I had suggested a test that validates all candidate hypergraphs, even the
non-optimal ones, to improve the coverage of the code. I didn't see one. Is it
possible to add one?
I added asserts during the enumeration process to verify the rowType of each
candidate result. Is this OK? @mihaibudiu
--
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]