Hello Alex Behm,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6653
to look at the new patch set (#3).
Change subject: IMPALA-5125: SimplifyConditionalsRule incorrectly handles
aggregates
......................................................................
IMPALA-5125: SimplifyConditionalsRule incorrectly handles aggregates
This patch addresses 3 issues:
- SelectList.reset() didn't properly reset some of its members, though
they're documented as needing to be reset. This was causing a crash
when the Planner attempted to make an aggregation node for an agg
function that had been eliminated by expr rewriting. While I'm here,
I added resetting of all of SelectList's members that need to be
reset, and fixed the documentation of one member that shouldn't be
reset.
- SimplifyConditionalsRule was changing the meaning of queries that
contain agg functions, e.g. because "select if(true, 0, sum(id))"
is not equivalent to "select 0". The fix is to not return the
simplfied expr if it removes all aggregates.
- ExprRewriteRulesTest was performing rewrites on the result exprs of
the SelectStmt, which causes problems if the result exprs have been
substituted. In normal query execution, we don't rewrite the result
exprs anyway, so the fix is to match normal query execution and
rewrite the select list exprs.
Testing:
- Added e2e test to exprs.test.
- Added unit test to ExprRewriteRulesTest.
Change-Id: Ic20b1621753980b47a612e0885804363b733f6da
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
4 files changed, 41 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/6653/3
--
To view, visit http://gerrit.cloudera.org:8080/6653
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic20b1621753980b47a612e0885804363b733f6da
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
