[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Tested-by: Impala Public Jenkins --- 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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 29 Mar 2023 20:53:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 29 Mar 2023 15:35:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 29 Mar 2023 15:35:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 29 Mar 2023 14:19:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 29 Mar 2023 14:07:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 29 Mar 2023 14:04:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
Hello Daniel Becker, Gabor Kaszab, Zoltan Borok-Nagy, lipeng...@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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 29 Mar 2023 13:59:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 29 Mar 2023 13:46:13 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 29 Mar 2023 13:36:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 29 Mar 2023 13:16:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 27 Mar 2023 12:17:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 23 Mar 2023 10:10:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 23 Mar 2023 09:55:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 23 Mar 2023 09:52:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 10 Mar 2023 14:47:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 09 Mar 2023 16:16:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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/19
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
lipeng...@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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 28 Feb 2023 08:28:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 27 Feb 2023 13:39:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 27 Feb 2023 13:19:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
lipeng...@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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 24 Feb 2023 03:05:19 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 23 Feb 2023 16:58:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 21 Feb 2023 10:31:57 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 14 Feb 2023 13:40:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
lipeng...@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 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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 13 Feb 2023 11:08:03 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 09 Feb 2023 16:12:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 09 Feb 2023 12:29:38 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 09 Feb 2023 11:00:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 09 Feb 2023 10:57:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 09 Feb 2023 10:53:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 09 Feb 2023 10:34:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11908: Parser change for Iceberg metadata querying
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 Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Zoltan Borok-Nagy