[jira] [Created] (IMPALA-7752) Invalid test logic in ExprRewriterTest

["Paul Rogers (JIRA)" <ji...@apache.org>]

Paul Rogers created IMPALA-7752:
-----------------------------------

             Summary: Invalid test logic in ExprRewriterTest
                 Key: IMPALA-7752
                 URL: https://issues.apache.org/jira/browse/IMPALA-7752
             Project: IMPALA
          Issue Type: Bug
          Components: Frontend
    Affects Versions: Impala 3.0
            Reporter: Paul Rogers
            Assignee: Paul Rogers


The test {{ExprRewriteTest}} has the following logic:

{code:java}
  public void RewritesOk(String stmt, int expectedNumChanges,
      int expectedNumExprTrees) throws ImpalaException {
    // Analyze without rewrites since that's what we want to test here.
    StatementBase parsedStmt = (StatementBase) ParsesOk(stmt);
    ...
    parsedStmt.rewriteExprs(exprToTrue_);
    ...

    // Make sure the stmt can be successfully re-analyzed.
    parsedStmt.reset();
    AnalyzesOkNoRewrite(parsedStmt);
  }
{code}

Basically, this replaces all expressions with a Boolean constant, then counts the number of replacements. A fine test. Then, the {{reset())}} call is supposed to put things back the way they were.

The problem is, the rewrite rule replaces the one and only copy of the {{SELECT}} list expressions. The second time around, we get a failure because the {{ORDER BY}} clause (which was kept as an original copy) refers to the now-gone {{SELECT}} clause.

This error was not previously seen because a prior bug masked it.

This is an odd bug as {{reset()}} is called only from this one place.

The premise of test itself is invalid: we want to know that, after we rewrite the query from

{code:sql}
select a.int_col a, 10 b, 20.2 c, ...
order by a.int_col, 4 limit 10
{code}

To

{code:sql}
select FALSE a, FALSE b, FALSE c, ...
order by a.int_col, 4 limit 10
{code}

We assert that the query should again analyze correctly. This is an unrealistic expectation. Once the above bug is fixed, we verify that the new query is actually invalid, which, in fact, it is.

Two fixes are possible:

# Create copies of all lists that are rewritten ({{SELECT}}, {{HAVING}}, etc.)
# Remove the {{reset()}} test and (since this is the only use), the {{reset()}} code since it cannot actually do what it is advertised to do.

Since {{reset()}} is never used except in tests, and the premise is invalid, this ticket proposes to remove the {{reset()}} logic and remove the part of the test code that validates the reset.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-all-unsubscribe@impala.apache.org
For additional commands, e-mail: issues-all-help@impala.apache.org