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: [email protected]
For additional commands, e-mail: [email protected]
