You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Paul Rogers (Code Review)" <ge...@cloudera.org> on 2019/02/21 18:47:24 UTC
[Impala-ASF-CR] IMPALA-7781: Clean up expr rewriter predicates
Paul Rogers has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11875
Change subject: IMPALA-7781: Clean up expr rewriter predicates
......................................................................
IMPALA-7781: Clean up expr rewriter predicates
The expression rewriter often checks if an expression is null or a
literal, but does so using ad-hoc "instance of" or similar checks.
Yet, the expression nodes provide predicates that do the same job.
This change uses the defined predicates in place of the ad-hoc tests.
Add new predicates for the most common cases: literal, null literal,
null value, non-null literal, and literal-value. Converted several Expr
class methods to predicates.
The predicates handle cases that the ad-hoc tests do not. For example,
when the constant folding rule produces a NULL result, it casts that
null to some type: CAST(NULL AS INT), say. Later, other rules want to
check if that result is null. A simple check for a null literal won't
work, we need the IS_NULL_VALUE predicate which checks for the CAST
case.
This change adds predicate and revises the rewrite rules to use the
new predicates.
Also adds a number of minor Java cleanups found while doing the fix.
One of which is the toSql() for numeric values: tests showed that
the form of zero printed depended on the precision and scale, with one
value printing as 0E-38. Modified the code to emit "0" for all zero
values (since, unlike Java, SQL attaches no meaning to 0.0 as oppposed
to just 0.)
Detailed testing revealed missed simplification opportunites, especially
in CASE. Added the missing logic. For CASE, this meant a wholesale
refactoring of the code in order to better deal with nuances such as
CASE null WHEN ... END --> NULL
CASE ... ELSE NULL END --> CASE ... END
CASE ... WHEN ... THEN NULL END CASE ... END
The constant folding rule always wrapped the result in a cast,
even if the result was of the correct type. Skipped the unnecessary
cast to avoid repeated passes through this rule.
Testing:
* Refactored conditional tests into a new class, broken down
by type expression type for easier debugging.
* Created a base class for the existing and new expression test
classes.
* Created a test fixture to manage analysis tests to make it easier
to grab and inspect various aspects of the plan, as well as to control
query and other options.
* Added new tests that failed without these changes, but which pass
with them.
* Some of the plans in PlannerTest changed: verified that the new
plans differ only in the added simplifications.
Change-Id: I92d117145e427fcc5c71299096cf0e2d964ab63c
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/common/TreeNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/rewrite/BetweenToCompoundRule.java
M fe/src/main/java/org/apache/impala/rewrite/EqualityDisjunctsToInRule.java
M fe/src/main/java/org/apache/impala/rewrite/FoldConstantsRule.java
M fe/src/main/java/org/apache/impala/rewrite/NormalizeBinaryPredicatesRule.java
M fe/src/main/java/org/apache/impala/rewrite/NormalizeCountStarRule.java
M fe/src/main/java/org/apache/impala/rewrite/NormalizeExprsRule.java
M fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java
M fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java
M fe/src/main/java/org/apache/impala/rewrite/SimplifyDistinctFromRule.java
A fe/src/test/java/org/apache/impala/analysis/AnalysisFixture.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/ExtendedRewriteTest.java
A fe/src/test/java/org/apache/impala/analysis/SimplifyConditionalsRuleTest.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/testutil/TestUtils.java
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-stats-agg.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-views.test
43 files changed, 1,375 insertions(+), 456 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/11875/3
--
To view, visit http://gerrit.cloudera.org:8080/11875
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I92d117145e427fcc5c71299096cf0e2d964ab63c
Gerrit-Change-Number: 11875
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-7781: Clean up expr rewriter predicates
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has abandoned this change. ( http://gerrit.cloudera.org:8080/11875 )
Change subject: IMPALA-7781: Clean up expr rewriter predicates
......................................................................
Abandoned
Draft
--
To view, visit http://gerrit.cloudera.org:8080/11875
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I92d117145e427fcc5c71299096cf0e2d964ab63c
Gerrit-Change-Number: 11875
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-7781: Clean up expr rewriter predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11875 )
Change subject: IMPALA-7781: Clean up expr rewriter predicates
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11875/3/fe/src/test/java/org/apache/impala/testutil/TestUtils.java
File fe/src/test/java/org/apache/impala/testutil/TestUtils.java:
http://gerrit.cloudera.org:8080/#/c/11875/3/fe/src/test/java/org/apache/impala/testutil/TestUtils.java@293
PS3, Line 293: return createQueryContext(Catalog.DEFAULT_DB, System.getProperty("user.name"), options);
line too long (92 > 90)
http://gerrit.cloudera.org:8080/#/c/11875/3/fe/src/test/java/org/apache/impala/testutil/TestUtils.java@301
PS3, Line 301: public static TQueryCtx createQueryContext(String defaultDb, String user, TQueryOptions options) {
line too long (100 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/11875
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I92d117145e427fcc5c71299096cf0e2d964ab63c
Gerrit-Change-Number: 11875
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Feb 2019 18:48:12 +0000
Gerrit-HasComments: Yes