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

Register all combinations of materialization substitutions

    Details

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

      Description

      When a query has multiple table references, there could be:
      1) Multiple combinations of substituted Rels if one materialization is applicable for more than one sub-tree.
      2) Multiple combinations of substituted Rels if different materializations are applicable for different sub-trees respectively.

        @Test public void testSingleMaterializationMultiUsage() {
          String q = "select *\n"
              + "from (select * from \"emps\" where \"empid\" < 300)\n"
              + "join (select * from \"emps\" where \"empid\" < 200) using (\"empid\")";
          try {
            Prepare.THREAD_TRIM.set(true);
            MaterializationService.setThreadLocal();
            CalciteAssert.that()
                .withMaterializations(JdbcTest.HR_MODEL, 
                    "m0", "select * from \"emps\" where \"empid\" < 500")              
                .query(q)
                .enableMaterializations(true)
                .explainMatches("", new Function<ResultSet, Void>() {
                  public Void apply(ResultSet s) {
                    try {
                      final String actual = Util.toLinux(CalciteAssert.toString(s));
                      final String scan = "EnumerableTableScan(table=[[hr, m0]])";
                      assertTrue(actual + " should have had two occurrences of " + scan, 
                          StringUtils.countMatches(actual, scan) == 2);
                      return null;
                    } catch (SQLException e) {
                      throw new RuntimeException(e);
                    }
                  }
                })
                .sameResultWithMaterializationsDisabled();
          } finally {
            Prepare.THREAD_TRIM.set(false);
          }
        }
      
        @Test public void testMultiMaterializationMultiUsage() {
          String q = "select *\n"
              + "from (select * from \"emps\" where \"empid\" < 300)\n"
              + "join (select * from \"emps\" where \"deptno\" < 10) using (\"empid\")";
          try {
            Prepare.THREAD_TRIM.set(true);
            MaterializationService.setThreadLocal();
            CalciteAssert.that()
                .withMaterializations(JdbcTest.HR_MODEL, 
                    "m0", "select * from \"emps\" where \"empid\" < 500",
                    "m1", "select * from \"emps\" where \"deptno\" < 20")              
                .query(q)
                .enableMaterializations(true)
                .explainContains("EnumerableTableScan(table=[[hr, m0]])")
                .explainContains("EnumerableTableScan(table=[[hr, m1]])")
                .sameResultWithMaterializationsDisabled();
          } finally {
            Prepare.THREAD_TRIM.set(false);
          }
        }
      
      1. CALCITE-890_plus_2.patch
        18 kB
        Maryann Xue
      2. CALCITE-890_plus_3.patch
        22 kB
        Maryann Xue
      3. CALCITE-890_plus.patch
        21 kB
        Maryann Xue
      4. CALCITE-890.patch
        8 kB
        Maryann Xue

        Activity

        Hide
        julianhyde Julian Hyde added a comment -
        Show
        julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/9d0fef31 . Thanks for the patch, Maryann Xue !
        Hide
        maryannxue Maryann Xue added a comment - - edited

        Just found the fix wasn't complete. For example, A join B, while A and B can be both substituted by materialization M, it could only return M join B, M join M, but leaving out A join M. This was because the SubstitutionVisitor could only match from the beginning and was not able to "skip".

        Show
        maryannxue Maryann Xue added a comment - - edited Just found the fix wasn't complete. For example, A join B, while A and B can be both substituted by materialization M, it could only return M join B, M join M, but leaving out A join M. This was because the SubstitutionVisitor could only match from the beginning and was not able to "skip".
        Hide
        maryannxue Maryann Xue added a comment -

        Not sure, but just let me know if I need to create a new issue for follow up, Julian Hyde.

        Show
        maryannxue Maryann Xue added a comment - Not sure, but just let me know if I need to create a new issue for follow up, Julian Hyde .
        Hide
        julianhyde Julian Hyde added a comment -

        This issue will be fine. If you manage to make some improvements check in with a 'further work on CALCITE-890' comment or similar.

        @amogh, Since you have been working in this area, please chime in if you have ideas. SubstitutionVisitor is powerful but I am not convinced we have the right design yet. I'm willing to consider a wide range of options, including a rewrite, or moving its functionality into Volcano.

        Show
        julianhyde Julian Hyde added a comment - This issue will be fine. If you manage to make some improvements check in with a 'further work on CALCITE-890 ' comment or similar. @amogh, Since you have been working in this area, please chime in if you have ideas. SubstitutionVisitor is powerful but I am not convinced we have the right design yet. I'm willing to consider a wide range of options, including a rewrite, or moving its functionality into Volcano.
        Hide
        maryannxue Maryann Xue added a comment -

        Here's an improved solution with much less changes than CALCITE-890_plus.patch. Now going with the "find-all-and-then-replace" approach, but only that it's doing reverse replace. All substitutions go in one pass over the tree, and meanwhile we keep track of all the substitutions that have happened, and in the later processing stage we do recursive reverse replacement according to our records.
        Also added some comments in SubstitutionVisitor.go(), hopefully that'll be helpful to anyone who will look at or touch this piece of logic in future if SubstitutionVisitor still lingers for a while.
        Looks like we'll end up doing some materialization substitution in VolcanoPlanner as well, esp. for our local index join. Not sure, but I guess SubstitutionVisitor's top-down matching is still better in some cases, e.g., once we know that our index covers the projection we don't have to consider the non-covering approach for a child TableScan at all.

        Show
        maryannxue Maryann Xue added a comment - Here's an improved solution with much less changes than CALCITE-890 _plus.patch. Now going with the "find-all-and-then-replace" approach, but only that it's doing reverse replace. All substitutions go in one pass over the tree, and meanwhile we keep track of all the substitutions that have happened, and in the later processing stage we do recursive reverse replacement according to our records. Also added some comments in SubstitutionVisitor.go(), hopefully that'll be helpful to anyone who will look at or touch this piece of logic in future if SubstitutionVisitor still lingers for a while. Looks like we'll end up doing some materialization substitution in VolcanoPlanner as well, esp. for our local index join. Not sure, but I guess SubstitutionVisitor's top-down matching is still better in some cases, e.g., once we know that our index covers the projection we don't have to consider the non-covering approach for a child TableScan at all.
        Hide
        maryannxue Maryann Xue added a comment -

        + 1 bug fix.

        Show
        maryannxue Maryann Xue added a comment - + 1 bug fix.
        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/5149568c . Thanks for the patch(es), Maryann Xue !
        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)

          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