[jira] [Commented] (CALCITE-5503) CheapestPlanReplacer should reuse repeated nodes in a DAG plan

["Evgeny Stanilovsky (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CALCITE-5503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17705852#comment-17705852 ] 

Evgeny Stanilovsky commented on CALCITE-5503:
---------------------------------------------

[~mosche] [~libenchao] thanks for this issue, but i misunderstand here [1], why you put OLD id into map, instead new one ? After 
p = p.copy(p.getTraitSet(), inputs);
*RelNode id* will change but you still return old one with differ inputs

[1] https://github.com/apache/calcite/pull/3050/files#diff-23f79975c4d8d77b7a09299831fe47c69c4f65fe8639bc4ceb956321a33da634R749

> CheapestPlanReplacer should reuse repeated nodes in a DAG plan
> --------------------------------------------------------------
>
>                 Key: CALCITE-5503
>                 URL: https://issues.apache.org/jira/browse/CALCITE-5503
>             Project: Calcite
>          Issue Type: Improvement
>          Components: core
>            Reporter: Moritz Mack
>            Assignee: Moritz Mack
>            Priority: Minor
>              Labels: pull-request-available
>             Fix For: 1.34.0
>
>          Time Spent: 1h 10m
>  Remaining Estimate: 0h
>
> When using CheapestPlanReplacer, semantics of a RelNode tree change if it contains the same node multiple times in case this node has inputs itself that have to be replaced. During replacement such nodes get copied on each occurrence.
> Instead CheapestPlanReplacer should memoize previously visited nodes and emit the same result again in the that case.
> The test case below illustrates the issue and fails on the current main branch.
>  
> {code:java}
> @Test void testMemoizeInputRelNodes() {
>   VolcanoPlanner planner = new VolcanoPlanner();
>   planner.addRelTraitDef(ConventionTraitDef.INSTANCE);
>   RelOptCluster cluster = newCluster(planner);
>   // The rule that triggers the assert rule
>   planner.addRule(PhysLeafRule.INSTANCE);
>   planner.addRule(GoodSingleRule.INSTANCE);
>   // Leaf RelNode
>   NoneLeafRel leafRel = new NoneLeafRel(cluster, "a");
>   RelNode leafPhy = planner
>       .changeTraits(leafRel, cluster.traitSetOf(PHYS_CALLING_CONVENTION));
>   // RelNode with leaf RelNode as single input
>   NoneSingleRel singleRel = new NoneSingleRel(cluster, leafPhy);
>   RelNode singlePhy = planner
>       .changeTraits(singleRel, cluster.traitSetOf(PHYS_CALLING_CONVENTION));
>   // Binary RelNode with identical input on either side
>   PhysBiRel parent = new PhysBiRel(
>       cluster, cluster.traitSetOf(PHYS_CALLING_CONVENTION), singlePhy, singlePhy);
>   planner.setRoot(parent);
>   RelNode result = planner.chooseDelegate().findBestExp();
>   // Expect inputs to remain identical
>   assertEquals(result.getInput(0), result.getInput(1));
> } {code}
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)