Hello Philip Zeyliger, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11760
to look at the new patch set (#4).
Change subject: IMPALA-7655: Rewrite if, isnull, coalesce to use CASE
......................................................................
IMPALA-7655: Rewrite if, isnull, coalesce to use CASE
See IMPALA-7655 for backgound. Tim found that the current interpreted
forms of if, isnull and coalesce are slow compared to the code-generated
CASE statement.
This patch rewrites the above functions into the equivalent CASE
structure.
The functions ifnull and nvl are aliases for isnull, so revised those to
directly rewrite them to CASE rather than first converting them to if().
Moved rewriting into our existing set of conditional rewrite rules
into SimplifyConditionalRules instead of in the parser. To make this work
(and to solve the issue in IMPALA-7741, added ifnull and nvl to the list
of builtin Impala functions.
Found a few additional simplifications for coalesce, ifnull and nvl. See
the Javadoc and unit tests for details. (See IMPALA-7750 for why the
rules are needed; solve IMPALA-7750 and the rules can be removed as the
rewrite for CASE will handle them.)
Tests for conditional functions were in one huge function along with
other rewrite tests. Moved them into a new file, then broke up the tests
by function to allow much easier debugging of each function one-by-one.
This required moving the common test mechanims into a new common base
class.
Once tests verified that the rewrites worked, we no longer have need of
the specialized C++ implementations, so removed those.
Also cleaned up a few typos, unnecessary imports and JavaDocs found
during this work.
Change-Id: I526654d8546e17b2545c42cc59dab66d9fe1b163
---
M be/src/exprs/conditional-functions-ir.cc
M be/src/exprs/conditional-functions.cc
M be/src/exprs/conditional-functions.h
M be/src/exprs/scalar-expr.cc
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
A fe/src/test/java/org/apache/impala/analysis/BaseRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
A fe/src/test/java/org/apache/impala/analysis/SimplifyConditionalRulesTest.java
12 files changed, 617 insertions(+), 493 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/11760/4
--
To view, visit http://gerrit.cloudera.org:8080/11760
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I526654d8546e17b2545c42cc59dab66d9fe1b163
Gerrit-Change-Number: 11760
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <par0...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <par0...@yahoo.com>
Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
