[CALCITE-5503] CheapestPlanReplacer should reuse repeated nodes in a DAG plan

• Type: Improvement
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Fix Version/s: 1.34.0
• Component/s: core
• Labels: pull-request-available
• Assignee: Moritz Mack
• Reporter: Moritz Mack
• Created: 27/Jan/23 17:55
• Updated: 29/Mar/23 10:58
• Resolved: 18/Feb/23 10:47

Description

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.

 

@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));
} 

 

Issue Links

• links to: GitHub Pull Request #3050