impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Impala Public Jenkins (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5125: SimplifyConditionalsRule incorrectly handles aggregates
Date Mon, 24 Apr 2017 21:41:12 GMT
Impala Public Jenkins has submitted this change and it was merged.

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
Reviewed-on: http://gerrit.cloudera.org:8080/6653
Reviewed-by: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Tested-by: Impala Public Jenkins
---
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(-)

Approvals:
  Impala Public Jenkins: Verified
  Thomas Tauber-Marshall: Looks good to me, approved



-- 
To view, visit http://gerrit.cloudera.org:8080/6653
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic20b1621753980b47a612e0885804363b733f6da
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>

Mime
View raw message