Hello Bharath Vissapragada, Philip Zeyliger, Tim Armstrong, Csaba Ringhofer,
Impala Public Jenkins, Vuk Ercegovac,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11760
to look at the new patch set (#11).
Change subject: IMPALA-7655: Rewrite if, isnull to use CASE
......................................................................
IMPALA-7655: Rewrite if, isnull 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 if and isnull functions into the equivalent
CASE structure. Coalesce is omitted due to limitations within the
code.
Caveats:
* IMPALA-7753: Conditionals in the top-level ORDER BY clause are not
rewritten, but those one or more levels down are. The result is that
PlannerTest files still show uses of if and isnull despite this fix.
* IMPALA-7785: Similar, but says that the GROUP BY clause is not
analyzed prior to rewrites, so the code again forces analysis.
There are some odd "analyze()" calls in the code to work around this
limitation snd the next two.
* IMPALA-7769: Some expressions involving NULL are not simplified.
As a result, some of the rewrites don't result in the most optimal
result.
* IMPALA-7754: The rewrite engine ignores unanalyzed expressions, yet
the rewrite engine does not, in general, re-analyze the expressions it
produces, causing simplifications to be skipped.
* IMPALA-4356: Even after this change, code gen won't occur for SELECT
expressions executed in teh root fragment (the most common case
in simple tests.)
Because of the above, some if and isnull calls are not rewritten, and
others are not rewritten to the simplest forms. The tests contain
commented-out tests, and comments with bug numbers, to record these
known issues. Still, the current fix is useful enough by itself to
proceed despite the above.
While the desire was to replace the BE implementation of if and isnull,
the caveats prevent doing so at present, so the BE retains the original
interpreted forms. They are used when: 1) top-level conditions in the
ORDER BY clause, and 2) if the user disables rewrites.
Conditional function rewrites are moved into a new class,
RewriteConditionalsRule in order to keep things simple.
Coalesce() is implemented, but disabled. The old rewrites are retained
until blocking issues are fixed.
Testing:
* Tests for conditional functions are pulled out into a new test class.
* A new base class holds code common to the new class and the existing
ExprRewriterTest.
* A new FullRewriteTest class tests rewrites that require multiple
passes thorugh teh rewrite engine.
* Planner Test expected results were updated based on the rewritten
expressions.
Change-Id: I526654d8546e17b2545c42cc59dab66d9fe1b163
---
M be/src/exprs/conditional-functions.h
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
A fe/src/main/java/org/apache/impala/rewrite/RewriteConditionalFnsRule.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/FullRewriteTest.java
A fe/src/test/java/org/apache/impala/analysis/RewriteConditionalFnsRuleTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M
testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
M
testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
13 files changed, 939 insertions(+), 256 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/11760/11
--
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: 11
Gerrit-Owner: Paul Rogers <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Paul Rogers <[email protected]>
Gerrit-Reviewer: Philip Zeyliger <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Vuk Ercegovac <[email protected]>
