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 2018/11/07 17:34:55 UTC
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr predicates
Paul Rogers has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11887
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
IMPALA-7818: Standardize use of Expr predicates
The Expr node provids two kinds of queries about node categories:
predicates and isMuble() functions. This change standardizes two
existing methods to use predicates instead.
First, the existing isLiteral() method is replaced by a new
IS_LITERAL predicate.
The key purpose is to clean up the confusing use of the existing
isNullLiteral() method, which actually is a check for either a null
literal or a cast of a null. To make this clearer, replaced this
method with a new IS_NULL_VALUE() predicate.
Added a new IS_NULL_LITERAL predicate for the case when a node must be
exactly the NULL literal, not a cast to NULL. Replaced instances of
foo instanceof NullLiteral with a use of the new IS_NULL_LITERAL
predicate. (This revealed bugs which will be addressed separately.)
Added an IS_NON_NULL_LITERAL to replace the two-method ideom used in
several places.
Tests: No functional change. Reran all tests to ensure nothing changed.
Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
---
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/CaseExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.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/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSet.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.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/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.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/rewrite/FoldConstantsRule.java
M fe/src/main/java/org/apache/impala/rewrite/NormalizeCountStarRule.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/service/FeSupport.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
29 files changed, 135 insertions(+), 93 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/11887/3
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr predicates
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11887 )
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
Patch Set 4:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/Expr.java@1169
PS4, Line 1169: Expr.
drop here, and other places where its not needed.
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java:
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@432
PS4, Line 432: Expr.
can drop
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
File fe/src/main/java/org/apache/impala/analysis/LikePredicate.java:
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java@132
PS4, Line 132: IS_NON_NULL_LITERAL
double-checking if this translation preserves the previous expression? isNullLiteral is replaced by the value variant but IS_NON_NULL_LITERAL uses the instanceof check. I can be convinced that it falls out to be the same, but it is not obvious.
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/rewrite/FoldConstantsRule.java
File fe/src/main/java/org/apache/impala/rewrite/FoldConstantsRule.java:
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/rewrite/FoldConstantsRule.java@52
PS4, Line 52: IS_LITERAL_VALUE
IS_LITERAL?
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 19:32:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr predicates
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/11887 )
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
Patch Set 4:
(2 comments)
Fixed check style warnings.
http://gerrit.cloudera.org:8080/#/c/11887/3/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:
http://gerrit.cloudera.org:8080/#/c/11887/3/fe/src/main/java/org/apache/impala/analysis/Expr.java@700
PS3, Line 700: Preconditions.checkState(Expr.IS_NULL_LITERAL.apply(this) ||
> line too long (92 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/11887/3/fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java
File fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java:
http://gerrit.cloudera.org:8080/#/c/11887/3/fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java@87
PS3, Line 87: if (Expr.IS_NULL_LITERAL.apply(literalValue) ||
> line too long (94 > 90)
Done
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 19:02:22 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr predicates
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11887 )
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
Patch Set 3: Code-Review+1
LGTM apart from the bot's style warnings.
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 17:56:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr predicates
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/11887 )
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
Patch Set 3:
Dry-run passed: https://jenkins.impala.io/job/pre-review-test/221/
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 17:35:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr predicates
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Impala Public Jenkins, Vuk Ercegovac,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11887
to look at the new patch set (#5).
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
IMPALA-7818: Standardize use of Expr predicates
The Expr node provide two kinds of queries about node categories:
predicates and isMumble() functions. This change standardizes two
existing methods to use predicates instead.
First, the existing isLiteral() method is replaced by a new
IS_LITERAL predicate.
The key purpose is to clean up the confusing use of the existing
isNullLiteral() method, which actually is a check for either a null
literal or a cast of a null. To make this clearer, replaced this
method with a new IS_NULL_VALUE() predicate.
Added a new IS_NULL_LITERAL predicate for the case when a node must be
exactly the NULL literal, not a cast to NULL. Replaced instances of
foo instanceof NullLiteral with a use of the new IS_NULL_LITERAL
predicate. (This revealed bugs which will be addressed separately.)
Added an IS_NON_NULL_LITERAL to replace the two-method idiom used in
several places.
Tests: No functional change. Reran all tests to ensure nothing changed.
Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
---
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/CaseExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.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/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSet.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.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/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.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/rewrite/FoldConstantsRule.java
M fe/src/main/java/org/apache/impala/rewrite/NormalizeCountStarRule.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/service/FeSupport.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
29 files changed, 138 insertions(+), 94 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/11887/5
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr predicates
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/11887 )
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
Patch Set 5:
(5 comments)
Thanks Vuk for the review. Addressed your comments.
http://gerrit.cloudera.org:8080/#/c/11887/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11887/4//COMMIT_MSG@10
PS4, Line 10: isMumble(
> sp?
Done
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/Expr.java@1169
PS4, Line 1169: IS_NU
> drop here, and other places where its not needed.
Thanks for catching this.
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java:
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@432
PS4, Line 432: IS_NU
> can drop
Done
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
File fe/src/main/java/org/apache/impala/analysis/LikePredicate.java:
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java@132
PS4, Line 132: IS_NON_NULL_LITERAL
> double-checking if this translation preserves the previous expression? isNu
I agree, the original is confusing with "isNullLiteral()" meaning "null literal or cast." Let's work this through.
The first term in the original matches only literal nodes. The second term matched Null literal or a cast of a null. But, since the first term excluded the cast, we only care about the null literal check.
The new predicate allows all literals except the null literal. So, unless I'm missing something, they seem identical.
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/rewrite/FoldConstantsRule.java
File fe/src/main/java/org/apache/impala/rewrite/FoldConstantsRule.java:
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/rewrite/FoldConstantsRule.java@52
PS4, Line 52: IS_LITERAL.apply
> IS_LITERAL?
Yes. The change shown here is needed, but is better presented in a separate patch that includes new test cases.
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 21:17:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr 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/11887 )
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 6
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Nov 2018 01:45:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11887 )
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
IMPALA-7818: Standardize use of Expr predicates
The Expr node provide two kinds of queries about node categories:
predicates and isMumble() functions. This change standardizes two
existing methods to use predicates instead.
First, the existing isLiteral() method is replaced by a new
IS_LITERAL predicate.
The key purpose is to clean up the confusing use of the existing
isNullLiteral() method, which actually is a check for either a null
literal or a cast of a null. To make this clearer, replaced this
method with a new IS_NULL_VALUE() predicate.
Added a new IS_NULL_LITERAL predicate for the case when a node must be
exactly the NULL literal, not a cast to NULL. Replaced instances of
foo instanceof NullLiteral with a use of the new IS_NULL_LITERAL
predicate. (This revealed bugs which will be addressed separately.)
Added an IS_NON_NULL_LITERAL to replace the two-method idiom used in
several places.
Tests: No functional change. Reran all tests to ensure nothing changed.
Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Reviewed-on: http://gerrit.cloudera.org:8080/11887
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
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/CaseExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.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/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSet.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.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/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.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/rewrite/FoldConstantsRule.java
M fe/src/main/java/org/apache/impala/rewrite/NormalizeCountStarRule.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/service/FeSupport.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
29 files changed, 138 insertions(+), 94 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 7
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr 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/11887 )
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1308/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 18:11:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr 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/11887 )
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1314/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 19:38:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr predicates
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11887 )
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
Patch Set 5: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
File fe/src/main/java/org/apache/impala/analysis/LikePredicate.java:
http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java@132
PS4, Line 132: IS_NON_NULL_LITERAL
> I agree, the original is confusing with "isNullLiteral()" meaning "null lit
makes sense, thanks.
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 21:35:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr predicates
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11887
to look at the new patch set (#4).
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
IMPALA-7818: Standardize use of Expr predicates
The Expr node provids two kinds of queries about node categories:
predicates and isMuble() functions. This change standardizes two
existing methods to use predicates instead.
First, the existing isLiteral() method is replaced by a new
IS_LITERAL predicate.
The key purpose is to clean up the confusing use of the existing
isNullLiteral() method, which actually is a check for either a null
literal or a cast of a null. To make this clearer, replaced this
method with a new IS_NULL_VALUE() predicate.
Added a new IS_NULL_LITERAL predicate for the case when a node must be
exactly the NULL literal, not a cast to NULL. Replaced instances of
foo instanceof NullLiteral with a use of the new IS_NULL_LITERAL
predicate. (This revealed bugs which will be addressed separately.)
Added an IS_NON_NULL_LITERAL to replace the two-method ideom used in
several places.
Tests: No functional change. Reran all tests to ensure nothing changed.
Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
---
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/CaseExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.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/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSet.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.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/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.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/rewrite/FoldConstantsRule.java
M fe/src/main/java/org/apache/impala/rewrite/NormalizeCountStarRule.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/service/FeSupport.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
29 files changed, 137 insertions(+), 93 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/11887/4
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr 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/11887 )
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 6
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 21:51:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr 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/11887 )
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1318/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 21:48:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr 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/11887 )
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11887/3/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:
http://gerrit.cloudera.org:8080/#/c/11887/3/fe/src/main/java/org/apache/impala/analysis/Expr.java@700
PS3, Line 700: Preconditions.checkState(Expr.IS_NULL_LITERAL.apply(this) || this instanceof SlotRef);
line too long (92 > 90)
http://gerrit.cloudera.org:8080/#/c/11887/3/fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java
File fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java:
http://gerrit.cloudera.org:8080/#/c/11887/3/fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java@87
PS3, Line 87: if (Expr.IS_NULL_LITERAL.apply(literalValue) || literalValue.getStringValue().isEmpty()) {
line too long (94 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 17:35:43 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr predicates
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11887 )
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11887/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11887/4//COMMIT_MSG@10
PS4, Line 10: isMuble()
sp?
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 19:34:35 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7818: Standardize use of Expr 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/11887 )
Change subject: IMPALA-7818: Standardize use of Expr predicates
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3441/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/11887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a
Gerrit-Change-Number: 11887
Gerrit-PatchSet: 6
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 21:51:32 +0000
Gerrit-HasComments: No