[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
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 Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Wed, 05 Dec 2018 21:15:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
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 Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Wed, 05 Dec 2018 20:32:07 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
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 Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
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 Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Wed, 05 Dec 2018 16:26:34 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
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 Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Tue, 04 Dec 2018 02:08:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
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 Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Tue, 04 Dec 2018 01:35:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
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 Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
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 Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 04 Dec 2018 00:02:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12017 ) 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/1491/ : 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/12017 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I39dc783ee1535da5fad8ee2f2dd5f8ca17004bbc Gerrit-Change-Number: 12017 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 01 Dec 2018 00:14:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 01 Dec 2018 00:12:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
Paul Rogers has abandoned this change. ( http://gerrit.cloudera.org:8080/12017 ) Change subject: IMPALA-7914: Base interface for statement-like AST nodes .. Abandoned Wrong branch -- To view, visit http://gerrit.cloudera.org:8080/12017 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I39dc783ee1535da5fad8ee2f2dd5f8ca17004bbc Gerrit-Change-Number: 12017 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
Paul Rogers has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12017 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: I39dc783ee1535da5fad8ee2f2dd5f8ca17004bbc --- 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 A fe/src/main/java/org/apache/impala/analysis/Parser.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/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 24 files changed, 142 insertions(+), 80 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/17/12017/1 -- To view, visit http://gerrit.cloudera.org:8080/12017 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I39dc783ee1535da5fad8ee2f2dd5f8ca17004bbc Gerrit-Change-Number: 12017 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Rogers
[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/12017 )
Change subject: IMPALA-7914: Base interface for statement-like AST nodes
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12017/1/fe/src/main/java/org/apache/impala/analysis/Parser.java
File fe/src/main/java/org/apache/impala/analysis/Parser.java:
http://gerrit.cloudera.org:8080/#/c/12017/1/fe/src/main/java/org/apache/impala/analysis/Parser.java@41
PS1, Line 41: public static StatementBase parse(String query, TQueryOptions
options) throws ParseException {
line too long (96 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/12017
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I39dc783ee1535da5fad8ee2f2dd5f8ca17004bbc
Gerrit-Change-Number: 12017
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 30 Nov 2018 23:50:28 +
Gerrit-HasComments: Yes
