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/01/07 01:11:39 UTC

[Impala-ASF-CR] IMPALA-8041, Part 1: Move rewrite rules into expr nodes

Paul Rogers has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12143


Change subject: IMPALA-8041, Part 1: Move rewrite rules into expr nodes
......................................................................

IMPALA-8041, Part 1: Move rewrite rules into expr nodes

This patch is part of the task to integrate expression rewrites into
the analysis process to avoid the need for two analysis passes.

The analyzer provides a rewrite engine which traverses the
expression tree and compares each node against a list of rewrite rules.

The project goal is to apply the rules as we analyze each node. The
simplest way to do so is for each node to hold the code for its own
rewrites, callable via a virtual method: rewrite().

This patch is a first step: the rewrite code moves from the various
rewrite rules into the expression node that is to be rewritten. In some
cases nodes have a single rule which is moved from the rewrite rule
directly into the rewrite() method.

In other cases, a node has multiple rules. The different rules for the
same node are moved into distinct methods. Though these methods are
all called by the main rewrite() method, nothing calls that rewrite()
method in this patch.

Finally, this patch introduces a "stub" expression analyzer. In a later
patch this class will drive the combined analysis/rewrite logic. For
now, it is mostly a placeholder.

The rewrite rules themselves are now just stubs retained to minimze
change. A future patch will retire them once the analysis/rewrite
integration is complete.

This patch is designed to have no functional change: code merely
moves from one location (the rewrite rules) to another (the expression
nodes). Some "shim" code is added, but the functionality is unchanged.

Tests: reran all FE tests to verify no change in behavior.

Change-Id: Ifda7082abea4f6448e6cfef0d9ef7b8565d73fce
---
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/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
A fe/src/main/java/org/apache/impala/analysis/ExprAnalyzer.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/common/TreeNode.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/ExprRewriter.java
M fe/src/main/java/org/apache/impala/rewrite/ExtractCommonConjunctRule.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/SimplifyConditionalsRule.java
M fe/src/main/java/org/apache/impala/rewrite/SimplifyDistinctFromRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
18 files changed, 676 insertions(+), 375 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/12143/1
-- 
To view, visit http://gerrit.cloudera.org:8080/12143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifda7082abea4f6448e6cfef0d9ef7b8565d73fce
Gerrit-Change-Number: 12143
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-8041, Part 1: Move rewrite rules into expr nodes

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12143 )

Change subject: IMPALA-8041, Part 1: Move rewrite rules into expr nodes
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1865/ : 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/12143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifda7082abea4f6448e6cfef0d9ef7b8565d73fce
Gerrit-Change-Number: 12143
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Jan 2019 21:38:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8041, Part 1: Move rewrite rules into expr nodes

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12143 )

Change subject: IMPALA-8041, Part 1: Move rewrite rules into expr nodes
......................................................................


Patch Set 1:

Passed pre-commit tests: https://jenkins.impala.io/job/pre-review-test/273/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifda7082abea4f6448e6cfef0d9ef7b8565d73fce
Gerrit-Change-Number: 12143
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Mon, 07 Jan 2019 01:12:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8041, Part 1: Move rewrite rules into expr nodes

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12143 )

Change subject: IMPALA-8041, Part 1: Move rewrite rules into expr nodes
......................................................................


Patch Set 2:

Rebased on latest master


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifda7082abea4f6448e6cfef0d9ef7b8565d73fce
Gerrit-Change-Number: 12143
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 24 Jan 2019 03:44:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8041, Part 1: Move rewrite rules into expr nodes

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12143 )

Change subject: IMPALA-8041, Part 1: Move rewrite rules into expr nodes
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/2014/ : 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/12143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifda7082abea4f6448e6cfef0d9ef7b8565d73fce
Gerrit-Change-Number: 12143
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Feb 2019 02:10:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8041, Part 1: Move rewrite rules into expr nodes

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12143

to look at the new patch set (#2).

Change subject: IMPALA-8041, Part 1: Move rewrite rules into expr nodes
......................................................................

IMPALA-8041, Part 1: Move rewrite rules into expr nodes

This patch is part of the task to integrate expression rewrites into
the analysis process to avoid the need for two analysis passes.

The analyzer provides a rewrite engine which traverses the
expression tree and compares each node against a list of rewrite rules,
typically by performing an "instanceof" comparison of the node with the
target node type.

This project seeks to apply the rules as we analyze each node. The
simplest way to do so is for each node to hold the code for its own
rewrites, callable via a virtual method: rewrite().

This patch is a first step: the rewrite code moves from the various
rewrite rules into the expression node that is to be rewritten. In some
cases nodes have a single rule which is moved from the rewrite rule
directly into the rewrite() method.

In other cases, a node has multiple rules. The different rules for the
same node are moved into distinct methods. Though these methods are
all called by the main rewrite() method, nothing calls that rewrite()
method in this patch.

Finally, this patch introduces a "stub" expression analyzer. In a later
patch this class will drive the combined analysis/rewrite logic. For
now, it is mostly a placeholder.

The rewrite rules themselves are now just stubs retained to minimze
change. A future patch will retire them once the analysis/rewrite
integration is complete.

This patch is designed to have no functional change: code merely
moves from one location (the rewrite rules) to another (the expression
nodes). Some "shim" code is added, but the functionality is unchanged.

Tests: reran all FE tests to verify no change in behavior.

Change-Id: Ifda7082abea4f6448e6cfef0d9ef7b8565d73fce
---
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/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
A fe/src/main/java/org/apache/impala/analysis/ExprAnalyzer.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/common/TreeNode.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/ExprRewriter.java
M fe/src/main/java/org/apache/impala/rewrite/ExtractCommonConjunctRule.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/SimplifyConditionalsRule.java
M fe/src/main/java/org/apache/impala/rewrite/SimplifyDistinctFromRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
18 files changed, 686 insertions(+), 378 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/12143/2
-- 
To view, visit http://gerrit.cloudera.org:8080/12143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifda7082abea4f6448e6cfef0d9ef7b8565d73fce
Gerrit-Change-Number: 12143
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-8041, Part 1: Move rewrite rules into expr nodes

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12143 )

Change subject: IMPALA-8041, Part 1: Move rewrite rules into expr nodes
......................................................................


Patch Set 2:

Rebased on latest master. Resolved check style issues.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifda7082abea4f6448e6cfef0d9ef7b8565d73fce
Gerrit-Change-Number: 12143
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Jan 2019 20:59:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8041, Part 1: Move rewrite rules into expr nodes

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has abandoned this change. ( http://gerrit.cloudera.org:8080/12143 )

Change subject: IMPALA-8041, Part 1: Move rewrite rules into expr nodes
......................................................................


Abandoned

Not a priority at the moment.
-- 
To view, visit http://gerrit.cloudera.org:8080/12143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ifda7082abea4f6448e6cfef0d9ef7b8565d73fce
Gerrit-Change-Number: 12143
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-8041, Part 1: Move rewrite rules into expr nodes

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12143 )

Change subject: IMPALA-8041, Part 1: Move rewrite rules into expr nodes
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1721/ : 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/12143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifda7082abea4f6448e6cfef0d9ef7b8565d73fce
Gerrit-Change-Number: 12143
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Mon, 07 Jan 2019 01:47:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8041, Part 1: Move rewrite rules into expr nodes

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12143

to look at the new patch set (#4).

Change subject: IMPALA-8041, Part 1: Move rewrite rules into expr nodes
......................................................................

IMPALA-8041, Part 1: Move rewrite rules into expr nodes

This patch is part of the task to integrate expression rewrites into
the analysis process to avoid the need for two analysis passes.

The analyzer provides a rewrite engine which traverses the
expression tree and compares each node against a list of rewrite rules,
typically by performing an "instanceof" comparison of the node with the
target node type.

This project seeks to apply the rules as we analyze each node. The
simplest way to do so is for each node to hold the code for its own
rewrites, callable via a virtual method: rewrite().

This patch is a first step: the rewrite code moves from the various
rewrite rules into the expression node that is to be rewritten. In some
cases nodes have a single rule which is moved from the rewrite rule
directly into the rewrite() method.

In other cases, a node has multiple rules. The different rules for the
same node are moved into distinct methods. Though these methods are
all called by the main rewrite() method, nothing calls that rewrite()
method in this patch.

Finally, this patch introduces a "stub" expression analyzer. In a later
patch this class will drive the combined analysis/rewrite logic. For
now, it is mostly a placeholder.

The rewrite rules themselves are now just stubs retained to minimze
change. A future patch will retire them once the analysis/rewrite
integration is complete.

This patch is designed to have no functional change: code merely
moves from one location (the rewrite rules) to another (the expression
nodes). Some "shim" code is added, but the functionality is unchanged.

Tests: reran all FE tests to verify no change in behavior.

Change-Id: Ifda7082abea4f6448e6cfef0d9ef7b8565d73fce
---
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/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
A fe/src/main/java/org/apache/impala/analysis/ExprAnalyzer.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.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/ExprRewriter.java
M fe/src/main/java/org/apache/impala/rewrite/ExtractCommonConjunctRule.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/SimplifyConditionalsRule.java
M fe/src/main/java/org/apache/impala/rewrite/SimplifyDistinctFromRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
17 files changed, 686 insertions(+), 375 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/12143/4
-- 
To view, visit http://gerrit.cloudera.org:8080/12143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifda7082abea4f6448e6cfef0d9ef7b8565d73fce
Gerrit-Change-Number: 12143
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-8041, Part 1: Move rewrite rules into expr nodes

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12143 )

Change subject: IMPALA-8041, Part 1: Move rewrite rules into expr nodes
......................................................................


Patch Set 1:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/12143/1/fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
File fe/src/main/java/org/apache/impala/analysis/CaseExpr.java:

http://gerrit.cloudera.org:8080/#/c/12143/1/fe/src/main/java/org/apache/impala/analysis/CaseExpr.java@404
PS1, Line 404:           whenExpr = exprAnalyzer.analyzer().getConstantFolder().rewrite(pred, exprAnalyzer.analyzer());
line too long (104 > 90)


http://gerrit.cloudera.org:8080/#/c/12143/1/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
File fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java:

http://gerrit.cloudera.org:8080/#/c/12143/1/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java@380
PS1, Line 380:    * Takes the children of an OR predicate and attempts to combine them into a single IN predicate.
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/12143/1/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java@381
PS1, Line 381:    * The transformation is applied if both children are equality predicates with a literal on the
line too long (97 > 90)


http://gerrit.cloudera.org:8080/#/c/12143/1/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/12143/1/fe/src/main/java/org/apache/impala/analysis/Expr.java@444
PS1, Line 444:   protected Expr rewrite(ExprAnalyzer exprAnalyzer) throws AnalysisException { return this; }
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/12143/1/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/12143/1/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@710
PS1, Line 710:    * Simplify COALESCE by skipping leading nulls and applying the following transformations:
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/12143/1/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@726
PS1, Line 726:         List<Expr> newChildren = Lists.newArrayList(getChildren().subList(i, numChildren));
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/12143/1/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@750
PS1, Line 750:     return new FunctionCallExpr(new FunctionName("count"), FunctionParams.createStarParam());
line too long (93 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifda7082abea4f6448e6cfef0d9ef7b8565d73fce
Gerrit-Change-Number: 12143
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Mon, 07 Jan 2019 01:12:34 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8041, Part 1: Move rewrite rules into expr nodes

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12143

to look at the new patch set (#3).

Change subject: IMPALA-8041, Part 1: Move rewrite rules into expr nodes
......................................................................

IMPALA-8041, Part 1: Move rewrite rules into expr nodes

This patch is part of the task to integrate expression rewrites into
the analysis process to avoid the need for two analysis passes.

The analyzer provides a rewrite engine which traverses the
expression tree and compares each node against a list of rewrite rules,
typically by performing an "instanceof" comparison of the node with the
target node type.

This project seeks to apply the rules as we analyze each node. The
simplest way to do so is for each node to hold the code for its own
rewrites, callable via a virtual method: rewrite().

This patch is a first step: the rewrite code moves from the various
rewrite rules into the expression node that is to be rewritten. In some
cases nodes have a single rule which is moved from the rewrite rule
directly into the rewrite() method.

In other cases, a node has multiple rules. The different rules for the
same node are moved into distinct methods. Though these methods are
all called by the main rewrite() method, nothing calls that rewrite()
method in this patch.

Finally, this patch introduces a "stub" expression analyzer. In a later
patch this class will drive the combined analysis/rewrite logic. For
now, it is mostly a placeholder.

The rewrite rules themselves are now just stubs retained to minimze
change. A future patch will retire them once the analysis/rewrite
integration is complete.

This patch is designed to have no functional change: code merely
moves from one location (the rewrite rules) to another (the expression
nodes). Some "shim" code is added, but the functionality is unchanged.

Tests: reran all FE tests to verify no change in behavior.

Change-Id: Ifda7082abea4f6448e6cfef0d9ef7b8565d73fce
---
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/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
A fe/src/main/java/org/apache/impala/analysis/ExprAnalyzer.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/common/TreeNode.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/ExprRewriter.java
M fe/src/main/java/org/apache/impala/rewrite/ExtractCommonConjunctRule.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/SimplifyConditionalsRule.java
M fe/src/main/java/org/apache/impala/rewrite/SimplifyDistinctFromRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
18 files changed, 686 insertions(+), 378 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/12143/3
-- 
To view, visit http://gerrit.cloudera.org:8080/12143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifda7082abea4f6448e6cfef0d9ef7b8565d73fce
Gerrit-Change-Number: 12143
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-8041, Part 1: Move rewrite rules into expr nodes

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12143 )

Change subject: IMPALA-8041, Part 1: Move rewrite rules into expr nodes
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1874/ : 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/12143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifda7082abea4f6448e6cfef0d9ef7b8565d73fce
Gerrit-Change-Number: 12143
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 24 Jan 2019 04:43:20 +0000
Gerrit-HasComments: No