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/30 23:49:37 UTC
[Impala-ASF-CR] Fix
Paul Rogers has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12018
Change subject: Fix
......................................................................
Fix
Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
---
D fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
M fe/src/main/java/org/apache/impala/catalog/View.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
8 files changed, 62 insertions(+), 91 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/12018/1
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
[Impala-ASF-CR] IMPALA-7914: Base class for statement-like AST nodes
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12018 )
Change subject: IMPALA-7914: Base class for statement-like AST nodes
......................................................................
Patch Set 4:
Turns out a base class is even more useful than a base interface.
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Wed, 05 Dec 2018 23:05:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST 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/12018 )
Change subject: IMPALA-7914: Base interface for statement-like AST nodes
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1492/ : 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/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sat, 01 Dec 2018 00:12:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7914: Base class for statement-like AST nodes
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/12018 )
Change subject: IMPALA-7914: Base class for statement-like AST nodes
......................................................................
IMPALA-7914: Base class for statement-like AST nodes
In order to integrate expression rewrites into the analysis phase, the
expression analyze() operation must be able to replace one expression
node with another. Statements, however, are analyzed in place. The two
types of parse nodes thus need different analyze() semantics.
To prepare for that goal, this patch introduces a new StmtNode class
as the base for all statement-like AST nodes. The existing analyze()
method moves to StmtNode. While Expr still defines this method for now,
the future goal is to change the Expr analyze() semantics.
Tests: This is purely a code restructuring, no functional changes. Reran
all FE tests.
Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Reviewed-on: http://gerrit.cloudera.org:8080/12018
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/FunctionArgs.java
M fe/src/main/java/org/apache/impala/analysis/HdfsCachingOp.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionParam.java
M fe/src/main/java/org/apache/impala/analysis/ParseNode.java
M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
A fe/src/main/java/org/apache/impala/analysis/StmtNode.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/TableSampleClause.java
M fe/src/main/java/org/apache/impala/analysis/TypeDef.java
M fe/src/main/java/org/apache/impala/analysis/WithClause.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
18 files changed, 72 insertions(+), 21 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 8
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST 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/12018 )
Change subject: IMPALA-7914: Base interface for statement-like AST nodes
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1525/ : 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/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Tue, 04 Dec 2018 02:08:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12018 )
Change subject: IMPALA-7914: Base interface for statement-like AST nodes
......................................................................
Patch Set 3:
(1 comment)
Bharath, thanks for the good documentation suggestion; went ahead and made the change.
http://gerrit.cloudera.org:8080/#/c/12018/3/fe/src/main/java/org/apache/impala/analysis/StmtNode.java
File fe/src/main/java/org/apache/impala/analysis/StmtNode.java:
http://gerrit.cloudera.org:8080/#/c/12018/3/fe/src/main/java/org/apache/impala/analysis/StmtNode.java@25
PS3, Line 25: * Parse nodes divide into two broad categories: statement-like nodes (derived
: * from this interface) and expression nodes (which derive from Expr.)
: *
: * Statement-like nodes form the structure of a query and mostly retain their
: * structure during analysis. That is, they are "analyzed in place." By contrast,
: * expressions are often rewritten so that the final expression out of the analyzer
: * may be different than the expression received from the parser. As a result, the
: * analyze interface differs between the two categories.
: *
: * Error reporting often wants to emit the user's original SQL expression before
: * rewrites. Statements hold onto both the original and rewritten expressions.
: * Expressions, by contrast don't know if they are original or rewritten.
> I think this overview makes more sense in the parent ParseNode class?
Done
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Wed, 05 Dec 2018 20:32:07 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7914: Base class for statement-like AST 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/12018 )
Change subject: IMPALA-7914: Base class for statement-like AST nodes
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3546/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 6
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Mon, 10 Dec 2018 19:43:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7914: Base class for statement-like AST 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/12018
to look at the new patch set (#5).
Change subject: IMPALA-7914: Base class for statement-like AST nodes
......................................................................
IMPALA-7914: Base class for statement-like AST nodes
In order to integrate expression rewrites into the analysis phase, the
expression analyze() operation must be able to replace one expression
node with another. Statements, however, are analyzed in place. The two
types of parse nodes thus need different analyze() semantics.
To prepare for that goal, this patch introduces a new StmtNode class
as the base for all statement-like AST nodes. The existing analyze()
method moves to StmtNode. While Expr still defines this method for now,
the future goal is to change the Expr analyze() semantics.
Tests: This is purely a code restructuring, no functional changes. Reran
all FE tests.
Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/FunctionArgs.java
M fe/src/main/java/org/apache/impala/analysis/HdfsCachingOp.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionParam.java
M fe/src/main/java/org/apache/impala/analysis/ParseNode.java
M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
A fe/src/main/java/org/apache/impala/analysis/StmtNode.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/TableSampleClause.java
M fe/src/main/java/org/apache/impala/analysis/TypeDef.java
M fe/src/main/java/org/apache/impala/analysis/WithClause.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
18 files changed, 72 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/12018/5
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST 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/12018
to look at the new patch set (#3).
Change subject: IMPALA-7914: Base interface for statement-like AST nodes
......................................................................
IMPALA-7914: Base interface for statement-like AST nodes
In order to integrate expression rewrites into the analysis phase, the
expression analyze() operation must be able to replace one expression
node with another. Statements, however, are analyzed in place. The two
types of parse nodes thus need different analyze() semantics.
To prepare for that goal, this patch introduces a new StmtNode interface
as the base for all statement-like AST nodes. The existing analyze()
method moves to StmtNode. While Expr still defines this method for now,
the future goal is to change the Expr analyze() semantics.
Tests: This is purely a code restructuring, no functional changes. Reran
all FE tests.
Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/FunctionArgs.java
M fe/src/main/java/org/apache/impala/analysis/HdfsCachingOp.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionParam.java
M fe/src/main/java/org/apache/impala/analysis/ParseNode.java
M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
A fe/src/main/java/org/apache/impala/analysis/StmtNode.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/TableSampleClause.java
M fe/src/main/java/org/apache/impala/analysis/TypeDef.java
M fe/src/main/java/org/apache/impala/analysis/WithClause.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
18 files changed, 71 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/12018/3
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12018 )
Change subject: IMPALA-7914: Base interface for statement-like AST nodes
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12018/3/fe/src/main/java/org/apache/impala/analysis/StmtNode.java
File fe/src/main/java/org/apache/impala/analysis/StmtNode.java:
http://gerrit.cloudera.org:8080/#/c/12018/3/fe/src/main/java/org/apache/impala/analysis/StmtNode.java@25
PS3, Line 25: * Parse nodes divide into two broad categories: statement-like nodes (derived
: * from this interface) and expression nodes (which derive from Expr.)
: *
: * Statement-like nodes form the structure of a query and mostly retain their
: * structure during analysis. That is, they are "analyzed in place." By contrast,
: * expressions are often rewritten so that the final expression out of the analyzer
: * may be different than the expression received from the parser. As a result, the
: * analyze interface differs between the two categories.
: *
: * Error reporting often wants to emit the user's original SQL expression before
: * rewrites. Statements hold onto both the original and rewritten expressions.
: * Expressions, by contrast don't know if they are original or rewritten.
I think this overview makes more sense in the parent ParseNode class?
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Wed, 05 Dec 2018 16:26:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7914: Base class for statement-like AST 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/12018 )
Change subject: IMPALA-7914: Base class for statement-like AST nodes
......................................................................
Patch Set 6: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3546/
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 6
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Mon, 10 Dec 2018 23:36:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7914: Base class for statement-like AST 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/12018 )
Change subject: IMPALA-7914: Base class for statement-like AST nodes
......................................................................
Patch Set 5:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/1551/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Wed, 05 Dec 2018 23:15:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7914: Base class for statement-like AST nodes
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12018 )
Change subject: IMPALA-7914: Base class for statement-like AST nodes
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Mon, 10 Dec 2018 19:42:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST 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/12018
to look at the new patch set (#4).
Change subject: IMPALA-7914: Base interface for statement-like AST nodes
......................................................................
IMPALA-7914: Base interface for statement-like AST nodes
In order to integrate expression rewrites into the analysis phase, the
expression analyze() operation must be able to replace one expression
node with another. Statements, however, are analyzed in place. The two
types of parse nodes thus need different analyze() semantics.
To prepare for that goal, this patch introduces a new StmtNode interface
as the base for all statement-like AST nodes. The existing analyze()
method moves to StmtNode. While Expr still defines this method for now,
the future goal is to change the Expr analyze() semantics.
Tests: This is purely a code restructuring, no functional changes. Reran
all FE tests.
Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/FunctionArgs.java
M fe/src/main/java/org/apache/impala/analysis/HdfsCachingOp.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionParam.java
M fe/src/main/java/org/apache/impala/analysis/ParseNode.java
M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
A fe/src/main/java/org/apache/impala/analysis/StmtNode.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/TableSampleClause.java
M fe/src/main/java/org/apache/impala/analysis/TypeDef.java
M fe/src/main/java/org/apache/impala/analysis/WithClause.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
18 files changed, 72 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/12018/4
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST 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/12018 )
Change subject: IMPALA-7914: Base interface for statement-like AST nodes
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1548/ : 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/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Wed, 05 Dec 2018 21:15:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7914: Base class for statement-like AST 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/12018 )
Change subject: IMPALA-7914: Base class for statement-like AST nodes
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 6
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Mon, 10 Dec 2018 19:43:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7914: Base class for statement-like AST 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/12018 )
Change subject: IMPALA-7914: Base class for statement-like AST nodes
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 7
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Tue, 11 Dec 2018 05:45:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7914: Base class for statement-like AST 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/12018 )
Change subject: IMPALA-7914: Base class for statement-like AST nodes
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3548/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 7
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Tue, 11 Dec 2018 05:45:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/12018 )
Change subject: IMPALA-7914: Base interface for statement-like AST nodes
......................................................................
IMPALA-7914: Base interface for statement-like AST nodes
In order to integrate expression rewrites into the analysis phase, the
expression analyze() operation must be able to replace one expression
node with another. But, statements are analyzed in place. The result is
that statement-like nodes and expression nodes need different analyze()
semantics.
To prepare for that goal, this patch introduces a new StmtNode interface
as the base for all statement-like AST nodes. The existing analyze()
method moves to StmtNode. While Expr still defines this method for now,
the future goal is to change the Expr analyze() semantics.
Tests: This is purely a code restructuring, no functional changes. Reran
all FE tests.
Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/FunctionArgs.java
M fe/src/main/java/org/apache/impala/analysis/HdfsCachingOp.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionParam.java
M fe/src/main/java/org/apache/impala/analysis/ParseNode.java
M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
A fe/src/main/java/org/apache/impala/analysis/StmtNode.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/TableSampleClause.java
M fe/src/main/java/org/apache/impala/analysis/TypeDef.java
M fe/src/main/java/org/apache/impala/analysis/WithClause.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
18 files changed, 54 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/12018/2
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12018 )
Change subject: IMPALA-7914: Base interface for statement-like AST nodes
......................................................................
Patch Set 2:
(3 comments)
Bharath, thanks much for your review. Addressed your comments.
http://gerrit.cloudera.org:8080/#/c/12018/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12018/2//COMMIT_MSG@17
PS2, Line 17: While Expr still defines this method for now,
: the future goal is to change the Expr analyze() semantics.
> Mind adding a TODO for this?
Done
http://gerrit.cloudera.org:8080/#/c/12018/2/fe/src/main/java/org/apache/impala/analysis/StmtNode.java
File fe/src/main/java/org/apache/impala/analysis/StmtNode.java:
http://gerrit.cloudera.org:8080/#/c/12018/2/fe/src/main/java/org/apache/impala/analysis/StmtNode.java@25
PS2, Line 25: They are not subject to expression rewrites.
> This seems out of context for anyone who reads the code and don't know abou
Done
http://gerrit.cloudera.org:8080/#/c/12018/2/fe/src/main/java/org/apache/impala/analysis/StmtNode.java@24
PS2, Line 24: before-
: * of after- rewrite SQL
> typo?
Done
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Tue, 04 Dec 2018 01:35:05 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12018 )
Change subject: IMPALA-7914: Base interface for statement-like AST nodes
......................................................................
Patch Set 2: Code-Review+2
(4 comments)
Nice cleanup. Looking forward to the future changes that fix the expr-analyze() lifecycle.
http://gerrit.cloudera.org:8080/#/c/12018/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12018/2//COMMIT_MSG@17
PS2, Line 17: While Expr still defines this method for now,
: the future goal is to change the Expr analyze() semantics.
Mind adding a TODO for this?
http://gerrit.cloudera.org:8080/#/c/12018/2/fe/src/main/java/org/apache/impala/analysis/StmtNode.java
File fe/src/main/java/org/apache/impala/analysis/StmtNode.java:
http://gerrit.cloudera.org:8080/#/c/12018/2/fe/src/main/java/org/apache/impala/analysis/StmtNode.java@23
PS2, Line 23: clauses
nit: period after this.
http://gerrit.cloudera.org:8080/#/c/12018/2/fe/src/main/java/org/apache/impala/analysis/StmtNode.java@25
PS2, Line 25: They are not subject to expression rewrites.
This seems out of context for anyone who reads the code and don't know about this commit.
http://gerrit.cloudera.org:8080/#/c/12018/2/fe/src/main/java/org/apache/impala/analysis/StmtNode.java@24
PS2, Line 24: before-
: * of after- rewrite SQL
typo?
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Dec 2018 00:02:47 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7914: Base class for statement-like AST 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/12018 )
Change subject: IMPALA-7914: Base class for statement-like AST nodes
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 7
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Tue, 11 Dec 2018 09:53:42 +0000
Gerrit-HasComments: No