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

TableScan without Project cannot be substituted by any projected materialization

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.5.0
    • Component/s: None
    • Labels:

      Description

      A TableScan that does not have a Project as its parent cannot be matched by any UnifyRules and thus cannot be substituted by a projected materialization; while the same TableScan with an identity projection could.

        @Test public void testJoinMaterialization2() {
          String q = "select *\n"
                  + "from \"emps\"\n"
                  + "join \"depts\" using (\"deptno\")";
          checkMaterialize("select \"deptno\", \"empid\", \"name\", \"salary\", \"commission\" from \"emps\"", q);
        }
      

      In the above test case, the initial Rel was like:

      LogicalProject(...)
        LogicalJoin(...)
          TableScan(table=[[hr, emps]])
          TableScan(table=[[hr, depts]])
      

      And the queryDescendent TableScan(table=[[hr, emps]]) cannot be matched by any UnifyRules that could have matched LogicalProject(<identity_project_list>, TableScan(table=[[hr, emps]])).

      The following changes would make this test case pass. Not sure if this would be the best fix, but it is at least a good illustration of the problem.

      diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
      index 7762bf7..ccece75 100644
      --- a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
      +++ b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
      @@ -2861,9 +2861,6 @@ public static RelNode permute(
         public static RelNode createProject(final RelFactories.ProjectFactory factory,
             final RelNode child, final List<Integer> posList) {
           RelDataType rowType = child.getRowType();
      -    if (Mappings.isIdentity(posList, rowType.getFieldCount())) {
      -      return child;
      -    }
           final List<String> fieldNames = rowType.getFieldNames();
           final RexBuilder rexBuilder = child.getCluster().getRexBuilder();
           return factory.createProject(child,
      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 50f22c1..b0a519a 100644
      --- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
      +++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
      @@ -2240,6 +2240,8 @@ protected RelNode createJoin(
             }
           }
      
      +    leftRel = leftRel instanceof Project ? leftRel : RelOptUtil.createProject(leftRel, ImmutableIntList.identity(leftRel.getRowType().getFieldCount()));
      +    rightRel = rightRel instanceof Project ? rightRel : RelOptUtil.createProject(rightRel, ImmutableIntList.identity(rightRel.getRowType().getFieldCount()));
           final Join originalJoin =
               (Join) RelFactories.DEFAULT_JOIN_FACTORY.createJoin(leftRel, rightRel,
                   joinCond, joinType, ImmutableSet.<String>of(), false);
      

        Activity

        Hide
        jcamachorodriguez Jesus Camacho Rodriguez added a comment -

        Resolved in release 1.5.0 (2015-11-10)

        Show
        jcamachorodriguez Jesus Camacho Rodriguez added a comment - Resolved in release 1.5.0 (2015-11-10)
        Hide
        julianhyde Julian Hyde added a comment -
        Show
        julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/5c455475 . Thanks for the patch, Maryann Xue !
        Hide
        maryannxue Maryann Xue added a comment -

        Julian Hyde Could you please review this patch?

        Show
        maryannxue Maryann Xue added a comment - Julian Hyde Could you please review this patch?
        Hide
        maryannxue Maryann Xue added a comment -

        I agree, Julian Hyde. We may need a substitution rule to match the tableScan with the projected materialization when it is a full projection.

        Show
        maryannxue Maryann Xue added a comment - I agree, Julian Hyde . We may need a substitution rule to match the tableScan with the projected materialization when it is a full projection.
        Hide
        julianhyde Julian Hyde added a comment -

        I made the changes you suggest and was able to reproduce the problem. However, we would need a more robust solution than the code you provide.

        Show
        julianhyde Julian Hyde added a comment - I made the changes you suggest and was able to reproduce the problem. However, we would need a more robust solution than the code you provide.

          People

          • Assignee:
            maryannxue Maryann Xue
            Reporter:
            maryannxue Maryann Xue
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development