[
https://issues.apache.org/jira/browse/IMPALA-7752?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul Rogers reassigned IMPALA-7752:
-----------------------------------
Assignee: (was: Paul Rogers)
Summary: Stmt.reset() doesn't (and can't) (was: Invalid test logic in
ExprRewriterTest)
> Stmt.reset() doesn't (and can't)
> --------------------------------
>
> 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
> Priority: Minor
>
> 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: [email protected]
For additional commands, e-mail: [email protected]
