Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-6554

nested correlated sub-query in aggregation does not have inner correlation variable bound to inner projection

    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 1.37.0
    • 1.38.0
    • None

    Description

      It appears that nested correlated subqueries can (at least in aggregates) result in project nodes not having their variablesSet populated with the correct (or any) correlation variables.

      For example:

      @Test void testCorrelationInProjectionWith1xNestedCorrelatedProjection() {
        final String sql =
              "select e1.empno,\n"
            + "  (select sum(e2.sal +\n"
            + "    (select sum(e3.sal) from emp e3 where e3.mgr = e2.empno)\n"
            + "   ) from emp e2 where e2.mgr = e1.empno)\n"
            + "from emp e1";
        sql(sql).withExpand(false).withDecorrelate(false).ok();
      }
      

      currently gives the following plan:

      LogicalProject(variablesSet=[[$cor0]], EMPNO=[$0], EXPR$1=[$SCALAR_QUERY({
      LogicalAggregate(group=[{}], EXPR$0=[SUM($0)])
        LogicalProject($f0=[+($5, $SCALAR_QUERY({
      LogicalAggregate(group=[{}], EXPR$0=[SUM($0)])
        LogicalProject(SAL=[$5])
          LogicalFilter(condition=[=($3, $cor1.EMPNO)])
            LogicalTableScan(table=[[CATALOG, SALES, EMP]])
      }))])
          LogicalFilter(condition=[=($3, $cor0.EMPNO)])
            LogicalTableScan(table=[[CATALOG, SALES, EMP]])
      })])
        LogicalTableScan(table=[[CATALOG, SALES, EMP]])
      

      Notice that cor1 is not bound to the inner query's projection nodes variablesSet as cor0 is to the outer query's projection node.
      This results in improper subquery removal (which was originally believed to be an issue with the rule itself, discussed in CALCITE-5213)

      This is because in SqlToRelConverter.createAggImpl, it does not check for correlation variables after creating an intermediate project in the same way that SqlToRelConverter.convertSelectList does.

      This patch fixes addresses this issue:

      diff --git a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
      index d88daa631..5668bc825 100644
      --- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
      +++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
      @@ -3587,11 +3587,26 @@ private void createAggImpl(Blackboard bb,
             final RelNode inputRel = bb.root();
      
             // Project the expressions required by agg and having.
      -      bb.setRoot(
      -          relBuilder.push(inputRel)
      -              .projectNamed(preExprs.leftList(), preExprs.rightList(), false)
      -              .build(),
      -          false);
      +      {
      +        RelNode intermediateProject = relBuilder.push(inputRel)
      +            .projectNamed(preExprs.leftList(), preExprs.rightList(), false)
      +            .build();
      +        final RelNode r2;
      +        // deal with correlation
      +        final CorrelationUse p = getCorrelationUse(bb, intermediateProject);
      +        if (p != null) {
      +          assert p.r instanceof Project;
      +          // correlation variables have been normalized in p.r, we should use expressions
      +          // in p.r instead of the original exprs
      +          Project project1 = (Project) p.r;
      +          r2 = relBuilder.push(bb.root())
      +              .projectNamed(project1.getProjects(), project1.getRowType().getFieldNames(), true, ImmutableSet.of(p.id))
      +              .build();
      +        } else {
      +          r2 = intermediateProject;
      +        }
      +        bb.setRoot(r2, false);
      +      }
             bb.mapRootRelToFieldProjection.put(bb.root(), r.groupExprProjection);
      
             // REVIEW jvs 31-Oct-2007:  doesn't the declaration of
      

      Attachments

        Issue Links

          Activity

            People

              ian.bertolacci Ian Bertolacci
              ian.bertolacci Ian Bertolacci
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: