You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Gergely Fürnstáhl (Code Review)" <ge...@cloudera.org> on 2022/07/18 14:32:07 UTC
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Gergely Fürnstáhl has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18743
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
IMPALA-11268: Allow STORED BY and STORED AS as well
Extended parser to support 'stored by' keyword for storage engines,
namely Kudu and Iceberg at the moment.
Testing:
Added front-end Parser and Analyzer tests and query tests for table
creation.
Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
---
M fe/src/main/cup/sql-parser.cup
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
5 files changed, 49 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/18743/4
--
To view, visit http://gerrit.cloudera.org:8080/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 4
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
Patch Set 11: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 11
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Jul 2022 15:40:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18743/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java:
http://gerrit.cloudera.org:8080/#/c/18743/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@2316
PS7, Line 2316: STORED AS
Did you mean to add also STORED AS examples here? Or examples with both STORED BY and STORED AS? If not, I think the comment would be better understandable if it was "Allow STORED BY as well" without mentioning STORED AS.
--
To view, visit http://gerrit.cloudera.org:8080/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 7
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Jul 2022 11:14:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Posted by "Gergely Fürnstáhl (Code Review)" <ge...@cloudera.org>.
Gergely Fürnstáhl has posted comments on this change. ( http://gerrit.cloudera.org:8080/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18743/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java:
http://gerrit.cloudera.org:8080/#/c/18743/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@2316
PS7, Line 2316: STORED AS
> Did you mean to add also STORED AS examples here? Or examples with both STO
I just copied the Jira task title here but I agree it's a bit confusing in this context.
For the task it makes sense, as we introduce stored by iceberg while keeping stored as iceberg as well for backward compatibility
--
To view, visit http://gerrit.cloudera.org:8080/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 7
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Jul 2022 11:22:40 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
Patch Set 12: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 12
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Jul 2022 18:21:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
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/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
IMPALA-11268: Allow STORED BY and STORED AS as well
Extended the parser to support 'stored by' keyword for storage
engines, namely Kudu and Iceberg at the moment, to match hive's
syntax. Furthermore, this lays the ground work for seperating
storage engines from file formats to be able specify both with
"STORED BY ... STORED AS ...".
Testing:
Added front-end Parser and Analyzer tests and query tests for table
creation.
Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Reviewed-on: http://gerrit.cloudera.org:8080/18743
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/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
5 files changed, 52 insertions(+), 0 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 13
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Posted by "Gergely Fürnstáhl (Code Review)" <ge...@cloudera.org>.
Gergely Fürnstáhl has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
IMPALA-11268: Allow STORED BY and STORED AS as well
Extended the parser to support 'stored by' keyword for storage
engines, namely Kudu and Iceberg at the moment, to match hive's
syntax. Furthermore, this lays the ground work for seperating
storage engines from file formats to be able specify both with
"STORED BY ... STORED AS ...".
Testing:
Added front-end Parser and Analyzer tests and query tests for table
creation.
Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
---
M fe/src/main/cup/sql-parser.cup
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
5 files changed, 52 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/18743/11
--
To view, visit http://gerrit.cloudera.org:8080/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 11
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Posted by "Gergely Fürnstáhl (Code Review)" <ge...@cloudera.org>.
Gergely Fürnstáhl has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
IMPALA-11268: Allow STORED BY and STORED AS as well
Extended the parser to support 'stored by' keyword for storage
engines, namely Kudu and Iceberg at the moment, to match hive's
syntax. Furthermore, this lays the ground work for seperating
storage engines from file formats to be able specify both with
"STORED BY ... STORED AS ...".
Testing:
Added front-end Parser and Analyzer tests and query tests for table
creation.
Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
---
M fe/src/main/cup/sql-parser.cup
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
5 files changed, 52 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/18743/8
--
To view, visit http://gerrit.cloudera.org:8080/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 8
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
Patch Set 12: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 12
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Jul 2022 13:31:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
Patch Set 12:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8367/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 12
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Jul 2022 13:31:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11030/ : 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/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 7
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Jul 2022 11:20:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11032/ : 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/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 8
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Jul 2022 11:38:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18743/8/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java:
http://gerrit.cloudera.org:8080/#/c/18743/8/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@2321
PS8, Line 2321: AnalyzesOk("create table t primary key (id) stored by iceberg as select id, bool_col," +
line too long (92 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 8
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Jul 2022 11:15:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
Patch Set 11: Code-Review+2
Thanks for the changes Gergo!
--
To view, visit http://gerrit.cloudera.org:8080/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 11
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Jul 2022 13:31:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
Patch Set 4:
(3 comments)
Thank you for working on this!
The change looks good, added some minor comments.
http://gerrit.cloudera.org:8080/#/c/18743/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18743/4//COMMIT_MSG@10
PS4, Line 10: namely Kudu and Iceberg at the moment.
Could you add some explanation of intent here as well? About the ground work to support specifying file formats for storage engines.
http://gerrit.cloudera.org:8080/#/c/18743/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java:
http://gerrit.cloudera.org:8080/#/c/18743/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@2318
PS4, Line 2318: +
nit: in this file the plus signs are in the previous line
http://gerrit.cloudera.org:8080/#/c/18743/4/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:
http://gerrit.cloudera.org:8080/#/c/18743/4/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@2854
PS4, Line 2854: STORED TEXTFILE
Could you add some negative tests as well, like this test.
--
To view, visit http://gerrit.cloudera.org:8080/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 4
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Jul 2022 09:21:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/10986/ : 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/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 4
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Jul 2022 14:47:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
Patch Set 11:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11033/ : 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/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 11
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Jul 2022 11:47:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Posted by "Gergely Fürnstáhl (Code Review)" <ge...@cloudera.org>.
Gergely Fürnstáhl has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
IMPALA-11268: Allow STORED BY and STORED AS as well
Extended the parser to support 'stored by' keyword for storage
engines, namely Kudu and Iceberg at the moment, to match hive's
syntax. Furthermore, this lays the ground work for seperating
storage engines from file formats to be able specify both with
"STORED BY ... STORED AS ...".
Testing:
Added front-end Parser and Analyzer tests and query tests for table
creation.
Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
---
M fe/src/main/cup/sql-parser.cup
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-create.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
5 files changed, 52 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/18743/7
--
To view, visit http://gerrit.cloudera.org:8080/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 7
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11268: Allow STORED BY and STORED AS as well
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18743 )
Change subject: IMPALA-11268: Allow STORED BY and STORED AS as well
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18743/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java:
http://gerrit.cloudera.org:8080/#/c/18743/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@2321
PS7, Line 2321: AnalyzesOk("create table t primary key (id) stored by iceberg as select id, bool_col," +
line too long (92 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/18743
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib677bea8e582bbc01c5fb8c81df57eb60b0ed961
Gerrit-Change-Number: 18743
Gerrit-PatchSet: 7
Gerrit-Owner: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Jul 2022 10:59:43 +0000
Gerrit-HasComments: Yes