juntaozhang commented on code in PR #4436:
URL: https://github.com/apache/calcite/pull/4436#discussion_r2174711387
##########
core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java:
##########
@@ -843,6 +843,39 @@ public RelSubset getSubsetNonNull(RelNode rel) {
prunedNodes.add(rel);
}
+ /**
+ * Replaces all references to the given old subset with the new subset
+ * in the parent nodes' inputs. This is typically used when a rule
+ * rewrites a subquery or merges subsets, such as replacing a filter
+ * with a join in {@link org.apache.calcite.rel.rules.SubQueryRemoveRule}.
+ *
+ * @param oldSubset The subset to be replaced
+ * @param newSubset The subset to replace with
+ */
+ public void replaceSubset(RelSubset oldSubset, RelSubset newSubset) {
+ if (oldSubset == newSubset) {
+ // Nop.
+ return;
+ }
+
+ for (RelNode parent : oldSubset.getParents()) {
+ List<RelNode> inputs = parent.getInputs();
+ boolean updated = false;
+ for (int i = 0; i < inputs.size(); i++) {
+ RelNode child = inputs.get(i);
+ if (child != oldSubset) {
+ continue;
+ }
+ parent.replaceInput(i, newSubset);
Review Comment:
Hi @silundong @xiedeyantu, really appreciate your suggestions.
As I outlined in the Jira ticket, the root cause of this bug is as follows:
>In the error case (case2), after the rewrite, the new subtree (rel#38) has
a different distribution trait (any) compared to the original input (rel#15,
which is broadcast). If the parent node (LogicalProject#16) still points to the
old input (rel#15), the planner cannot find a valid conversion path to the
required trait (e.g., ENUMERABLE.broadcast).
It's precisely because different distribution traits lead to the creation of
a new subset that we've noticed VolcanoPlanner fails to replace the parent's
input, which lead a invalid node path.
I believe that if we explicitly replace the original node in the
Rule(implements `SubstitutionRule` and set `autoReplaceOld` to true), this
indicates sufficient confidence in performing the replacement.
Not sure if I've made this clear, will happy have deepdive in this issue.
Thanks again!
--
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]
