Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: Impala 2.7.0
    • Fix Version/s: Impala 2.8.0
    • Component/s: Frontend
    • Labels:
      None

      Description

      To address issues like IMPALA-1286 we should introduce a new expr rewrite phase where analyzed exprs can be transformed with rules. The new phase could be similar to the subquery rewrite phase where we transform exprs in-place and then reset() and analyze() the whole statement again.

        Issue Links

          Activity

          Hide
          alex.behm Alexander Behm added a comment -

          commit 1d8cdb02c67fad9279a899626f3cd56bce0b0eb3
          Author: Alex Behm <alex.behm@cloudera.com>
          Date: Fri Oct 14 11:19:39 2016 -0700

          IMPALA-4309: Introduce Expr rewrite phase and supporting classes.

          Introduces a new phase for rewriting Exprs after analysis and
          before subquery rewriting. The transformed Exprs replace the
          original ones in analyzed statements. If Exprs were changed,
          the whole statement is reset() and re-analyzed, similar to how
          subqueries are rewritten. If both Exprs and subqueries are
          rewritten there is only one re-analysis of the changed statement.

          The following new classes work together to perform transformations:
          1. ExprRewriteRule

          • base class for Expr transformation rules
            2. ExprRewriter
          • drives the transformation of Exprs using a list of
            ExprRewriteRules

          Statements that have exprs to be rewritten need to implement
          a new method rewriteExprs() that accepts an ExprRewriter.

          As an example, this patch adds a rule for converting
          BetweenPredicates into their equivalent CompoundPredicates.
          The BetweenPredicate has been notoriously buggy due to a lack
          of such a separate rewrite phase and is now cleaned up.

          Testing:
          1. Added a new test for checking that the rewrite framework
          covers all relevant statements, clauses and can properly
          handle nested statements and subqueries.
          2. Added a new test for ExprRewriteRules and implemented
          tests for the BetweenPredicate rewrite.
          2. There are many existing tests for BetweePredicates and
          they all exercise the new rewrite rule/phase.
          3. Ran a private core/hdfs run and it passed.

          Change-Id: I2279dc984bcf7742db4fa3b1aa67283ecbb05e6e
          Reviewed-on: http://gerrit.cloudera.org:8080/4746
          Reviewed-by: Alex Behm <alex.behm@cloudera.com>
          Tested-by: Internal Jenkins

          Show
          alex.behm Alexander Behm added a comment - commit 1d8cdb02c67fad9279a899626f3cd56bce0b0eb3 Author: Alex Behm <alex.behm@cloudera.com> Date: Fri Oct 14 11:19:39 2016 -0700 IMPALA-4309 : Introduce Expr rewrite phase and supporting classes. Introduces a new phase for rewriting Exprs after analysis and before subquery rewriting. The transformed Exprs replace the original ones in analyzed statements. If Exprs were changed, the whole statement is reset() and re-analyzed, similar to how subqueries are rewritten. If both Exprs and subqueries are rewritten there is only one re-analysis of the changed statement. The following new classes work together to perform transformations: 1. ExprRewriteRule base class for Expr transformation rules 2. ExprRewriter drives the transformation of Exprs using a list of ExprRewriteRules Statements that have exprs to be rewritten need to implement a new method rewriteExprs() that accepts an ExprRewriter. As an example, this patch adds a rule for converting BetweenPredicates into their equivalent CompoundPredicates. The BetweenPredicate has been notoriously buggy due to a lack of such a separate rewrite phase and is now cleaned up. Testing: 1. Added a new test for checking that the rewrite framework covers all relevant statements, clauses and can properly handle nested statements and subqueries. 2. Added a new test for ExprRewriteRules and implemented tests for the BetweenPredicate rewrite. 2. There are many existing tests for BetweePredicates and they all exercise the new rewrite rule/phase. 3. Ran a private core/hdfs run and it passed. Change-Id: I2279dc984bcf7742db4fa3b1aa67283ecbb05e6e Reviewed-on: http://gerrit.cloudera.org:8080/4746 Reviewed-by: Alex Behm <alex.behm@cloudera.com> Tested-by: Internal Jenkins

            People

            • Assignee:
              alex.behm Alexander Behm
              Reporter:
              alex.behm Alexander Behm
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development