You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tamas Mate (Code Review)" <ge...@cloudera.org> on 2023/02/08 14:45:46 UTC
[Impala-ASF-CR] [WIP] IMPALA-11908: Parser change for Iceberg metadata querying
Tamas Mate has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19483
Change subject: [WIP] IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
[WIP] IMPALA-11908: Parser change for Iceberg metadata querying
This commit is WIP, the design decisions have been made, some cleanup is
yet to be done.
This change extends parsing table references with Iceberg metadata
tables. The TableName class has been extended with an extra vTbl field
which is filled when a virtual table reference is suspected. This
additional field helps to keep the real table in the statment table
cache next to the virtual table. Which should be loaded so Iceberg
metadat tables can be created.
Iceberg provides a rich API to query metadata, these Iceberg API tables
are accessible through the MetadataTableUtils class. Using these table
schemas it is possible to create an Impala table that can be queried
later on.
Querying a metadata table at this point is expected to throw a
NotImplementedExpception.
Testing:
- Added E2E test to test this feature.
Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
A fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/analysis/TableName.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergSchemaConverter.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/query_test/test_iceberg.py
13 files changed, 304 insertions(+), 28 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/19483/1
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate <tm...@apache.org>
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 9:
Hi everyone, thank you for the reviews so far.
This patch got two +1s so far, it is a reasonably big change, so I would like to wait for a +2. Anyone would like to and has time to go through it again?
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 9
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Mar 2023 09:52:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
lipenglin@apache.org has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 5:
(1 comment)
Thanks Tamas!
The code looks good to me.
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@a1323
PS4, Line 1323:
> I think rawPath cannot be empty at this point. The value originates from a
Thanks for explain!
But I actually tested it:
The rawPath is a List: ["functional_parquet", "iceberg_v2_test_tbl"]
tblNameIdx is index: 1
The result of "rawPath.subList(tblNameIdx + 1, rawPath.size())" is always an empty ArrayList.
I am confused that why the empty ArrayList passed as a parameter to the constructor of the org.apache.impala.analysis.Path.
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 24 Feb 2023 03:05:19 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12343/ : 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/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Feb 2023 10:53:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Feb 2023 16:12:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 10:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19483/10/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/19483/10/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3340
PS10, Line 3340: paramter
> Nit: typo.
Done
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Mar 2023 13:59:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12599/ : 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/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Mar 2023 16:16:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 6:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3341
PS6, Line 3341: addMetadataVirtualTable
nit: I feel that the name should indicate that we don't add a metadata table in every case. On the callsite I had the impression that a metadata table is added regardless we refer to a metadata table or not.
Name proposal: tryAddingMetadataTable(...)
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3342
PS6, Line 3342: if (IcebergMetadataTable.isIcebergMetadataTable(tblRefPath)) {
nit: ususally my preference is exiting early if some conditions are not met instead of putting the whole body of a function inside an if. This reduces deep indentations.
if (not metadata table) return;
...
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
File fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java:
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java@37
PS6, Line 37: (IcebergMetadataTable)resolvedPath.getRootTable();
Precondition to verify this conversion will work well?
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/analysis/TableName.java
File fe/src/main/java/org/apache/impala/analysis/TableName.java:
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/analysis/TableName.java@46
PS6, Line 46: public TableName(String db, String tbl) {
This constructor could call the other one like:
this(db, tbl, null) and then we can avoid code duplication between these constructors.
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/analysis/TableName.java@52
PS6, Line 52: this.vTbl_ = "";
In Path.getCandidateTables() you pass null for vTbl_ if the table is not virtual but in this constructor you set it to empty string. To reach consistency I think we should pick one of those 2 approaches. I'd vote for setting it null if the table is not virtual.
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/analysis/TableName.java@126
PS6, Line 126: if (!vTbl_.isEmpty()) {
This could cause a NPE when vTbl_ is null.
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java:
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@47
PS6, Line 47: tblRefPath
If I'm not mistaken this path is only used to get the name of the metadata table. Would it make sense then to pass the metadata table name into this constructor instead of the path?
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@51
PS6, Line 51: get
I miss a few Precondition checks on the inputs here, e.g. to verify that 'baseTable' is an instance of FeIcebergTable, or to check the number of items in 'tblRefPath' before we access the items.
I guess it;s expected to call isIcebergMetadataTable() on the parameter at the callsite but you don't have any garantee that any future implementaton will also call it.
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@51
PS6, Line 51: metadataTableTypeString
No need to introduce a variable for this. You can use 'tblRefPath.get(2)' in the below line.
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@52
PS6, Line 52: MetadataTableType.valueOf
I think MetadataTableType.from() is the preferred way to do the conversion from String that is a wrapper around MetadataTableType.valueOf(). It also takes care of the IllegalArgumentException that you didn't do here. You just have to check for null result.
http://gerrit.cloudera.org:8080/#/c/19483/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test:
http://gerrit.cloudera.org:8080/#/c/19483/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@1
PS6, Line 1: ====
Would it make sense to include all the metadata table types here not just 'history' and 'manifests'?
http://gerrit.cloudera.org:8080/#/c/19483/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@2
PS6, Line 2: ---- QUERY
There are some metadata tables that have nested type columns, e.g. partitions.partition is a struct. Could you check if you can reference the members of these struct columns in the select list?
Also, manifests.partition_summaries seems nested type too. Would be nice to exercise it a little bit early to see if we have any gaps with these types.
http://gerrit.cloudera.org:8080/#/c/19483/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@26
PS6, Line 26: join
I understand this test is only for checking the join syntax, but for later usage I think we could have a more meaningful join here instead of joining a table to itself. E.g. join history with snapshots?
http://gerrit.cloudera.org:8080/#/c/19483/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@40
PS6, Line 40: ====
I just bumped into another use case wrt metadata table: you can also make 'time travel' with metadata tables. E.g. this is valid in Spark:
SELECT * from db.tbl.files VERSION AS OF <snapshot_id>
We might want to put this into a follow-up patch, or include here, it's up to you.
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Mar 2023 11:11:02 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 9: Code-Review+1
(2 comments)
Just a minor comment. I can +2 once that's covered
http://gerrit.cloudera.org:8080/#/c/19483/9/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test:
http://gerrit.cloudera.org:8080/#/c/19483/9/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@11
PS9, Line 11: # 'Files' is a keyword and need to be escaped
Good to know about this limitation :)
http://gerrit.cloudera.org:8080/#/c/19483/9/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@119
PS9, Line 119: select * from functional_parquet.iceberg_alltypes_part_orc.manifests a, a.partition_summaries
Have you tried out if we can refer to the ITEM and POS pseudo columns of this array?
Anyway, one more thing worth covering with a test is that now we can include arrays into the select list without doing that join think in the from clause.
E.g.: SELECT partition_summaries FROM db.tbl.manifests;
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 9
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Mar 2023 12:17:22 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3341
PS6, Line 3341: addMetadataVirtualTable
> I can't see that the name changed, did you change it?
Thanks for catching this. I did not change it, I added a new condition to the Analyzer, I should have made this a precondition.
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 10 Mar 2023 14:47:05 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
IMPALA-11908: Parser change for Iceberg metadata querying
This change extends parsing table references with Iceberg metadata
tables. The TableName class has been extended with an extra vTbl field
which is filled when a virtual table reference is suspected. This
additional field helps to keep the real table in the statment table
cache next to the virtual table. Which should be loaded so Iceberg
metadat tables can be created.
Iceberg provides a rich API to query metadata, these Iceberg API tables
are accessible through the MetadataTableUtils class. Using these table
schemas it is possible to create an Impala table that can be queried
later on.
Querying a metadata table at this point is expected to throw a
NotImplementedExpception.
Testing:
- Added E2E test to test it for some tables.
Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
A fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/analysis/TableName.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergSchemaConverter.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/query_test/test_iceberg.py
14 files changed, 312 insertions(+), 28 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/19483/4
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12680/ : 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/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 9
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Mar 2023 10:10:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
IMPALA-11908: Parser change for Iceberg metadata querying
This change extends parsing table references with Iceberg metadata
tables. The TableName class has been extended with an extra vTbl field
which is filled when a virtual table reference is suspected. This
additional field helps to keep the real table in the statement table
cache next to the virtual table, which should be loaded so Iceberg
metadata tables can be created.
Iceberg provides a rich API to query metadata, these Iceberg API tables
are accessible through the MetadataTableUtils class. Using these table
schemas it is possible to create an Impala table that can be queried
later on.
Querying a metadata table at this point is expected to throw a
NotImplementedException.
Testing:
- Added E2E test to test it for some tables.
Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
A fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/analysis/TableName.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergSchemaConverter.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/query_test/test_iceberg.py
14 files changed, 423 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/19483/10
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 12:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9184/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 12
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Mar 2023 15:35:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 11: Code-Review+2
Great works! Thanks, Tamas!
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 11
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Mar 2023 14:07:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/analysis/TableName.java
File fe/src/main/java/org/apache/impala/analysis/TableName.java:
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/analysis/TableName.java@59
PS6, Line 59: Preconditions.checkNotNull(tbl);
Is it allowed to be empty?
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java:
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@51
PS6, Line 51: metadataTableTypeString
> No need to introduce a variable for this. You can use 'tblRefPath.get(2)' i
On the other hand, putting it into a variable gives it a name which makes it easier to understand.
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@2215
PS6, Line 2215: t
Nit: extra t.
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Mar 2023 14:13:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
IMPALA-11908: Parser change for Iceberg metadata querying
This change extends parsing table references with Iceberg metadata
tables. The TableName class has been extended with an extra vTbl field
which is filled when a virtual table reference is suspected. This
additional field helps to keep the real table in the statment table
cache next to the virtual table. Which should be loaded so Iceberg
metadat tables can be created.
Iceberg provides a rich API to query metadata, these Iceberg API tables
are accessible through the MetadataTableUtils class. Using these table
schemas it is possible to create an Impala table that can be queried
later on.
Querying a metadata table at this point is expected to throw a
NotImplementedExpception.
Testing:
- Added E2E test to test it for some tables.
Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
A fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/analysis/TableName.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergSchemaConverter.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/query_test/test_iceberg.py
14 files changed, 311 insertions(+), 28 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/19483/3
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 3:
Finished cleaning up, I have already tested it with a verify job, but will start another one just to be sure. LMK your thoughts.
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Feb 2023 10:34:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
IMPALA-11908: Parser change for Iceberg metadata querying
This change extends parsing table references with Iceberg metadata
tables. The TableName class has been extended with an extra vTbl field
which is filled when a virtual table reference is suspected. This
additional field helps to keep the real table in the statment table
cache next to the virtual table, which should be loaded so Iceberg
metadata tables can be created.
Iceberg provides a rich API to query metadata, these Iceberg API tables
are accessible through the MetadataTableUtils class. Using these table
schemas it is possible to create an Impala table that can be queried
later on.
Querying a metadata table at this point is expected to throw a
NotImplementedExpception.
Testing:
- Added E2E test to test it for some tables.
Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
A fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/analysis/TableName.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergSchemaConverter.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/query_test/test_iceberg.py
14 files changed, 329 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/19483/5
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Hello Daniel Becker, Gabor Kaszab, Zoltan Borok-Nagy, lipenglin@apache.org, Gergely Fürnstáhl, Noemi Pap-Takacs, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19483
to look at the new patch set (#11).
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
IMPALA-11908: Parser change for Iceberg metadata querying
This change extends parsing table references with Iceberg metadata
tables. The TableName class has been extended with an extra vTbl field
which is filled when a virtual table reference is suspected. This
additional field helps to keep the real table in the statement table
cache next to the virtual table, which should be loaded so Iceberg
metadata tables can be created.
Iceberg provides a rich API to query metadata, these Iceberg API tables
are accessible through the MetadataTableUtils class. Using these table
schemas it is possible to create an Impala table that can be queried
later on.
Querying a metadata table at this point is expected to throw a
NotImplementedException.
Testing:
- Added E2E test to test it for some tables.
Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
A fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/analysis/TableName.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergSchemaConverter.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/query_test/test_iceberg.py
14 files changed, 423 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/19483/11
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 11
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 10:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12706/ : 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/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Mar 2023 13:36:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 12: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 12
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Mar 2023 15:35:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 10:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19483/10/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/19483/10/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3340
PS10, Line 3340: paramter
Nit: typo.
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Mar 2023 13:46:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3341
PS6, Line 3341: addMetadataVirtualTable
> Thanks for catching this. I did not change it, I added a new condition to t
You could mention in the doc comment that tblRefPath is expected to be an Iceberg metadata table.
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 9
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Mar 2023 09:55:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
IMPALA-11908: Parser change for Iceberg metadata querying
This change extends parsing table references with Iceberg metadata
tables. The TableName class has been extended with an extra vTbl field
which is filled when a virtual table reference is suspected. This
additional field helps to keep the real table in the statement table
cache next to the virtual table, which should be loaded so Iceberg
metadata tables can be created.
Iceberg provides a rich API to query metadata, these Iceberg API tables
are accessible through the MetadataTableUtils class. Using these table
schemas it is possible to create an Impala table that can be queried
later on.
Querying a metadata table at this point is expected to throw a
NotImplementedException.
Testing:
- Added E2E test to test it for some tables.
Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Reviewed-on: http://gerrit.cloudera.org:8080/19483
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/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
A fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/analysis/TableName.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergSchemaConverter.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/query_test/test_iceberg.py
14 files changed, 423 insertions(+), 36 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 13
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 4:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/19483/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19483/4//COMMIT_MSG@13
PS4, Line 13: .
Nit: comma?
http://gerrit.cloudera.org:8080/#/c/19483/4//COMMIT_MSG@14
PS4, Line 14: metadat
Nit: metadata.
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3289
PS4, Line 3289: database
'dbName' is no longer a parameter of this function.
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3337
PS4, Line 3337: * Adds a new metadata table to the stmt table cache. At this point it is unknown if
Could add that this is for Iceberg only (at least now).
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3342
PS4, Line 3342: MetaVirtual
MetadataVirtualTable?
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3349
PS4, Line 3349: virtualTableName
It's a bit confusing that we get 'originalTable' from 'virtualTableName' and not for example 'originalTableName'.
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/TableName.java
File fe/src/main/java/org/apache/impala/analysis/TableName.java:
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/TableName.java@42
PS4, Line 42: vTbl_
Could add a comment describing what 'vTbl_' is used for.
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/TableName.java@55
PS4, Line 55: Preconditions.checkNotNull(db);
In the other constructor on L44, we check that
db == null || !db.isEmpty()
I guess we don't allow 'db' to be null because of what is written on L82, but do we allow 'db.isEmpty()'?
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@36
PS4, Line 36: import org.apache.impala.catalog.HdfsPartition.FileDescriptor;
: import org.apache.impala.common.Pair;
Unused imports.
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java:
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@95
PS4, Line 95: Boolean
Can't we return a primitive 'boolean'?
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@2216
PS4, Line 2216: that are
Nit: "which is currently not supported" would be better.
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Feb 2023 12:29:38 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12373/ : 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/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 14 Feb 2023 13:40:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
IMPALA-11908: Parser change for Iceberg metadata querying
This change extends parsing table references with Iceberg metadata
tables. The TableName class has been extended with an extra vTbl field
which is filled when a virtual table reference is suspected. This
additional field helps to keep the real table in the statement table
cache next to the virtual table, which should be loaded so Iceberg
metadata tables can be created.
Iceberg provides a rich API to query metadata, these Iceberg API tables
are accessible through the MetadataTableUtils class. Using these table
schemas it is possible to create an Impala table that can be queried
later on.
Querying a metadata table at this point is expected to throw a
NotImplementedException.
Testing:
- Added E2E test to test it for some tables.
Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
A fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/analysis/TableName.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergSchemaConverter.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/query_test/test_iceberg.py
14 files changed, 330 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/19483/6
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
lipenglin@apache.org has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 6: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@a1323
PS4, Line 1323:
> Thanks for describing it further. Sorry, I thought you were asking about ra
I got it. I ignore that because I didn't use nested types very often.
Thanks again Tamas!
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 28 Feb 2023 08:28:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12457/ : 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/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Feb 2023 13:39:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 11:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12707/ : 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/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 11
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Mar 2023 14:19:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 12: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 12
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Mar 2023 20:53:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: [WIP] IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/19483/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/19483/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@414
PS1, Line 414: for (Column col : IcebergSchemaConverter.convertToImpalaSchema(getIcebergSchema())) {
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/19483/1/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java:
http://gerrit.cloudera.org:8080/#/c/19483/1/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@55
PS1, Line 55: for (Column col : IcebergSchemaConverter.convertToImpalaSchema(metadataTableSchema)) {
line too long (92 > 90)
http://gerrit.cloudera.org:8080/#/c/19483/1/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/19483/1/tests/query_test/test_iceberg.py@1006
PS1, Line 1006:
flake8: W292 no newline at end of file
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Feb 2023 14:46:39 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12344/ : 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/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Feb 2023 10:57:46 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9039/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Feb 2023 11:00:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 6:
(8 comments)
Thanks all for the review, updated the patch.
http://gerrit.cloudera.org:8080/#/c/19483/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19483/5//COMMIT_MSG@12
PS5, Line 12: statemen
> typo: statement
Done
http://gerrit.cloudera.org:8080/#/c/19483/5//COMMIT_MSG@22
PS5, Line 22: NotImplementedException.
> typo: NotImplementedException
Done
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@a1323
PS4, Line 1323:
> Thanks for explain!
Thanks for describing it further. Sorry, I thought you were asking about rawPath of this method.
This subList can contain values when querying nested types, for example: describe functional_parquet.allcomplextypes.map_map_col;
In this case the new Path object will have map_map_col as rawPath.
http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@876
PS5, Line 876: if (table instanceof IcebergMetadataTable) {
: return new IcebergMetadataTableRef(tableRef, resolvedPath);
> nit: multi-line if statements should use brackets
Done
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/TableName.java
File fe/src/main/java/org/apache/impala/analysis/TableName.java:
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/TableName.java@55
PS4, Line 55: public TableName(String db, String tbl, String vTbl) {
> If none should be null or empty, shouldn't the condition be
Indeed, this broke a few things, updated these conditions.
http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@417
PS5, Line 417: ,
> We could also pass 'e' as the second parameter so we'd get more context whe
Done
http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java:
http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@39
PS5, Line 39: metadata
> typo: metadata
Done
http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@2215
PS5, Line 2215: metadtata
> typo: metadata, and probably write "metadata table"
Done
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Feb 2023 13:19:37 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 6:
(2 comments)
Thank you for the reviews, updated the patch.
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/19483/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3341
PS6, Line 3341: addMetadataVirtualTable
> You could mention in the doc comment that tblRefPath is expected to be an I
Done
http://gerrit.cloudera.org:8080/#/c/19483/9/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test:
http://gerrit.cloudera.org:8080/#/c/19483/9/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@119
PS9, Line 119:
> Have you tried out if we can refer to the ITEM and POS pseudo columns of th
Added 'pos' for this test case.
Added another test case with this exploding join functionality.
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Mar 2023 13:16:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 5:
(1 comment)
Thanks Tamás!
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/TableName.java
File fe/src/main/java/org/apache/impala/analysis/TableName.java:
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/TableName.java@55
PS4, Line 55: public TableName(String db, String tbl, String vTbl) {
> Nice catch, yes, none of these should be null or empty.
If none should be null or empty, shouldn't the condition be
db != null && !db.isEmpty()
and the same for tbl and vTbl?
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 21 Feb 2023 10:31:57 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
IMPALA-11908: Parser change for Iceberg metadata querying
This change extends parsing table references with Iceberg metadata
tables. The TableName class has been extended with an extra vTbl field
which is filled when a virtual table reference is suspected. This
additional field helps to keep the real table in the statement table
cache next to the virtual table, which should be loaded so Iceberg
metadata tables can be created.
Iceberg provides a rich API to query metadata, these Iceberg API tables
are accessible through the MetadataTableUtils class. Using these table
schemas it is possible to create an Impala table that can be queried
later on.
Querying a metadata table at this point is expected to throw a
NotImplementedException.
Testing:
- Added E2E test to test it for some tables.
Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
A fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/analysis/TableName.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergSchemaConverter.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/query_test/test_iceberg.py
14 files changed, 414 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/19483/9
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 9
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
lipenglin@apache.org has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 4:
(4 comments)
Some minor comments.
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@a1323
PS4, Line 1323:
Just a question:
Is it always an empty List?
For org.apache.impala.analysis.Path#Path(org.apache.impala.analysis.TupleDescriptor, java.util.List<java.lang.String> rawPath),
In what case is the parameter rawPath not an empty List?
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1324
PS4, Line 1324: IcebergTimeTravelTable timeTravelTable =
: new IcebergTimeTravelTable(rootTable, timeTravelSpec);
: tbl = timeTravelTable;
'timeTravelTable' is redundant, maybe we can fix it by the way.
"tbl = new IcebergTimeTravelTable(rootTable, timeTravelSpec);"
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3339
PS4, Line 3339: orinal
nit:
original?
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
File fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java:
http://gerrit.cloudera.org:8080/#/c/19483/4/fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java@37
PS4, Line 37: (
nit:
redundancy parentheses.
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 13 Feb 2023 11:08:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: [WIP] IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12340/ : 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/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Feb 2023 15:06:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19483/3/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/19483/3/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@2216
PS3, Line 2216: + "table that are not currently supported.", String.join(".", tblRef.getPath())));
line too long (92 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Feb 2023 10:34:20 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 5:
(6 comments)
Did an initial look, found some minor issues, but overall looks good!
http://gerrit.cloudera.org:8080/#/c/19483/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19483/5//COMMIT_MSG@12
PS5, Line 12: statment
typo: statement
http://gerrit.cloudera.org:8080/#/c/19483/5//COMMIT_MSG@22
PS5, Line 22: NotImplementedExpception
typo: NotImplementedException
http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@876
PS5, Line 876: if (table instanceof IcebergMetadataTable)
: return new IcebergMetadataTableRef(tableRef, resolvedPath);
nit: multi-line if statements should use brackets
http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@417
PS5, Line 417: );
We could also pass 'e' as the second parameter so we'd get more context when an exception is thrown.
http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java:
http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@39
PS5, Line 39: metadtata
typo: metadata
http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/19483/5/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@2215
PS5, Line 2215: metadtata
typo: metadata, and probably write "metadata table"
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Feb 2023 16:58:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
Patch Set 11: Code-Review+1
Thanks Tamas, let's wait for the +2 from Gábor.
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 11
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Mar 2023 14:04:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/19483 )
Change subject: IMPALA-11908: Parser change for Iceberg metadata querying
......................................................................
IMPALA-11908: Parser change for Iceberg metadata querying
This change extends parsing table references with Iceberg metadata
tables. The TableName class has been extended with an extra vTbl field
which is filled when a virtual table reference is suspected. This
additional field helps to keep the real table in the statement table
cache next to the virtual table, which should be loaded so Iceberg
metadata tables can be created.
Iceberg provides a rich API to query metadata, these Iceberg API tables
are accessible through the MetadataTableUtils class. Using these table
schemas it is possible to create an Impala table that can be queried
later on.
Querying a metadata table at this point is expected to throw a
NotImplementedException.
Testing:
- Added E2E test to test it for some tables.
Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
A fe/src/main/java/org/apache/impala/analysis/IcebergMetadataTableRef.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
M fe/src/main/java/org/apache/impala/analysis/TableName.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergSchemaConverter.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
M tests/query_test/test_iceberg.py
14 files changed, 415 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/19483/7
--
To view, visit http://gerrit.cloudera.org:8080/19483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0b5db884b5f3fecbd132fcb2c2cbd6c622ff965b
Gerrit-Change-Number: 19483
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>