[ 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: issues-all-unsubscr...@impala.apache.org For additional commands, e-mail: issues-all-h...@impala.apache.org