[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/21061 ) Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables .. Patch Set 11: I have not figured out the reason behind the clang-tidy failures yet: The fail is: ``` FAILED: Unified backend test executable returned an error when trying to list tests. Command: /home/ubuntu/Impala/bin/run-binary.sh /home/ubuntu/Impala/be/build/release//service/unifiedbetests --gtest_list_tests Return Code: -11 ``` Which suggests that the unifiedbetests failed with SIGSEGV. As PS8 was fine, I narrowed down the issue to one commit: ``` commit 9b8e43e9a1ae871e676cded8e9eddc90aa96f314 IMPALA-12426: QueryStateRecord Refactor ``` Looks like this these changes are colliding, but I do not see why ATM. -- To view, visit http://gerrit.cloudera.org:8080/21061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 Gerrit-Change-Number: 21061 Gerrit-PatchSet: 11 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 01 Mar 2024 16:12:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/21061 ) Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables .. Patch Set 10: Carrying over +2 after rebase. -- To view, visit http://gerrit.cloudera.org:8080/21061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 Gerrit-Change-Number: 21061 Gerrit-PatchSet: 10 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 29 Feb 2024 22:50:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables
Tamas Mate has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/21061 ) Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables .. IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables This commit adds support for reading ARRAY columns inside Iceberg Metadata tables. The change starts with some refactoring, to consolidate accessing JVM through JNI a new backend class was introduced, IcebergMetadataScanner. This class is the C++ part of the Java IcebergMetadataScanner, it is responsible to manage the Java scanner object. In Iceberg array types do not have accessors, so structs inside arrays have to be accessed by position, for the value obtaining logics have been changed to allow access by position. The IcebergRowReader needed an IcebergMetadataScanner, so that it can iterate over the arrays returned by the scanner and add them to the collection. This change will not cover MAP, these slots are set to NULL, it will be done in IMPALA-12611. Testing: - Added E2E tests. Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 --- M be/src/exec/iceberg-metadata/CMakeLists.txt M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M be/src/service/impalad-main.cc M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 11 files changed, 734 insertions(+), 292 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/21061/10 -- To view, visit http://gerrit.cloudera.org:8080/21061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 Gerrit-Change-Number: 21061 Gerrit-PatchSet: 10 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/21061 ) Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables .. Patch Set 7: (3 comments) Thanks again for the swift reviews! http://gerrit.cloudera.org:8080/#/c/21061/7/be/src/exec/iceberg-metadata/iceberg-row-reader.cc File be/src/exec/iceberg-metadata/iceberg-row-reader.cc: http://gerrit.cloudera.org:8080/#/c/21061/7/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@250 PS7, Line 250: VLOG(3) << "Skipping unsupported column type: " << type.type; > ahh, I just noticed that we just write a log line if there is an unsupporte It is about skipping the resolution currently and making these slots NULL later. It cannot be skipped if a DCHECK catches it. http://gerrit.cloudera.org:8080/#/c/21061/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/21061/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@715 PS6, Line 715: # Describe all the metadata tables once > Ok, I'm going to add support for MAPs probably soon anyway. But please ment Done http://gerrit.cloudera.org:8080/#/c/21061/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@715 PS6, Line 715: # Describe all the metadata tables once > Expand complex types would come after we have MAP support anyway. So I stil Keeping it as NULL has already been approved by you and Zoltan a few months ago, given that MAP support is around the corner I would just leave it as is. reference: https://gerrit.cloudera.org/#/c/20010/ -- To view, visit http://gerrit.cloudera.org:8080/21061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 Gerrit-Change-Number: 21061 Gerrit-PatchSet: 7 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 29 Feb 2024 15:23:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables
Tamas Mate has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/21061 ) Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables .. IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables This commit adds support for reading ARRAY columns inside Iceberg Metadata tables. The change starts with some refactoring, to consolidate accessing JVM through JNI a new backend class was introduced, IcebergMetadataScanner. This class is the C++ part of the Java IcebergMetadataScanner, it is responsible to manage the Java scanner object. In Iceberg array types do not have accessors, so structs inside arrays have to be accessed by position, for the value obtaining logics have been changed to allow access by position. The IcebergRowReader needed an IcebergMetadataScanner, so that it can iterate over the arrays returned by the scanner and add them to the collection. This change will not cover MAP, these slots are set to NULL, it will be done in IMPALA-12611. Testing: - Added E2E tests. Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 --- M be/src/exec/iceberg-metadata/CMakeLists.txt M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M be/src/service/impalad-main.cc M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 12 files changed, 734 insertions(+), 293 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/21061/8 -- To view, visit http://gerrit.cloudera.org:8080/21061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 Gerrit-Change-Number: 21061 Gerrit-PatchSet: 8 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables
Tamas Mate has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/21061 ) Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables .. IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables This commit adds support for reading ARRAY columns inside Iceberg Metadata tables. The change starts with some refactoring, to consolidate accessing JVM through JNI a new backend class was introduced, IcebergMetadataScanner. This class is the C++ part of the Java IcebergMetadataScanner, it is responsible to manage the Java scanner object. In Iceberg array types do not have accessors, so structs inside arrays have to be accessed by position, for the value obtaining logics have been changed to allow access by position. The IcebergRowReader needed an IcebergMetadataScanner, so that it can iterate over the arrays returned by the scanner and add them to the collection. Testing: - Added E2E tests. Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 --- M be/src/exec/iceberg-metadata/CMakeLists.txt M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M be/src/service/impalad-main.cc M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 12 files changed, 735 insertions(+), 293 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/21061/7 -- To view, visit http://gerrit.cloudera.org:8080/21061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 Gerrit-Change-Number: 21061 Gerrit-PatchSet: 7 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables
Tamas Mate has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/21061 ) Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables .. IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables This commit adds support for reading ARRAY columns inside Iceberg Metadata tables. The change starts with some refactoring, to consolidate accessing JVM through JNI a new backend class was introduced, IcebergMetadataScanner. This class is the C++ part of the Java IcebergMetadataScanner, it is responsible to manage the Java scanner object. In Iceberg array types do not have accessors, so structs inside arrays have to be accessed by position, for the alue obtaining logics have been changed to allow access by position. The IcebergRowReader needed an IcebergMetadataScanner, so that it can iterate over the arrays returned by the scanner and add them to the collection. Testing: - Added E2E tests. Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 --- M be/src/exec/iceberg-metadata/CMakeLists.txt M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M be/src/service/impalad-main.cc M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 12 files changed, 746 insertions(+), 293 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/21061/6 -- To view, visit http://gerrit.cloudera.org:8080/21061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 Gerrit-Change-Number: 21061 Gerrit-PatchSet: 6 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables
Tamas Mate has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/21061 ) Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables .. IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables This commit adds support for reading ARRAY columns inside Iceberg Metadata tables. The change starts with some refactoring, to consolidate accessing JVM through JNI a new backend class was introduced, IcebergMetadataScanner. This class is the C++ part of the Java IcebergMetadataScanner, it is responsible to manage the Java scanner object. In Iceberg array types do not have accessors, so structs inside arrays have to be accessed by position, for the alue obtaining logics have been changed to allow access by position. The IcebergRowReader needed an IcebergMetadataScanner, so that it can iterate over the arrays returned by the scanner and add them to the collection. Testing: - Added E2E tests. Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 --- M be/src/exec/iceberg-metadata/CMakeLists.txt M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M be/src/service/impalad-main.cc M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 12 files changed, 746 insertions(+), 293 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/21061/5 -- To view, visit http://gerrit.cloudera.org:8080/21061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 Gerrit-Change-Number: 21061 Gerrit-PatchSet: 5 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables
Tamas Mate has posted comments on this change. (
http://gerrit.cloudera.org:8080/21061 )
Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg
Metadata tables
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21061/3/fe/src/main/java/org/apache/impala/analysis/FromClause.java
File fe/src/main/java/org/apache/impala/analysis/FromClause.java:
http://gerrit.cloudera.org:8080/#/c/21061/3/fe/src/main/java/org/apache/impala/analysis/FromClause.java@169
PS3, Line 169: tblRef.getDesc().getType().isMapType()) {
> I just realised that this change won't cover collections inside from clause
Adding Jira: IMPALA-12853
--
To view, visit http://gerrit.cloudera.org:8080/21061
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1
Gerrit-Change-Number: 21061
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Gabor Kaszab
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tamas Mate
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Wed, 28 Feb 2024 15:22:25 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables
Tamas Mate has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/21061 ) Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables .. IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables This commit adds support for reading ARRAY columns inside Iceberg Metadata tables. The change starts with some refactoring, to consolidate accessing JVM through JNI a new backend class was introduced IcebergMetadataScanner. This class is the C++ part of the Java IcebergMetadataScanner, it is responsible to manage the Java scanner object. In Iceberg array types do not have accessors, so structs inside arrays have to be accessed by position, for the the value obtaining logics have been changed to allow access by position. The IcebergRowReader needed an IcebergMetadataScanner, so that it can iterate over the arrays returned by the scanner and add them to the collection. Testing: - Added E2E tests. Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 --- M be/src/exec/iceberg-metadata/CMakeLists.txt M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M be/src/service/impalad-main.cc M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 12 files changed, 621 insertions(+), 238 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/21061/3 -- To view, visit http://gerrit.cloudera.org:8080/21061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 Gerrit-Change-Number: 21061 Gerrit-PatchSet: 3 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables
Tamas Mate has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/21061 ) Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables .. IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables This commit adds support for reading ARRAY columns inside Iceberg Metadata tables. The change starts with some refactoring, to consolidate accessing JVM through JNI a new backend class was introduced IcebergMetadataScanner. This class is the C++ part of the Java IcebergMetadataScanner, it is responsible to manage the Java scanner object. In Iceberg array types do not have accessors, so structs inside arrays have to be accessed by position, for the the value obtaining logics have been changed to allow access by position. The IcebergRowReader needed an IcebergMetadataScanner, so that it can iterate over the arrays returned by the scanner and add them to the collection. Testing: - Added E2E tests. Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 --- M be/src/exec/iceberg-metadata/CMakeLists.txt M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M be/src/service/impalad-main.cc M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 12 files changed, 622 insertions(+), 238 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/21061/2 -- To view, visit http://gerrit.cloudera.org:8080/21061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 Gerrit-Change-Number: 21061 Gerrit-PatchSet: 2 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables
Tamas Mate has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21061 Change subject: IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables .. IMPALA-12610: Support reading ARRAY columns for Iceberg Metadata tables This commit adds support for reading ARRAY columns inside Iceberg Metadata tables. The change starts with some refactoring, to consolidate accessing JVM through JNI a new backend class was introduced IcebergMetadataScanner. This class is the C++ part of the Java IcebergMetadataScanner, it is responsible to manage the Java scanner object. In Iceberg array types do not have accessors, so structs inside arrays have to be accessed by position, for the the value obtaining logics have been changed to allow access by position. The IcebergRowReader needed an IcebergMetadataScanner, so that it can iterate over the arrays returned by the scanner and add them to the collection. Testing: - Added E2E tests. Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 --- M be/src/exec/iceberg-metadata/CMakeLists.txt M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc A be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M be/src/service/impalad-main.cc M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 12 files changed, 621 insertions(+), 238 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/21061/1 -- To view, visit http://gerrit.cloudera.org:8080/21061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ieb9bac1822a17bd3cd074b4b98e4d010703cecb1 Gerrit-Change-Number: 21061 Gerrit-PatchSet: 1 Gerrit-Owner: Tamas Mate
[Impala-ASF-CR] IMPALA-12787: Concurrent DELETE and UPDATE operations on Iceberg tables can be problematic
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20999 ) Change subject: IMPALA-12787: Concurrent DELETE and UPDATE operations on Iceberg tables can be problematic .. Patch Set 1: Code-Review+1 (1 comment) Thanks for this fix Zoltan! LGTM! http://gerrit.cloudera.org:8080/#/c/20999/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/20999/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@372 PS1, Line 372: //added in the meantime, they potentially contain records that should have been nit: space -- To view, visit http://gerrit.cloudera.org:8080/20999 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e581ea17fa8f6ccd9c87aaad1281bb694079f6e Gerrit-Change-Number: 20999 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Mon, 05 Feb 2024 13:21:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12764: Fix Iceberg metadata table scan's LIMIT clause
Tamas Mate has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/20968 ) Change subject: IMPALA-12764: Fix Iceberg metadata table scan's LIMIT clause .. IMPALA-12764: Fix Iceberg metadata table scan's LIMIT clause Earlier the ReachedLimit() condition was not evaluated during IcebergMetadataScanNode::GetNext(). This missing condition has been added and LIMIT is now being evaluated. Testing: - Added E2E test Change-Id: Iea73716fe475c8b063235d2ae971a4074b8e2a20 --- M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 2 files changed, 24 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/20968/2 -- To view, visit http://gerrit.cloudera.org:8080/20968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iea73716fe475c8b063235d2ae971a4074b8e2a20 Gerrit-Change-Number: 20968 Gerrit-PatchSet: 2 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zihao Ye
[Impala-ASF-CR] IMPALA-12072: Include snapshot id of Iceberg tables in query plan / profile
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20204 ) Change subject: IMPALA-12072: Include snapshot id of Iceberg tables in query plan / profile .. Patch Set 11: Code-Review+1 (2 comments) Thanks for the clarifications Zoltan. LGTM! http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java: http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@132 PS9, Line 132: 0 nit: in Impala I have found -1s as invalid snapshot ids. http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@145 PS9, Line 145: IcebergUtil.getSnapshotId(getIceTable(), tblRef_.getTimeTravelSpec()); > An IcebergScanPlanner can create up to 3 IcebergScanNode objects, so we can Thanks for the explanation Zoltan, it totally makes sense to keep it as is. My intent with the comment was to keep the constructor light if possible. In case someone in the future has to instantiate an IcebergScanNode in a place where it is difficult to obtain. -- To view, visit http://gerrit.cloudera.org:8080/20204 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee0b4967429ea733729ad8e44df32e3b24b88525 Gerrit-Change-Number: 20204 Gerrit-PatchSet: 11 Gerrit-Owner: Gergely Fürnstáhl Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 01 Feb 2024 18:41:58 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12072: Include snapshot id of Iceberg tables in query plan / profile
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20204 ) Change subject: IMPALA-12072: Include snapshot id of Iceberg tables in query plan / profile .. Patch Set 9: (3 comments) Thanks for reviving this change Zoltan. Could you check if my idea to simplify the code makes sense? http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java File fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java: http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@74 PS9, Line 74: private long snapshotId_; This could be final. http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@277 PS9, Line 277: nit: indentation http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java: http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@145 PS9, Line 145: IcebergUtil.getSnapshotId(getIceTable(), tblRef_.getTimeTravelSpec()); Shouldn't we put this inside the IcebergScanNode's constructor? I think we could avoid passing it to every object because the TableRef is passed to the ScanNode, which holds a reference to the IcebergTable object. We might not even need the getSnapshotId() method. -- To view, visit http://gerrit.cloudera.org:8080/20204 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee0b4967429ea733729ad8e44df32e3b24b88525 Gerrit-Change-Number: 20204 Gerrit-PatchSet: 9 Gerrit-Owner: Gergely Fürnstáhl Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 01 Feb 2024 11:39:03 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12764: Fix Iceberg metadata table scan's LIMIT clause
Tamas Mate has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20968 Change subject: IMPALA-12764: Fix Iceberg metadata table scan's LIMIT clause .. IMPALA-12764: Fix Iceberg metadata table scan's LIMIT clause Earlier the ReachedLimit() condition was not evaluated during IcebergMetadataScanNode::GetNext(). This missing condition has been added and LIMIT is now being evaluated. Testing: - Added E2E test Change-Id: Iea73716fe475c8b063235d2ae971a4074b8e2a20 --- M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 2 files changed, 13 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/20968/1 -- To view, visit http://gerrit.cloudera.org:8080/20968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iea73716fe475c8b063235d2ae971a4074b8e2a20 Gerrit-Change-Number: 20968 Gerrit-PatchSet: 1 Gerrit-Owner: Tamas Mate
[Impala-ASF-CR] IMPALA-12598: Allow multiple equality filed id lists for Iceberg tables
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20951 ) Change subject: IMPALA-12598: Allow multiple equality filed id lists for Iceberg tables .. Patch Set 3: (6 comments) Hi Gabor, thank you for the feature, it looks great. Marked some nits and I have a question, what was the reason behind moving from Thrift to FlatBuffer? http://gerrit.cloudera.org:8080/#/c/20951/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20951/3//COMMIT_MSG@7 PS3, Line 7: d nit: d http://gerrit.cloudera.org:8080/#/c/20951/3//COMMIT_MSG@9 PS3, Line 9: has nit: have It refers to the plural object. http://gerrit.cloudera.org:8080/#/c/20951/3/common/fbs/IcebergObjects.fbs File common/fbs/IcebergObjects.fbs: http://gerrit.cloudera.org:8080/#/c/20951/3/common/fbs/IcebergObjects.fbs@53 PS3, Line 53: equality_field_ids I saw this in multiple places, the new term became equality_fiel_ids, in many places it was left as equality_ids. I will mark some and I think we should use one of them to avoid confusion and to be consistent. Iceberg uses equality_ids to refer to equality field ids. https://iceberg.apache.org/spec/ http://gerrit.cloudera.org:8080/#/c/20951/3/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java: http://gerrit.cloudera.org:8080/#/c/20951/3/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@126 PS3, Line 126: private Set allEqualityFieldIds_ = new HashSet<>(); : private Map, Set> equalityIdsToDeleteFiles_ = : new HashMap<>(); nit: Here we use both equality ids and equality field ids. http://gerrit.cloudera.org:8080/#/c/20951/3/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@510 PS3, Line 510: d nit: D http://gerrit.cloudera.org:8080/#/c/20951/3/testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-read-equality-deletes.test File testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-read-equality-deletes.test: http://gerrit.cloudera.org:8080/#/c/20951/3/testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-read-equality-deletes.test@114 PS3, Line 114: filed nit: field -- To view, visit http://gerrit.cloudera.org:8080/20951 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3e52d7a5800bf1b479f0c234679be92442d09f79 Gerrit-Change-Number: 20951 Gerrit-PatchSet: 3 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Jan 2024 10:33:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12590: Fix dmesg call during precommit for Ubuntu 20.04
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20763 ) Change subject: IMPALA-12590: Fix dmesg call during precommit for Ubuntu 20.04 .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/20763 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic20193740c6e5cb9e8e155c03bede55184875de5 Gerrit-Change-Number: 20763 Gerrit-PatchSet: 1 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Thu, 18 Jan 2024 15:46:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12708: An UPDATE creates 2 new snapshots in Iceberg tables
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20903 ) Change subject: IMPALA-12708: An UPDATE creates 2 new snapshots in Iceberg tables .. Patch Set 2: Code-Review+2 (4 comments) Thank you for the update Zoltan! Marked some nits, but LGTM! http://gerrit.cloudera.org:8080/#/c/20903/2/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/20903/2/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@361 PS2, Line 361: public nit: this could be private for now http://gerrit.cloudera.org:8080/#/c/20903/2/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@373 PS2, Line 373: public nit: this could be private for now http://gerrit.cloudera.org:8080/#/c/20903/2/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@384 PS2, Line 384: nit: indentation http://gerrit.cloudera.org:8080/#/c/20903/2/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@393 PS2, Line 393: rowDelta.validateFromSnapshot(initialSnapshotId); : rowDelta.validateNoConflictingDataFiles(); : rowDelta.commit(); nit: this could be chained -- To view, visit http://gerrit.cloudera.org:8080/20903 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2ceb80b939c644388707b21061bf55451234dcd3 Gerrit-Change-Number: 20903 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 18 Jan 2024 15:35:44 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11710: Table properties are not updated in Iceberg metadata files
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20907 ) Change subject: IMPALA-11710: Table properties are not updated in Iceberg metadata files .. Patch Set 2: Code-Review+2 LGTM! -- To view, visit http://gerrit.cloudera.org:8080/20907 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a82d022534e1e212d542fbd7916ae033c381c20 Gerrit-Change-Number: 20907 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 17 Jan 2024 13:55:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11710: Table properties are not updated in Iceberg metadata files
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20907 ) Change subject: IMPALA-11710: Table properties are not updated in Iceberg metadata files .. Patch Set 1: Code-Review+1 Nice catch, LGTM! -- To view, visit http://gerrit.cloudera.org:8080/20907 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a82d022534e1e212d542fbd7916ae033c381c20 Gerrit-Change-Number: 20907 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Tue, 16 Jan 2024 22:15:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12708: An UPDATE creates 2 new snapshots in Iceberg tables
Tamas Mate has posted comments on this change. (
http://gerrit.cloudera.org:8080/20903 )
Change subject: IMPALA-12708: An UPDATE creates 2 new snapshots in Iceberg
tables
..
Patch Set 1:
(2 comments)
Thanks for this fix Zoltan, LGTM!
I left a suggestion on the modifyRows method, this could be just a matter of
preference.
http://gerrit.cloudera.org:8080/#/c/20903/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/20903/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@354
PS1, Line 354: case DELETE:
: case UPDATE: modifyRows(feIcebergTable, txn, icebergOp);
break;
The two operations that modifyRows manages with the two conditions makes it a
bit difficult to read. I think it would be more readable to keep these two
operations separate. ie.: deleteRows/updateRows.
http://gerrit.cloudera.org:8080/#/c/20903/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@379
PS1, Line 379: if (dataFilesFb != null) {
Following up on the previous comment, if I am not mistaken this can only happen
with UPDATEs.
--
To view, visit http://gerrit.cloudera.org:8080/20903
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ceb80b939c644388707b21061bf55451234dcd3
Gerrit-Change-Number: 20903
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Noemi Pap-Takacs
Gerrit-Reviewer: Tamas Mate
Gerrit-Comment-Date: Tue, 16 Jan 2024 09:49:52 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12714: Fix test reduced cardinality by filter for non-HDFS
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20902 ) Change subject: IMPALA-12714: Fix test_reduced_cardinality_by_filter for non-HDFS .. Patch Set 1: Code-Review+2 Thanks for the quick fix Riza! LGTM! -- To view, visit http://gerrit.cloudera.org:8080/20902 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icbf72687cc3c5a99aa0a0a74e229ed8c88ed06ef Gerrit-Change-Number: 20902 Gerrit-PatchSet: 1 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Mon, 15 Jan 2024 17:13:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12704: Fix NPE when quering empty iceberg table's metadata
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20890 ) Change subject: IMPALA-12704: Fix NPE when quering empty iceberg table's metadata .. Patch Set 4: Code-Review+2 Thanks Zihao Ye! LGTM! -- To view, visit http://gerrit.cloudera.org:8080/20890 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6b4d4fb81a45214045b8809a4bdd910a1f1f3843 Gerrit-Change-Number: 20890 Gerrit-PatchSet: 4 Gerrit-Owner: Zihao Ye Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Mon, 15 Jan 2024 13:06:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12704: Fix NPE when quering empty iceberg table's metadata
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20890 ) Change subject: IMPALA-12704: Fix NPE when quering empty iceberg table's metadata .. Patch Set 3: (1 comment) Hi Zihao Ye, thank you for catching and fixing this issue. The change looks good to me and can +2 after a small commit message change. http://gerrit.cloudera.org:8080/#/c/20890/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20890/3//COMMIT_MSG@12 PS3, Line 12: On the Impala project we usually add a short: "Tests" or "Testing" section to the commit message to highlight how the testing was done. In this case a simple: ``` Testing: - Added E2E test to cover this case ``` would suffice in my opinion. -- To view, visit http://gerrit.cloudera.org:8080/20890 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6b4d4fb81a45214045b8809a4bdd910a1f1f3843 Gerrit-Change-Number: 20890 Gerrit-PatchSet: 3 Gerrit-Owner: Zihao Ye Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Mon, 15 Jan 2024 10:20:34 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12706: Fix nested struct querying for Iceberg metadata tables
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20883 ) Change subject: IMPALA-12706: Fix nested struct querying for Iceberg metadata tables .. Patch Set 3: Looks like the failure is due to a timing issue in: query_test.test_cancellation.TestCancellationParallel.test_cancel_select -- To view, visit http://gerrit.cloudera.org:8080/20883 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iadd029a4edc500bd8d8fca3f958903c2dbe09e8e Gerrit-Change-Number: 20883 Gerrit-PatchSet: 3 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 12 Jan 2024 06:59:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12706: Fix nested struct querying for Iceberg metadata tables
Tamas Mate has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/20883 ) Change subject: IMPALA-12706: Fix nested struct querying for Iceberg metadata tables .. IMPALA-12706: Fix nested struct querying for Iceberg metadata tables This commit fixes a DCHECK failure when querying a struct inside a struct. The previous field accessor creation logic was trying to find the ColumnDescriptor for a struct inside a struct and hit a DCHECK because there are no ColumnDescriptors for struct fields. The logic has been reworked to only use ColumnDescriptors for top level columns. Testing: - Added E2E test to cover this case Change-Id: Iadd029a4edc500bd8d8fca3f958903c2dbe09e8e --- M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 3 files changed, 29 insertions(+), 22 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/20883/2 -- To view, visit http://gerrit.cloudera.org:8080/20883 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iadd029a4edc500bd8d8fca3f958903c2dbe09e8e Gerrit-Change-Number: 20883 Gerrit-PatchSet: 2 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12706: Fix nested struct querying for Iceberg metadata tables
Tamas Mate has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20883 Change subject: IMPALA-12706: Fix nested struct querying for Iceberg metadata tables .. IMPALA-12706: Fix nested struct querying for Iceberg metadata tables This commit fixes a DCHECK failure when querying a struct inside a struct. The previous field accessor creation logic was trying to find the ColumnDescriptor for a struct inside a struct and hit a DCHECK because there are no ColumnDescriptors for struct fields. The logic has been reworked to only use ColumnDescriptors for top level columns. Testing: - Added E2E test to cover this case Change-Id: Iadd029a4edc500bd8d8fca3f958903c2dbe09e8e --- M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 3 files changed, 29 insertions(+), 22 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/20883/1 -- To view, visit http://gerrit.cloudera.org:8080/20883 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iadd029a4edc500bd8d8fca3f958903c2dbe09e8e Gerrit-Change-Number: 20883 Gerrit-PatchSet: 1 Gerrit-Owner: Tamas Mate
[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables
Tamas Mate has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/20695 ) Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables .. IMPALA-12495: Describe statement for Iceberg metadata tables Iceberg metadata tables are virtual tables and their schemata are predefined by the Iceberg library. This commit extends the DESCRIBE statement, so the users can print the table description of these tables. Metadata tables do not exist in the HMS, therefore DESCRIBE FORMATTED|EXTENDED statements are not permitted. Testing: - Added E2E tests Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 --- M be/src/service/frontend.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/analysis/TableName.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 11 files changed, 377 insertions(+), 50 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/20695/12 -- To view, visit http://gerrit.cloudera.org:8080/20695 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 Gerrit-Change-Number: 20695 Gerrit-PatchSet: 12 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables
Tamas Mate has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/20695 ) Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables .. IMPALA-12495: Describe statement for Iceberg metadata tables Iceberg metadata tables are virtual tables and their schemata are predefined by the Iceberg library. This commit extends the DESCRIBE statement, so the users can print the table description of these tables. Metadata tables do not exist in the HMS, therefore DESCRIBE FORMATTED|EXTENDED statements are not permitted. Testing: - Added E2E tests Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 --- M be/src/service/frontend.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/analysis/TableName.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 11 files changed, 372 insertions(+), 45 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/20695/10 -- To view, visit http://gerrit.cloudera.org:8080/20695 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 Gerrit-Change-Number: 20695 Gerrit-PatchSet: 10 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables
Tamas Mate has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/20695 ) Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables .. IMPALA-12495: Describe statement for Iceberg metadata tables Iceberg metadata tables are virtual tables and their schemata are predefined by the Iceberg library. This commit extends the DESCRIBE statement, so the users can print the table description of these tables. Metadata tables do not exist in the HMS, therefore DESCRIBE FORMATTED|EXTENDED statements are not permitted. Testing: - Added E2E tests Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 --- M be/src/service/frontend.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/analysis/TableName.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 11 files changed, 375 insertions(+), 46 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/20695/9 -- To view, visit http://gerrit.cloudera.org:8080/20695 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 Gerrit-Change-Number: 20695 Gerrit-PatchSet: 9 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables
Tamas Mate has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/20695 ) Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables .. IMPALA-12495: Describe statement for Iceberg metadata tables Iceberg metadata tables are virtual tables and their schemata are predefined by the Iceberg library. This commit extends the DESCRIBE statement, so the users can print the table description of these tables. Metadata tables do not exist in the HMS, therefore DESCRIBE FORMATTED|EXTENDED statements are not permitted. Testing: - Added E2E tests Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 --- M be/src/service/frontend.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/analysis/TableName.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 11 files changed, 374 insertions(+), 46 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/20695/8 -- To view, visit http://gerrit.cloudera.org:8080/20695 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 Gerrit-Change-Number: 20695 Gerrit-PatchSet: 8 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables
Tamas Mate has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/20695 ) Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables .. IMPALA-12495: Describe statement for Iceberg metadata tables Iceberg metadata tables are virtual tables and their schemata are predefined by the Iceberg library. This commit extends the DESCRIBE statement, so the users can print the table description of these tables. Metadata tables do not exist in the HMS, therefore DESCRIBE FORMATTED|EXTENDED statements are not permitted. Testing: - Added E2E tests Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 --- M be/src/service/frontend.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/analysis/TableName.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 11 files changed, 371 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/20695/7 -- To view, visit http://gerrit.cloudera.org:8080/20695 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 Gerrit-Change-Number: 20695 Gerrit-PatchSet: 7 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20695 ) Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/20695/5/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/20695/5/fe/src/main/java/org/apache/impala/service/Frontend.java@1664 PS5, Line 1664: params.result_struct > I cannot hurt. Done. It* :) -- To view, visit http://gerrit.cloudera.org:8080/20695 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 Gerrit-Change-Number: 20695 Gerrit-PatchSet: 5 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 02 Jan 2024 15:02:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables
Tamas Mate has posted comments on this change. (
http://gerrit.cloudera.org:8080/20695 )
Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables
..
Patch Set 6:
(3 comments)
Thank you for the reviews!
http://gerrit.cloudera.org:8080/#/c/20695/5/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java:
http://gerrit.cloudera.org:8080/#/c/20695/5/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java@96
PS5, Line 96: path_ = analyzer.resolvePath(rawPath_, PathType.ANY);
: } catch (AnalysisException ae) {
: /
> Would it be possible to move 'isIcebergMetadataTable(rawPath_)' and 'analyz
Done, moved it to getTable, LMK your thoughts.
http://gerrit.cloudera.org:8080/#/c/20695/5/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/20695/5/fe/src/main/java/org/apache/impala/service/Frontend.java@1657
PS5, Line 1657:
> Did you add a precondition for it? I still can't see anywhere if we check '
Done
http://gerrit.cloudera.org:8080/#/c/20695/5/fe/src/main/java/org/apache/impala/service/Frontend.java@1664
PS5, Line 1664:
> Should we add precondition check that this is set?
I cannot hurt. Done.
--
To view, visit http://gerrit.cloudera.org:8080/20695
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
Gerrit-Change-Number: 20695
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tamas Mate
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Tue, 02 Jan 2024 14:58:10 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables
Tamas Mate has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/20695 ) Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables .. IMPALA-12495: Describe statement for Iceberg metadata tables Iceberg metadata tables are virtual tables and their schemata are predefined by the Iceberg library. This commit extends the DESCRIBE statement, so the users can print the table description of these tables. Metadata tables do not exist in the HMS, therefore DESCRIBE FORMATTED|EXTENDED statements are not permitted. Testing: - Added E2E tests Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 --- M be/src/service/frontend.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/analysis/TableName.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 11 files changed, 368 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/20695/6 -- To view, visit http://gerrit.cloudera.org:8080/20695 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 Gerrit-Change-Number: 20695 Gerrit-PatchSet: 6 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR](asf-site) IMPALA-11853: Fix formatted docs query options CSS
Tamas Mate has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20824 ) Change subject: IMPALA-11853: Fix formatted docs query options CSS .. IMPALA-11853: Fix formatted docs query options CSS This is the release part of the CSS issue that was fixed by IMPALA-11853. The recent documentation formatting changes introduced the navigation panel on the left. However, due to the length of the query options navigation title these could overlap with the documentation paragraphs. This commit removes the underscores from the navigation titles of the query options, so browsers can break them into multiple lines. Additionally, the "SET" and "Query Options for the SET Statement" pages are merged to save some more space for the query option navigation titles. Change-Id: Ifcf12f401f3d4975c94ba78084ecf2c20267fad1 Reviewed-on: http://gerrit.cloudera.org:8080/20824 Tested-by: Tamas Mate Reviewed-by: Tamas Mate --- M docs/build/asf-site-html/index.html M docs/build/asf-site-html/topics/impala_abort_on_error.html M docs/build/asf-site-html/topics/impala_allow_erasure_coded_files.html M docs/build/asf-site-html/topics/impala_allow_unsupported_formats.html M docs/build/asf-site-html/topics/impala_appx_count_distinct.html M docs/build/asf-site-html/topics/impala_batch_size.html M docs/build/asf-site-html/topics/impala_broadcast_bytes_limit.html M docs/build/asf-site-html/topics/impala_buffer_pool_limit.html M docs/build/asf-site-html/topics/impala_compression_codec.html M docs/build/asf-site-html/topics/impala_compute_stats_min_sample_size.html M docs/build/asf-site-html/topics/impala_config_options.html M docs/build/asf-site-html/topics/impala_debug_action.html M docs/build/asf-site-html/topics/impala_decimal_v2.html M docs/build/asf-site-html/topics/impala_default_file_format.html M docs/build/asf-site-html/topics/impala_default_hints_insert_statement.html M docs/build/asf-site-html/topics/impala_default_join_distribution_mode.html M docs/build/asf-site-html/topics/impala_default_spillable_buffer_size.html M docs/build/asf-site-html/topics/impala_default_transactional_type.html M docs/build/asf-site-html/topics/impala_delete_stats_in_truncate.html M docs/build/asf-site-html/topics/impala_disable_codegen.html M docs/build/asf-site-html/topics/impala_disable_codegen_rows_threshold.html M docs/build/asf-site-html/topics/impala_disable_hbase_num_rows_estimate.html M docs/build/asf-site-html/topics/impala_disable_row_runtime_filtering.html M docs/build/asf-site-html/topics/impala_disable_streaming_preaggregations.html M docs/build/asf-site-html/topics/impala_disable_unsafe_spills.html M docs/build/asf-site-html/topics/impala_enable_expr_rewrites.html M docs/build/asf-site-html/topics/impala_exec_single_node_rows_threshold.html M docs/build/asf-site-html/topics/impala_exec_time_limit_s.html M docs/build/asf-site-html/topics/impala_expand_complex_types.html M docs/build/asf-site-html/topics/impala_explain_level.html M docs/build/asf-site-html/topics/impala_fetch_rows_timeout_ms.html M docs/build/asf-site-html/topics/impala_hbase_cache_blocks.html M docs/build/asf-site-html/topics/impala_hbase_caching.html M docs/build/asf-site-html/topics/impala_idle_session_timeout.html M docs/build/asf-site-html/topics/impala_join_rows_produced_limit.html M docs/build/asf-site-html/topics/impala_kudu_read_mode.html M docs/build/asf-site-html/topics/impala_live_progress.html M docs/build/asf-site-html/topics/impala_live_summary.html M docs/build/asf-site-html/topics/impala_max_errors.html M docs/build/asf-site-html/topics/impala_max_mem_estimate_for_admission.html M docs/build/asf-site-html/topics/impala_max_num_runtime_filters.html M docs/build/asf-site-html/topics/impala_max_result_spooling_mem.html M docs/build/asf-site-html/topics/impala_max_row_size.html M docs/build/asf-site-html/topics/impala_max_scan_range_length.html M docs/build/asf-site-html/topics/impala_max_spilled_result_spooling_mem.html M docs/build/asf-site-html/topics/impala_mem_limit.html M docs/build/asf-site-html/topics/impala_min_spillable_buffer_size.html M docs/build/asf-site-html/topics/impala_mt_dop.html M docs/build/asf-site-html/topics/impala_num_nodes.html M docs/build/asf-site-html/topics/impala_num_rows_produced_limit.html M docs/build/asf-site-html/topics/impala_num_scanner_threads.html M docs/build/asf-site-html/topics/impala_optimize_partition_key_scans.html M docs/build/asf-site-html/topics/impala_parquet_annotate_strings_utf8.html M docs/build/asf-site-html/topics/impala_parquet_array_resolution.html M docs/build/asf-site-html/topics/impala_parquet_compression_codec.html M docs/build/asf-site-html/topics/impala_parquet_dictionary_filtering.html M docs/build/asf-site-html/topics/impala_parquet_fallback_schema_resolution.html M docs/build/asf-site-html/topics/impala_parquet_file_size.html M docs/build/asf-site-html/topics/impala_parquet_object_store_split_size.html
[Impala-ASF-CR](asf-site) IMPALA-11853: Fix formatted docs query options CSS
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20824 ) Change subject: IMPALA-11853: Fix formatted docs query options CSS .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/20824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: comment Gerrit-Change-Id: Ifcf12f401f3d4975c94ba78084ecf2c20267fad1 Gerrit-Change-Number: 20824 Gerrit-PatchSet: 1 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Tue, 02 Jan 2024 11:57:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12658: UPDATE Iceberg table FROM view throws IllegalStateException
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20825 ) Change subject: IMPALA-12658: UPDATE Iceberg table FROM view throws IllegalStateException .. Patch Set 1: Code-Review+2 Thanks for the quick fix! LGTM! -- To view, visit http://gerrit.cloudera.org:8080/20825 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I80ccdb61327a50082f792a6d51f946b11c467dab Gerrit-Change-Number: 20825 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Tue, 02 Jan 2024 11:53:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12661: Fix ASAN heap-use-after-free in IcebergMetadataScanNode
Tamas Mate has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20829 Change subject: IMPALA-12661: Fix ASAN heap-use-after-free in IcebergMetadataScanNode .. IMPALA-12661: Fix ASAN heap-use-after-free in IcebergMetadataScanNode The ASAN builds detected that the IcebergMetadataScanNode uses heap allocated memory after it has been freed. In CreateFieldAccessors() method, during tree traversal, the current_type variable is reassigned to its children which is part of of the object. However, by the end of the assignment the rhs object will be destroyed. To fix this issue, the variable was replaced with a pointer. Testing: - Ran tests on ASAN build Change-Id: I6df9c9cb6914a0c6c93b61aa0dd02acfdba68851 --- M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc 1 file changed, 4 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/20829/1 -- To view, visit http://gerrit.cloudera.org:8080/20829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6df9c9cb6914a0c6c93b61aa0dd02acfdba68851 Gerrit-Change-Number: 20829 Gerrit-PatchSet: 1 Gerrit-Owner: Tamas Mate
[Impala-ASF-CR](asf-site) IMPALA-11853: Fix formatted docs query options CSS
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20824 ) Change subject: IMPALA-11853: Fix formatted docs query options CSS .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/20824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: comment Gerrit-Change-Id: Ifcf12f401f3d4975c94ba78084ecf2c20267fad1 Gerrit-Change-Number: 20824 Gerrit-PatchSet: 1 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Wed, 20 Dec 2023 14:36:52 + Gerrit-HasComments: No
[Impala-ASF-CR](asf-site) IMPALA-11853: Fix formatted docs query options CSS
Tamas Mate has removed a vote on this change. Change subject: IMPALA-11853: Fix formatted docs query options CSS .. Removed Verified-1 by Impala Public Jenkins -- To view, visit http://gerrit.cloudera.org:8080/20824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ifcf12f401f3d4975c94ba78084ecf2c20267fad1 Gerrit-Change-Number: 20824 Gerrit-PatchSet: 1 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate
[Impala-ASF-CR](asf-site) IMPALA-11853: Fix formatted docs query options CSS
Tamas Mate has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20824 Change subject: IMPALA-11853: Fix formatted docs query options CSS .. IMPALA-11853: Fix formatted docs query options CSS This is the release part of the CSS issue that was fixed by IMPALA-11853. The recent documentation formatting changes introduced the navigation panel on the left. However, due to the length of the query options navigation title these could overlap with the documentation paragraphs. This commit removes the underscores from the navigation titles of the query options, so browsers can break them into multiple lines. Additionally, the "SET" and "Query Options for the SET Statement" pages are merged to save some more space for the query option navigation titles. Change-Id: Ifcf12f401f3d4975c94ba78084ecf2c20267fad1 --- M docs/build/asf-site-html/index.html M docs/build/asf-site-html/topics/impala_abort_on_error.html M docs/build/asf-site-html/topics/impala_allow_erasure_coded_files.html M docs/build/asf-site-html/topics/impala_allow_unsupported_formats.html M docs/build/asf-site-html/topics/impala_appx_count_distinct.html M docs/build/asf-site-html/topics/impala_batch_size.html M docs/build/asf-site-html/topics/impala_broadcast_bytes_limit.html M docs/build/asf-site-html/topics/impala_buffer_pool_limit.html M docs/build/asf-site-html/topics/impala_compression_codec.html M docs/build/asf-site-html/topics/impala_compute_stats_min_sample_size.html M docs/build/asf-site-html/topics/impala_config_options.html M docs/build/asf-site-html/topics/impala_debug_action.html M docs/build/asf-site-html/topics/impala_decimal_v2.html M docs/build/asf-site-html/topics/impala_default_file_format.html M docs/build/asf-site-html/topics/impala_default_hints_insert_statement.html M docs/build/asf-site-html/topics/impala_default_join_distribution_mode.html M docs/build/asf-site-html/topics/impala_default_spillable_buffer_size.html M docs/build/asf-site-html/topics/impala_default_transactional_type.html M docs/build/asf-site-html/topics/impala_delete_stats_in_truncate.html M docs/build/asf-site-html/topics/impala_disable_codegen.html M docs/build/asf-site-html/topics/impala_disable_codegen_rows_threshold.html M docs/build/asf-site-html/topics/impala_disable_hbase_num_rows_estimate.html M docs/build/asf-site-html/topics/impala_disable_row_runtime_filtering.html M docs/build/asf-site-html/topics/impala_disable_streaming_preaggregations.html M docs/build/asf-site-html/topics/impala_disable_unsafe_spills.html M docs/build/asf-site-html/topics/impala_enable_expr_rewrites.html M docs/build/asf-site-html/topics/impala_exec_single_node_rows_threshold.html M docs/build/asf-site-html/topics/impala_exec_time_limit_s.html M docs/build/asf-site-html/topics/impala_expand_complex_types.html M docs/build/asf-site-html/topics/impala_explain_level.html M docs/build/asf-site-html/topics/impala_fetch_rows_timeout_ms.html M docs/build/asf-site-html/topics/impala_hbase_cache_blocks.html M docs/build/asf-site-html/topics/impala_hbase_caching.html M docs/build/asf-site-html/topics/impala_idle_session_timeout.html M docs/build/asf-site-html/topics/impala_join_rows_produced_limit.html M docs/build/asf-site-html/topics/impala_kudu_read_mode.html M docs/build/asf-site-html/topics/impala_live_progress.html M docs/build/asf-site-html/topics/impala_live_summary.html M docs/build/asf-site-html/topics/impala_max_errors.html M docs/build/asf-site-html/topics/impala_max_mem_estimate_for_admission.html M docs/build/asf-site-html/topics/impala_max_num_runtime_filters.html M docs/build/asf-site-html/topics/impala_max_result_spooling_mem.html M docs/build/asf-site-html/topics/impala_max_row_size.html M docs/build/asf-site-html/topics/impala_max_scan_range_length.html M docs/build/asf-site-html/topics/impala_max_spilled_result_spooling_mem.html M docs/build/asf-site-html/topics/impala_mem_limit.html M docs/build/asf-site-html/topics/impala_min_spillable_buffer_size.html M docs/build/asf-site-html/topics/impala_mt_dop.html M docs/build/asf-site-html/topics/impala_num_nodes.html M docs/build/asf-site-html/topics/impala_num_rows_produced_limit.html M docs/build/asf-site-html/topics/impala_num_scanner_threads.html M docs/build/asf-site-html/topics/impala_optimize_partition_key_scans.html M docs/build/asf-site-html/topics/impala_parquet_annotate_strings_utf8.html M docs/build/asf-site-html/topics/impala_parquet_array_resolution.html M docs/build/asf-site-html/topics/impala_parquet_compression_codec.html M docs/build/asf-site-html/topics/impala_parquet_dictionary_filtering.html M docs/build/asf-site-html/topics/impala_parquet_fallback_schema_resolution.html M docs/build/asf-site-html/topics/impala_parquet_file_size.html M docs/build/asf-site-html/topics/impala_parquet_object_store_split_size.html M docs/build/asf-site-html/topics/impala_parquet_page_row_count_limit.html M docs/build/asf-site-html/topic
[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns
Tamas Mate has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20759 ) Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns .. IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns As the slots have already been created on the frontend this change focuses on populating them on the backend side. There are two major parts of this commit. Obtaining the right Accessors for the slot and recursively filling the tuples with data. The field ids are present in the struct slot's ColumnType field as a list of integers. This list can be indexed with the correct element of the SchemaPath to obtain the field id for a struct member and with that the Accessor. Once the Accessors are available the IcebergRowReader's MaterializeTuple method can be called recursively to write the primitive slots of a struct slot. Testing: - Added E2E tests Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 Reviewed-on: http://gerrit.cloudera.org:8080/20759 Tested-by: Impala Public Jenkins Reviewed-by: Gabor Kaszab --- M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test M tests/query_test/test_iceberg.py 11 files changed, 280 insertions(+), 94 deletions(-) Approvals: Impala Public Jenkins: Verified Gabor Kaszab: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/20759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 Gerrit-Change-Number: 20759 Gerrit-PatchSet: 12 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns
Tamas Mate has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/20759 ) Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns .. IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns As the slots have already been created on the frontend this change focuses on populating them on the backend side. There are two major parts of this commit. Obtaining the right Accessors for the slot and recursively filling the tuples with data. The field ids are present in the struct slot's ColumnType field as a list of integers. This list can be indexed with the correct element of the SchemaPath to obtain the field id for a struct member and with that the Accessor. Once the Accessors are available the IcebergRowReader's MaterializeTuple method can be called recursively to write the primitive slots of a struct slot. Testing: - Added E2E tests Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 --- M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test M tests/query_test/test_iceberg.py 11 files changed, 280 insertions(+), 94 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/20759/11 -- To view, visit http://gerrit.cloudera.org:8080/20759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 Gerrit-Change-Number: 20759 Gerrit-PatchSet: 11 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20759 ) Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns .. Patch Set 11: Forgot to update the planner test, with the new exception. -- To view, visit http://gerrit.cloudera.org:8080/20759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 Gerrit-Change-Number: 20759 Gerrit-PatchSet: 11 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 19 Dec 2023 15:59:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns
Tamas Mate has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/20759 ) Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns .. IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns As the slots have already been created on the frontend this change focuses on populating them on the backend side. There are two major parts of this commit. Obtaining the right Accessors for the slot and recursively filling the tuples with data. The field ids are present in the struct slot's ColumnType field as a list of integers. This list can be indexed with the correct element of the SchemaPath to obtain the field id for a struct member and with that the Accessor. Once the Accessors are available the IcebergRowReader's MaterializeTuple method can be called recursively to write the primitive slots of a struct slot. Testing: - Added E2E tests Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 --- M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test M tests/query_test/test_iceberg.py 10 files changed, 280 insertions(+), 58 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/20759/10 -- To view, visit http://gerrit.cloudera.org:8080/20759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 Gerrit-Change-Number: 20759 Gerrit-PatchSet: 10 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns
Hello Gabor Kaszab, Zoltan Borok-Nagy, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20759 to look at the new patch set (#8). Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns .. IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns As the slots have already been created on the frontend this change focuses on populating them on the backend side. There are two major parts of this commit. Obtaining the right Accessors for the slot and recursively filling the tuples with data. The field ids are present in the struct slot's ColumnType field as a list of integers. This list can be indexed with the correct element of the SchemaPath to obtain the field id for a struct member and with that the Accessor. Once the Accessors are available the IcebergRowReader's MaterializeTuple method can be called recursively to write the primitive slots of a struct slot. Testing: - Added E2E tests Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 --- M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 9 files changed, 270 insertions(+), 56 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/20759/8 -- To view, visit http://gerrit.cloudera.org:8080/20759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 Gerrit-Change-Number: 20759 Gerrit-PatchSet: 8 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20759 ) Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns .. Patch Set 8: Code-Review+2 (1 comment) Thanks Gabor! Carrying over the +2. http://gerrit.cloudera.org:8080/#/c/20759/7/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc File be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/20759/7/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc@105 PS7, Line 105: childr > nit: children Done -- To view, visit http://gerrit.cloudera.org:8080/20759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 Gerrit-Change-Number: 20759 Gerrit-PatchSet: 8 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 18 Dec 2023 15:58:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20759 ) Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns .. Patch Set 7: I opened IMPALA-12651 for binary type as string support. -- To view, visit http://gerrit.cloudera.org:8080/20759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 Gerrit-Change-Number: 20759 Gerrit-PatchSet: 7 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 18 Dec 2023 15:39:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20759 ) Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns .. Patch Set 7: (2 comments) Thanks Zoltan! http://gerrit.cloudera.org:8080/#/c/20759/6/fe/src/main/java/org/apache/impala/analysis/FromClause.java File fe/src/main/java/org/apache/impala/analysis/FromClause.java: http://gerrit.cloudera.org:8080/#/c/20759/6/fe/src/main/java/org/apache/impala/analysis/FromClause.java@96 PS6, Line 96: checkIcebergCollectionSupport(tblRef); : checkTopLevelComplexAcidScan(analyzer, (CollectionTableRef)tblRef); : if (firstZippingUnnestRef != null && tblRef.isZippingUnnest() && : > nit: for readability, can we put this check into its own method? Done http://gerrit.cloudera.org:8080/#/c/20759/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/20759/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@540 PS6, Line 540: > Thanks for adding these tests! Could you please add one with this JOIN synt Done -- To view, visit http://gerrit.cloudera.org:8080/20759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 Gerrit-Change-Number: 20759 Gerrit-PatchSet: 7 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 18 Dec 2023 15:27:39 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns
Tamas Mate has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/20759 ) Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns .. IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns As the slots have already been created on the frontend this change focuses on populating them on the backend side. There are two major parts of this commit. Obtaining the right Accessors for the slot and recursively filling the tuples with data. The field ids are present in the struct slot's ColumnType field as a list of integers. This list can be indexed with the correct element of the SchemaPath to obtain the field id for a struct member and with that the Accessor. Once the Accessors are available the IcebergRowReader's MaterializeTuple method can be called recursively to write the primitive slots of a struct slot. Testing: - Added E2E tests Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 --- M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 9 files changed, 270 insertions(+), 56 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/20759/7 -- To view, visit http://gerrit.cloudera.org:8080/20759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 Gerrit-Change-Number: 20759 Gerrit-PatchSet: 7 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns
Tamas Mate has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/20759 ) Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns .. IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns As the slots have already been created on the frontend this change focuses on populating them on the backend side. There are two major parts of this commit. Obtaining the right Accessors for the slot and recursively filling the tuples with data. The field ids are present in the struct slot's ColumnType field as a list of integers. This list can be indexed with the correct element of the SchemaPath to obtain the field id for a struct member and with that the Accessor. Once the Accessors are available the IcebergRowReader's MaterializeTuple method can be called recursively to write the primitive slots of a struct slot. Testing: - Added E2E tests Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 --- M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 9 files changed, 256 insertions(+), 56 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/20759/6 -- To view, visit http://gerrit.cloudera.org:8080/20759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 Gerrit-Change-Number: 20759 Gerrit-PatchSet: 6 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12313: (part 3) Add UPDATE support for Iceberg tables.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20760 ) Change subject: IMPALA-12313: (part 3) Add UPDATE support for Iceberg tables. .. Patch Set 11: (8 comments) Looks great Zoltan, just a few minor comments. http://gerrit.cloudera.org:8080/#/c/20760/11/be/src/exec/iceberg-buffered-delete-sink.h File be/src/exec/iceberg-buffered-delete-sink.h: http://gerrit.cloudera.org:8080/#/c/20760/11/be/src/exec/iceberg-buffered-delete-sink.h@28 PS11, Line 28: struct StringVal; How come StringVal has to be redefined here? http://gerrit.cloudera.org:8080/#/c/20760/11/be/src/exec/iceberg-buffered-delete-sink.h@138 PS11, Line 138: nit: empty space http://gerrit.cloudera.org:8080/#/c/20760/11/be/src/exec/iceberg-delete-sink-base.h File be/src/exec/iceberg-delete-sink-base.h: http://gerrit.cloudera.org:8080/#/c/20760/11/be/src/exec/iceberg-delete-sink-base.h@49 PS11, Line 49: Status ConstructPartitionInfo( : const TupleRow* row, : OutputPartition* output_partition); nit: this should fit into one line http://gerrit.cloudera.org:8080/#/c/20760/11/fe/src/main/java/org/apache/impala/analysis/IcebergUpdateImpl.java File fe/src/main/java/org/apache/impala/analysis/IcebergUpdateImpl.java: http://gerrit.cloudera.org:8080/#/c/20760/11/fe/src/main/java/org/apache/impala/analysis/IcebergUpdateImpl.java@20 PS11, Line 20: import static java.lang.String.format; unused import http://gerrit.cloudera.org:8080/#/c/20760/11/fe/src/main/java/org/apache/impala/analysis/IcebergUpdateImpl.java@30 PS11, Line 30: import org.apache.impala.catalog.FeFsTable; unused import http://gerrit.cloudera.org:8080/#/c/20760/11/fe/src/main/java/org/apache/impala/analysis/IcebergUpdateImpl.java@37 PS11, Line 37: import org.apache.impala.planner.IcebergDeleteSink; unused import http://gerrit.cloudera.org:8080/#/c/20760/11/fe/src/main/java/org/apache/impala/analysis/IcebergUpdateImpl.java@44 PS11, Line 44: import com.google.common.collect.ImmutableList; unused import http://gerrit.cloudera.org:8080/#/c/20760/11/fe/src/main/java/org/apache/impala/planner/IcebergBufferedDeleteSink.java File fe/src/main/java/org/apache/impala/planner/IcebergBufferedDeleteSink.java: http://gerrit.cloudera.org:8080/#/c/20760/11/fe/src/main/java/org/apache/impala/planner/IcebergBufferedDeleteSink.java@34 PS11, Line 34: IcebergBufferedDeleteSink For me it looks like that ~40% of this class is identical with IcebergDeleteSink. Do you think it would worth inheriting from IcebergDeleteSink class instead of TableSink? -- To view, visit http://gerrit.cloudera.org:8080/20760 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2bb97b4454165a292975d88dc9c23adb22ff7315 Gerrit-Change-Number: 20760 Gerrit-PatchSet: 11 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 15 Dec 2023 14:27:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns
Tamas Mate has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/20759 ) Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns .. IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns As the slots have already been created on the frontend this change focuses on populating them on the backend side. There are two major parts of this commit. Obtaining the right Accessors for the slot and recursively filling the tuples with data. The field ids are present in the struct slot's ColumnType field as a list of integers. This list can be indexed with the correct element of the SchemaPath to obtain the field id for a struct member and with that the Accessor. Once the Accessors are available the IcebergRowReader's MaterializeTuple method can be called recursively to write the primitive slots of a struct slot. Testing: - Added E2E tests Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 --- M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 8 files changed, 241 insertions(+), 56 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/20759/5 -- To view, visit http://gerrit.cloudera.org:8080/20759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 Gerrit-Change-Number: 20759 Gerrit-PatchSet: 5 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns
Tamas Mate has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/20759 ) Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns .. IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns As the slots have already been created on the frontend this change focuses on populating them on the backend side. There are two major parts of this commit. Obtaining the right Accessors for the slot and recursively filling the tuples with data. The field ids are present in the struct slot's ColumnType field as a list of integers. This list can be indexed with the correct element of the SchemaPath to obtain the field id for a struct member and with that the Accessor. Once the Accessors are available the IcebergRowReader's MaterializeTuple method can be called recursively to write the primitive slots of a struct slot. Testing: - Added E2E tests Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 --- M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 8 files changed, 209 insertions(+), 60 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/20759/4 -- To view, visit http://gerrit.cloudera.org:8080/20759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 Gerrit-Change-Number: 20759 Gerrit-PatchSet: 4 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns
Tamas Mate has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/20759 ) Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns .. IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns As the slots have already been created on the frontend this change focuses on populating them on the backend side. There are two major parts of this commit. Obtaining the right Accessors for the slot and recursively filling the tuples with data. The field ids are present in the struct slot's ColumnType field as a list of integers. This list can be indexed with the correct element of the SchemaPath to obtain the field id for a struct member and with that the Accessor. Once the Accessors are available the IcebergRowReader's MaterializeTuple method can be called recursively to write the primitive slots of a struct slot. Testing: - Added E2E tests Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 --- M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 8 files changed, 204 insertions(+), 60 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/20759/3 -- To view, visit http://gerrit.cloudera.org:8080/20759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 Gerrit-Change-Number: 20759 Gerrit-PatchSet: 3 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12597: Basic Equality delete read support for Iceberg tables
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20753 ) Change subject: IMPALA-12597: Basic Equality delete read support for Iceberg tables .. Patch Set 2: (4 comments) Looks great, found a few nits. http://gerrit.cloudera.org:8080/#/c/20753/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20753/1//COMMIT_MSG@41 PS1, Line 41: TODO Is there a Jira for these items? It would help linking this commit with other tasks. http://gerrit.cloudera.org:8080/#/c/20753/2/common/thrift/CatalogObjects.thrift File common/thrift/CatalogObjects.thrift: http://gerrit.cloudera.org:8080/#/c/20753/2/common/thrift/CatalogObjects.thrift@635 PS2, Line 635: } nit: missing new line http://gerrit.cloudera.org:8080/#/c/20753/2/fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java File fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java: http://gerrit.cloudera.org:8080/#/c/20753/2/fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java@106 PS2, Line 106: TODO TODO without Jira ID. http://gerrit.cloudera.org:8080/#/c/20753/2/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test File testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test: http://gerrit.cloudera.org:8080/#/c/20753/2/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test@1179 PS2, Line 1179: 04:UNION nit: trailing spaces -- To view, visit http://gerrit.cloudera.org:8080/20753 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2053e6f321c69f1c82059a84a5d99aeaa9814cad Gerrit-Change-Number: 20753 Gerrit-PatchSet: 2 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 08 Dec 2023 11:56:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12313: (part 2) Limited UPDATE support for Iceberg tables
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20677 ) Change subject: IMPALA-12313: (part 2) Limited UPDATE support for Iceberg tables .. Patch Set 9: (1 comment) Looks good to me! I can plus two after these comments. http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java File fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java: http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java@63 PS9, Line 63: // Currently only Kudu tables are supported. Not anymore :) -- To view, visit http://gerrit.cloudera.org:8080/20677 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff0ef6075a2b6ebe130d15daa389ac1a505a7a08 Gerrit-Change-Number: 20677 Gerrit-PatchSet: 9 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 08 Dec 2023 11:32:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12313: (part 2) Limited UPDATE support for Iceberg tables
Tamas Mate has posted comments on this change. (
http://gerrit.cloudera.org:8080/20677 )
Change subject: IMPALA-12313: (part 2) Limited UPDATE support for Iceberg tables
..
Patch Set 9:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/20677/9/be/src/exec/iceberg-delete-sink.h
File be/src/exec/iceberg-delete-sink.h:
http://gerrit.cloudera.org:8080/#/c/20677/9/be/src/exec/iceberg-delete-sink.h@81
PS9, Line 81: /// Verifies that the row batch does not contain duplicated
rows.
Could you add an explanation of intent, it is not clear for me why this is
needed.
http://gerrit.cloudera.org:8080/#/c/20677/9/be/src/exec/iceberg-delete-sink.h@82
PS9, Line 82: VerifyRowBatch
Nit: For readability could you rename to something like:
VerifyRowsNotDuplicated/VerifyRowDistinctiveness
http://gerrit.cloudera.org:8080/#/c/20677/9/be/src/exec/multi-table-sink.cc
File be/src/exec/multi-table-sink.cc:
http://gerrit.cloudera.org:8080/#/c/20677/9/be/src/exec/multi-table-sink.cc@52
PS9, Line 52: tsinkBase
nit: tsink_base
http://gerrit.cloudera.org:8080/#/c/20677/9/be/src/runtime/dml-exec-state.h
File be/src/runtime/dml-exec-state.h:
http://gerrit.cloudera.org:8080/#/c/20677/9/be/src/runtime/dml-exec-state.h@72
PS9, Line 72: void UpdatePartition(const std::string& partition_name,
: int64_t num_rows_delta, const DmlStatsPB* insert_stats,
: bool is_delete = false);
nit: should fit into 2 lines
http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java:
http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java@120
PS9, Line 120: id
nit: ++nextTableId_ and return nextTableId_
http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/analysis/IcebergModifyImpl.java
File fe/src/main/java/org/apache/impala/analysis/IcebergModifyImpl.java:
http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/analysis/IcebergModifyImpl.java@87
PS9, Line 87: public List getDeletePartitionExprs(Analyzer analyzer)
: throws AnalysisException {
nit: should fit into 1 line
http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/analysis/IcebergModifyImpl.java@94
PS9, Line 94: public List getDeleteResultExprs(Analyzer analyzer)
: throws AnalysisException {
nit: should fit into 1 line
http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/planner/Planner.java
File fe/src/main/java/org/apache/impala/planner/Planner.java:
http://gerrit.cloudera.org:8080/#/c/20677/9/fe/src/main/java/org/apache/impala/planner/Planner.java@185
PS9, Line 185:
nit: indentation
--
To view, visit http://gerrit.cloudera.org:8080/20677
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iff0ef6075a2b6ebe130d15daa389ac1a505a7a08
Gerrit-Change-Number: 20677
Gerrit-PatchSet: 9
Gerrit-Owner: Zoltan Borok-Nagy
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Gabor Kaszab
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Noemi Pap-Takacs
Gerrit-Reviewer: Tamas Mate
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Thu, 07 Dec 2023 17:55:35 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns
Tamas Mate has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/20759 ) Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns .. IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns As the slots have already been created on the frontend this change focuses on populating them on the backend side. There are two major parts of this commit. Obtaining the right Accessors for the slot and recursively filling the tuples with data. The field ids are present in the struct slot's ColumnType field as a list of integers. This list can be indexed with the correct element of the SchemaPath to obtain the field id for a struct member and with that the Accessor. Once the Accessors are available the IcebergRowReader's MaterializeTuple method can be called recursively to write the primitive slots of a struct slot. Testing: - Added E2E tests Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 --- M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 8 files changed, 164 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/20759/2 -- To view, visit http://gerrit.cloudera.org:8080/20759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 Gerrit-Change-Number: 20759 Gerrit-PatchSet: 2 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20759 ) Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/20759/1/be/src/exec/iceberg-metadata/iceberg-row-reader.h File be/src/exec/iceberg-metadata/iceberg-row-reader.h: http://gerrit.cloudera.org:8080/#/c/20759/1/be/src/exec/iceberg-metadata/iceberg-row-reader.h@45 PS1, Line 45: Status MaterializeTuple(JNIEnv* env, jobject struct_like_row, const TupleDescriptor* tuple_desc, > line too long (98 > 90) Done http://gerrit.cloudera.org:8080/#/c/20759/1/be/src/exec/iceberg-metadata/iceberg-row-reader.cc File be/src/exec/iceberg-metadata/iceberg-row-reader.cc: http://gerrit.cloudera.org:8080/#/c/20759/1/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@79 PS1, Line 79: accessed_value = env->CallObjectMethod(accessor, iceberg_accessor_get_, struct_like_row); > line too long (95 > 90) Done http://gerrit.cloudera.org:8080/#/c/20759/1/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@104 PS1, Line 104: RETURN_IF_ERROR(WriteStructSlot(env, struct_like_row, slot_desc, tuple, tuple_data_pool)); > line too long (98 > 90) Done http://gerrit.cloudera.org:8080/#/c/20759/1/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@172 PS1, Line 172: Status IcebergRowReader::WriteStructSlot(JNIEnv* env, jobject struct_like_row, SlotDescriptor* slot_desc, Tuple* tuple, > line too long (119 > 90) Done -- To view, visit http://gerrit.cloudera.org:8080/20759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 Gerrit-Change-Number: 20759 Gerrit-PatchSet: 1 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 07 Dec 2023 10:07:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12313: (part 2) Limited UPDATE support for Iceberg tables
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20677 ) Change subject: IMPALA-12313: (part 2) Limited UPDATE support for Iceberg tables .. Patch Set 7: (1 comment) Great change Zoltan! Started to review and tested it a bit. The MultiTableSink counters in the profile seem to be off, the counters are not updated based on the sum of "child" sinks. http://gerrit.cloudera.org:8080/#/c/20677/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20677/7//COMMIT_MSG@75 PS7, Line 75: Patch nit: Part -- To view, visit http://gerrit.cloudera.org:8080/20677 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff0ef6075a2b6ebe130d15daa389ac1a505a7a08 Gerrit-Change-Number: 20677 Gerrit-PatchSet: 7 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 06 Dec 2023 14:16:21 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns
Tamas Mate has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20759 Change subject: IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns .. IMPALA-12205: Add support to STRUCT type Iceberg Metadata table columns As the slots have already been created on the frontend this change focuses on populating them on the backend side. There are two major parts of this commit. Obtaining the right Accessors for the slot and recursively filling the tuples with data. The field ids are present in the struct slot's ColumnType field as a list of integers. This list can be indexed with the correct element of the SchemaPath to obtain the field id for a struct member and with that the Accessor. Once the Accessors are available the IcebergRowReader's MaterializeTuple method can be called recursively to write the primitive slots of a struct slot. Testing: - Added E2E tests Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 --- M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 8 files changed, 162 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/20759/1 -- To view, visit http://gerrit.cloudera.org:8080/20759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I953ad7253b270f2855bfcaee4ad023d1c4469273 Gerrit-Change-Number: 20759 Gerrit-PatchSet: 1 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables
Tamas Mate has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/20695 ) Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables .. IMPALA-12495: Describe statement for Iceberg metadata tables Iceberg metadata tables are virtual tables and their schemata are predefined by the Iceberg library. This commit extends the DESCRIBE statement, so the users can print the table description of these tables. Metadata tables do not exist in the HMS, therefore DESCRIBE FORMATTED|EXTENDED statements are not permitted. Testing: - Added E2E tests Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 --- M be/src/service/frontend.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 8 files changed, 363 insertions(+), 40 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/20695/5 -- To view, visit http://gerrit.cloudera.org:8080/20695 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 Gerrit-Change-Number: 20695 Gerrit-PatchSet: 5 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate
[Impala-ASF-CR] IMPALA-12243: Add support for DROP PARTITION for Iceberg tables
Tamas Mate has posted comments on this change. (
http://gerrit.cloudera.org:8080/20515 )
Change subject: IMPALA-12243: Add support for DROP PARTITION for Iceberg tables
..
Patch Set 9: Code-Review+1
(1 comment)
Thanks Peter! LGTM!
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java
File fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java:
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@253
PS8, Line 253: String.format("Unable to parse timestamp value from: %s",
> This throw statement is in the same block as the whole try-catch structure,
Probably a misclick and wanted to comment eh LOG.warn() lines.
--
To view, visit http://gerrit.cloudera.org:8080/20515
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a768ba2966f570454687e02e4e6d67df46741f9
Gerrit-Change-Number: 20515
Gerrit-PatchSet: 9
Gerrit-Owner: Peter Rozsa
Gerrit-Reviewer: Gabor Kaszab
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Peter Rozsa
Gerrit-Reviewer: Tamas Mate
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Wed, 22 Nov 2023 10:04:35 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables
Tamas Mate has posted comments on this change. (
http://gerrit.cloudera.org:8080/20695 )
Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables
..
Patch Set 2:
(9 comments)
Thank you for the review Daniel!
Updated the patch.
http://gerrit.cloudera.org:8080/#/c/20695/1/be/src/service/frontend.cc
File be/src/service/frontend.cc:
http://gerrit.cloudera.org:8080/#/c/20695/1/be/src/service/frontend.cc@183
PS1, Line 183: if (params.__isset.metadata_table_name)
tparams.__set_metadata_table_name(
> line too long (104 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java:
http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java@96
PS2, Line 96: analyzeIcebergMetadataTable
> I found the name 'analyzeIcebergMetadataTable' a bit misleading, to me it s
Indeed, I decided to move the 'isIcebergMetadataTable' outside of the
'analyzeIcebergMetadataTable' method.
http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
File fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java:
http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java@358
PS2, Line 358: a
> Nit: "an".
Done
http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/service/Frontend.java@1642
PS2, Line 1642:* Returns table metadata, such as the column descriptors, in
the specified table.
> Could mention 'vTableName' in the comment, including that it should be null
Refactored this part, let me know your thoughts.
http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/service/Frontend.java@1650
PS2, Line 1650: String.format("fetching table %s.%s",
tableName.db_name, tableName.table_name));
> Isn't this message misleading if we are actually looking up a metadata tabl
The "parent" table is being loaded for this query. The metadata table does not
exist in the catalog.
http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/service/Frontend.java@1708
PS2, Line 1708: Preconditions.checkArgument(outputStyle ==
TDescribeOutputStyle.FORMATTED ||
> Optional: could add a Precondition check that 'vTableName' is null.
Done
http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java
File fe/src/main/java/org/apache/impala/service/JniFrontend.java:
http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java@503
PS2, Line 503: metadata_table_name
> Shouldn't we check whether this param is set? I don't know if Thrift guaran
Taken care of as part of the refactoring.
http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
File
fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java:
http://gerrit.cloudera.org:8080/#/c/20695/2/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java@273
PS2, Line 273: ""
> Shouldn't it be null instead?
With the refactoring this changed.
http://gerrit.cloudera.org:8080/#/c/20695/2/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/20695/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@618
PS2, Line 618: describe
functional_parquet.iceberg_query_metadata.all_data_files;
> Shouldn't the 'all_*_files' tables be grouped together with the other '*fil
You mean to not duplicate them?
The reason I kept them separate is that there is an Iceberg library class
representation for each of these, such as
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/AllDataFilesTable.java
--
To view, visit http://gerrit.cloudera.org:8080/20695
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97
Gerrit-Change-Number: 20695
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tamas Mate
Gerrit-Comment-Date: Tue, 14 Nov 2023 11:28:25 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12243: Add support for DROP PARTITION for Iceberg tables
Tamas Mate has posted comments on this change. (
http://gerrit.cloudera.org:8080/20515 )
Change subject: IMPALA-12243: Add support for DROP PARTITION for Iceberg tables
..
Patch Set 8:
(17 comments)
Thank you for the changes Peter.
Added some comments, mainly nits/formatting.
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
File
fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java:
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java@96
PS8, Line 96: addPartParams
This is not originating from your change, but looks like an copy&paste from
AlterTableAddPartitionStmt. Could you rename it?
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java
File fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java:
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java@39
PS8, Line 39:
nit: empty new line
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java@129
PS8, Line 129: Column column = desc.getColumn();
: IcebergColumn icebergColumn = (IcebergColumn) column;
nit: could be one line; then we don't need to import Column
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java@133
PS8, Line 133: partitionSpec_.getFieldsBySourceId(
: icebergColumn.getFieldId());
nit: should fit into one line
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java@182
PS8, Line 182: /*Keeping it empty to avoid constant folding */
nit: missing space
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpressionRewriter.java
File
fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpressionRewriter.java:
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpressionRewriter.java@102
PS8, Line 102: if (!(literal instanceof LiteralExpr)) {
: return binaryPredicate;
: }
nit: this could fit into one line, like L107
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
File fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java:
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java@98
PS8, Line 98: if (table instanceof IcebergTable){
nit: space
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java@100
PS8, Line 100: }
: else{
nit: one line
http://gerrit.cloudera.org:8080/#/c/20515/2/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/20515/2/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@401
PS2, Line 401:
> It originates from IcebergUtil/getPartitionTransformType. Before this patch
Yeah, somehow we should prevent AnalysisException during table load.
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java
File fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java:
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@170
PS8, Line 170: Preconditions.checkState(false,
: "Unsupported Iceberg type considered for predicate:
%s",
: literal.getType().toSql());
: }
: }
: throw new ImpalaRuntimeException(
: String.format("Unable to parse Iceberg value: %s,",
: literal.getStringValue()));
This throw + precondition check looks redundant.
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@253
PS8, Line 253: throw new ImpalaRuntimeException(
nit: indentation
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@296
PS8, Line 296: String.format("Unable to create term from expression:
%s",
: expr.toSql()));
nit: one line
http://gerrit.cloudera.org:8080/#/c/20515/8/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@302
PS8, Line 302: String.format("Invalid column type %s for column: %s",
: column.getType(), column));
nit: one line
h
[Impala-ASF-CR] IMPALA-12308: DIRECTED distribution mode for V2 Iceberg tables
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20548 ) Change subject: IMPALA-12308: DIRECTED distribution mode for V2 Iceberg tables .. Patch Set 13: Code-Review+2 (1 comment) Thank you for the changes Gabor! LGTM! http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/query-state.h File be/src/runtime/query-state.h: http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/query-state.h@135 PS10, Line 135: int > protobuf doesn't have int just int32, but here int is a synonym for for int I do not think that anything could go south here, this was mainly a readability suggestion and to guarantee the size of the integer here. Protobuff generates int32_t here if I am not mistaken. https://protobuf.dev/getting-started/cpptutorial/#protobuf-api -- To view, visit http://gerrit.cloudera.org:8080/20548 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I212afd7c9e94551a1c50a40ccb0e3c1f7ecdf3d2 Gerrit-Change-Number: 20548 Gerrit-PatchSet: 13 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 13 Nov 2023 13:38:39 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12527: Fix Iceberg metadata table test S3 paths
Tamas Mate has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20659 ) Change subject: IMPALA-12527: Fix Iceberg metadata table test S3 paths .. IMPALA-12527: Fix Iceberg metadata table test S3 paths This commit excludes test_metadata_tables executions which could fail due to: 1) hardcoded uris in delete files 2) reused data load result, that points to a different file system Testing: - HDFS: tested in my local dev environment - S3: executed a jenkins job with the failing S3 parameters Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562 Reviewed-on: http://gerrit.cloudera.org:8080/20659 Tested-by: Impala Public Jenkins Reviewed-by: Zoltan Borok-Nagy Reviewed-by: Peter Rozsa --- M tests/query_test/test_iceberg.py 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: Impala Public Jenkins: Verified Zoltan Borok-Nagy: Looks good to me, approved Peter Rozsa: Looks good to me, but someone else must approve -- To view, visit http://gerrit.cloudera.org:8080/20659 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562 Gerrit-Change-Number: 20659 Gerrit-PatchSet: 6 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12527: Fix Iceberg metadata table test S3 paths
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20659 ) Change subject: IMPALA-12527: Fix Iceberg metadata table test S3 paths .. Patch Set 5: Thank you for the reviews! I will split the tests with the STRUCT type addition. -- To view, visit http://gerrit.cloudera.org:8080/20659 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562 Gerrit-Change-Number: 20659 Gerrit-PatchSet: 5 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 13 Nov 2023 09:27:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables
Tamas Mate has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/20695 ) Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables .. IMPALA-12495: Describe statement for Iceberg metadata tables Iceberg metadata tables are virtual tables and their schemata are predefined by the Iceberg library. This commit extends the DESCRIBE statement, so the users can print the table description of these tables. Metadata tables do not exist in the HMS, therefore DESCRIBE FORMATTED|EXTENDED statements are not permitted. Testing: - Added E2E tests Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 --- M be/src/service/frontend.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 8 files changed, 323 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/20695/2 -- To view, visit http://gerrit.cloudera.org:8080/20695 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 Gerrit-Change-Number: 20695 Gerrit-PatchSet: 2 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-12495: Describe statement for Iceberg metadata tables
Tamas Mate has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20695 Change subject: IMPALA-12495: Describe statement for Iceberg metadata tables .. IMPALA-12495: Describe statement for Iceberg metadata tables Iceberg metadata tables are virtual tables and their schemata are predefined by the Iceberg library. This commit extends the DESCRIBE statement, so the users can print the table description of these tables. Metadata tables do not exist in the HMS, therefore DESCRIBE FORMATTED|EXTENDED statements are not permitted. Testing: - Added E2E tests Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 --- M be/src/service/frontend.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 8 files changed, 322 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/20695/1 -- To view, visit http://gerrit.cloudera.org:8080/20695 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ibe22f271a59a6885035991c09b5101193ade6e97 Gerrit-Change-Number: 20695 Gerrit-PatchSet: 1 Gerrit-Owner: Tamas Mate
[Impala-ASF-CR] IMPALA-12308: DIRECTED distribution mode for V2 Iceberg tables
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20548 ) Change subject: IMPALA-12308: DIRECTED distribution mode for V2 Iceberg tables .. Patch Set 10: (13 comments) Thank you for the updates Gabor, I have left some further comments. http://gerrit.cloudera.org:8080/#/c/20548/4/be/src/exec/data-sink.h File be/src/exec/data-sink.h: http://gerrit.cloudera.org:8080/#/c/20548/4/be/src/exec/data-sink.h@90 PS4, Line 90: protected: nit: a new line was removed http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/fragment-state.h File be/src/runtime/fragment-state.h: http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/fragment-state.h@242 PS10, Line 242: p nit: P http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/fragment-state.cc File be/src/runtime/fragment-state.cc: http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/fragment-state.cc@97 PS10, Line 97: nit: empty line http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/fragment-state.cc@131 PS10, Line 131: filename_to_hosts_[full_file_path] Inserting full_file_path to a filename_to_hosts map looks a bit odd. http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/krpc-data-stream-sender.h File be/src/runtime/krpc-data-stream-sender.h: http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/krpc-data-stream-sender.h@193 PS10, Line 193: Funtions nit: Functions http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/krpc-data-stream-sender.h@197 PS10, Line 197: i nit: I http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/query-state.h File be/src/runtime/query-state.h: http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/query-state.h@135 PS10, Line 135: int int32 to make it a bit more aligned with admission_control_service.proto description. http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/runtime/query-state.h@172 PS10, Line 172: const http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/scheduling/scheduler.h File be/src/scheduling/scheduler.h: http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/scheduling/scheduler.h@86 PS10, Line 86: /// Map from filenames to hosts where those files are scheduled, grouped by scan node : // ID. nit: one line or the second line needs /// http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/scheduling/scheduler.cc File be/src/scheduling/scheduler.cc: http://gerrit.cloudera.org:8080/#/c/20548/10/be/src/scheduling/scheduler.cc@980 PS10, Line 980: ByNodeFilenamesToHosts duplicate_check; Unused variable, maybe even the parameter in PopulateFilenamesToHostsMapping. http://gerrit.cloudera.org:8080/#/c/20548/10/common/thrift/Partitions.thrift File common/thrift/Partitions.thrift: http://gerrit.cloudera.org:8080/#/c/20548/10/common/thrift/Partitions.thrift@44 PS10, Line 44: of nit: for http://gerrit.cloudera.org:8080/#/c/20548/10/fe/src/main/java/org/apache/impala/planner/DataPartition.java File fe/src/main/java/org/apache/impala/planner/DataPartition.java: http://gerrit.cloudera.org:8080/#/c/20548/10/fe/src/main/java/org/apache/impala/planner/DataPartition.java@79 PS10, Line 79: public final static DataPartition DIRECTED = new DataPartition(TPartitionType.DIRECTED); nit: I think this would look more consistent with the class structure in L70 http://gerrit.cloudera.org:8080/#/c/20548/10/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java File fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java: http://gerrit.cloudera.org:8080/#/c/20548/10/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@79 PS10, Line 79: } nit: missing empty line -- To view, visit http://gerrit.cloudera.org:8080/20548 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I212afd7c9e94551a1c50a40ccb0e3c1f7ecdf3d2 Gerrit-Change-Number: 20548 Gerrit-PatchSet: 10 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 09 Nov 2023 16:22:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12527: Fix Iceberg metadata table test S3 paths
Tamas Mate has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/20659 ) Change subject: IMPALA-12527: Fix Iceberg metadata table test S3 paths .. IMPALA-12527: Fix Iceberg metadata table test S3 paths This commit excludes test_metadata_tables executions which could fail due to: 1) hardcoded uris in delete files 2) reused data load result, that points to a different file system Testing: - HDFS: tested in my local dev environment - S3: executed a jenkins job with the failing S3 parameters Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562 --- M tests/query_test/test_iceberg.py 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/20659/5 -- To view, visit http://gerrit.cloudera.org:8080/20659 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562 Gerrit-Change-Number: 20659 Gerrit-PatchSet: 5 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12527: Fix Iceberg metadata table test S3 paths
Tamas Mate has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/20659 ) Change subject: IMPALA-12527: Fix Iceberg metadata table test S3 paths .. IMPALA-12527: Fix Iceberg metadata table test S3 paths The Iceberg library does not necessarily add the filesystem prefix to the file paths, which can cause tests on different filesystems to fail. This change removes the filesystem checks from the file paths. Testing: - HDFS: tested in my local dev environment - S3: executed the test on with the failing S3 parameters Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562 --- M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test M tests/query_test/test_iceberg.py 2 files changed, 43 insertions(+), 42 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/20659/4 -- To view, visit http://gerrit.cloudera.org:8080/20659 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562 Gerrit-Change-Number: 20659 Gerrit-PatchSet: 4 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12527: Fix Iceberg metadata table test S3 paths
Tamas Mate has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/20659 ) Change subject: IMPALA-12527: Fix Iceberg metadata table test S3 paths .. IMPALA-12527: Fix Iceberg metadata table test S3 paths The Iceberg library does not necessarily add the filesystem prefix to the file paths, which can cause tests on different filesystems to fail. This change removes the filesystem checks from the file paths. Testing: - HDFS: tested in my local dev environment - S3: executed the jenkins job with the failing S3 parameters Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562 --- M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test M tests/query_test/test_iceberg.py 2 files changed, 44 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/20659/2 -- To view, visit http://gerrit.cloudera.org:8080/20659 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562 Gerrit-Change-Number: 20659 Gerrit-PatchSet: 2 Gerrit-Owner: Tamas Mate
[Impala-ASF-CR] IMPALA-12527: Fix Iceberg metadata table test S3 paths
Tamas Mate has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20659 Change subject: IMPALA-12527: Fix Iceberg metadata table test S3 paths .. IMPALA-12527: Fix Iceberg metadata table test S3 paths The Iceberg library does not necessarily add the filesystem prefix to the file paths, which can cause tests on different filesystems to fail. This change removes the filesystem checks from the file paths. Testing: - HDFS: tested in my local dev environment - S3: executed the test on with the failing S3 parameters Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562 --- M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test M tests/query_test/test_iceberg.py 2 files changed, 44 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/20659/1 -- To view, visit http://gerrit.cloudera.org:8080/20659 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I04ee4798c643f7cce59efa80b3327f7f37c80562 Gerrit-Change-Number: 20659 Gerrit-PatchSet: 1 Gerrit-Owner: Tamas Mate
[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20010 ) Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying .. Patch Set 17: Code-Review+2 Fixed a test case that was introduced by the Iceberg lib upgrade. Carrying over the +2. -- To view, visit http://gerrit.cloudera.org:8080/20010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 Gerrit-Change-Number: 20010 Gerrit-PatchSet: 17 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 26 Oct 2023 08:17:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying
Tamas Mate has uploaded a new patch set (#17). ( http://gerrit.cloudera.org:8080/20010 ) Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying .. IMPALA-11996: Scanner change for Iceberg metadata querying This commit adds a scan node for querying Iceberg metadata tables. The scan node creates a Java scanner object that creates and scans the metadata table. The scanner uses the Iceberg API to scan the table after that the scan node fetches the rows one by one and materialises them into RowBatches. The Iceberg row reader on the backend does the translation between Iceberg and Impala types. There is only one fragment created to query the Iceberg metadata table which is supposed to be executed on the coordinator node that already has the Iceberg table loaded. This way there is no need for further table loading on the executor side. This change will not cover nested column types, these slots are set to NULL, it will be done in IMPALA-12205. Testing: - Added e2e tests for querying metadata tables - Updated planner tests Performance testing: Created a table and inserted ~5500 rows one by one, this generated ~27 ALL_MANIFESTS metadata table records. This table is quite wide and has a String column as well. I only mention count(*) test on ALL_MANIFESTS, because every row is materialized in every scenario currently: - Cold cache: 15.76s - IcebergApiScanTime: 124.407ms - MaterializeTupleTime: 8s368ms - Warm cache: 7.56s - IcebergApiScanTime: 3.646ms - MaterializeTupleTime: 7s477ms Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 --- M be/CMakeLists.txt M be/src/exec/CMakeLists.txt M be/src/exec/exec-node.cc A be/src/exec/iceberg-metadata/CMakeLists.txt A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h A be/src/exec/iceberg-metadata/iceberg-row-reader.cc A be/src/exec/iceberg-metadata/iceberg-row-reader.h M be/src/scheduling/scheduler.cc M be/src/service/frontend.cc M be/src/service/frontend.h M be/src/service/impalad-main.cc M be/src/util/jni-util.cc M be/src/util/jni-util.h M common/thrift/PlanNodes.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M 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/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/IcebergMetadataScanNode.java M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/datasets/functional/functional_schema_template.sql M testdata/datasets/functional/schema_constraints.csv M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test M tests/authorization/test_ranger.py M tests/query_test/test_iceberg.py 32 files changed, 1,419 insertions(+), 167 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/20010/17 -- To view, visit http://gerrit.cloudera.org:8080/20010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 Gerrit-Change-Number: 20010 Gerrit-PatchSet: 17 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-9118: Show catalog operation details in catalogd webUI
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20428 ) Change subject: IMPALA-9118: Show catalog operation details in catalogd webUI .. Patch Set 7: Code-Review+2 Thanks Quanlong, LGTM! -- To view, visit http://gerrit.cloudera.org:8080/20428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3cf3f0da2be2be79e546762a8083d4de338ff6aa Gerrit-Change-Number: 20428 Gerrit-PatchSet: 7 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Wed, 25 Oct 2023 09:05:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20010 ) Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying .. Patch Set 15: (2 comments) Thank you for the comments! Published PS15 and answered your questions/comments. http://gerrit.cloudera.org:8080/#/c/20010/15/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/20010/15/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@411 PS15, Line 411: RESULTS > In this test couldn't you assert on the exact values instead of checking re I don't think I can test it without using the same code path to generate the expected results. Also, I cannot pre-generate a table, because the metadata gets malformed when using avrotools (comment on PS7). http://gerrit.cloudera.org:8080/#/c/20010/15/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@416 PS15, Line 416: > Can we add a few more complex queries from https://www.apachecon.com/acna20 This commit does not resolve the nested type columns. I can add more complex queries with the nested type sub-task, IMPALA-12205. -- To view, visit http://gerrit.cloudera.org:8080/20010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 Gerrit-Change-Number: 20010 Gerrit-PatchSet: 15 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 24 Oct 2023 08:46:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying
Tamas Mate has uploaded a new patch set (#15). ( http://gerrit.cloudera.org:8080/20010 ) Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying .. IMPALA-11996: Scanner change for Iceberg metadata querying This commit adds a scan node for querying Iceberg metadata tables. The scan node creates a Java scanner object that creates and scans the metadata table. The scanner uses the Iceberg API to scan the table after that the scan node fetches the rows one by one and materialises them into RowBatches. The Iceberg row reader on the backend does the translation between Iceberg and Impala types. There is only one fragment created to query the Iceberg metadata table which is supposed to be executed on the coordinator node that already has the Iceberg table loaded. This way there is no need for further table loading on the executor side. This change will not cover nested column types, these slots are set to NULL, it will be done in IMPALA-12205. Testing: - Added e2e tests for querying metadata tables - Updated planner tests Performance testing: Created a table and inserted ~5500 rows one by one, this generated ~27 ALL_MANIFESTS metadata table records. This table is quite wide and has a String column as well. I only mention count(*) test on ALL_MANIFESTS, because every row is materialized in every scenario currently: - Cold cache: 15.76s - IcebergApiScanTime: 124.407ms - MaterializeTupleTime: 8s368ms - Warm cache: 7.56s - IcebergApiScanTime: 3.646ms - MaterializeTupleTime: 7s477ms Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 --- M be/CMakeLists.txt M be/src/exec/CMakeLists.txt M be/src/exec/exec-node.cc A be/src/exec/iceberg-metadata/CMakeLists.txt A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h A be/src/exec/iceberg-metadata/iceberg-row-reader.cc A be/src/exec/iceberg-metadata/iceberg-row-reader.h M be/src/scheduling/scheduler.cc M be/src/service/frontend.cc M be/src/service/frontend.h M be/src/service/impalad-main.cc M be/src/util/jni-util.cc M be/src/util/jni-util.h M common/thrift/PlanNodes.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M 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/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/IcebergMetadataScanNode.java M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/datasets/functional/functional_schema_template.sql M testdata/datasets/functional/schema_constraints.csv M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test M tests/authorization/test_ranger.py M tests/query_test/test_iceberg.py 32 files changed, 1,419 insertions(+), 167 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/20010/15 -- To view, visit http://gerrit.cloudera.org:8080/20010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 Gerrit-Change-Number: 20010 Gerrit-PatchSet: 15 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying
Tamas Mate has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/20010 ) Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying .. IMPALA-11996: Scanner change for Iceberg metadata querying This commit adds a scan node for querying Iceberg metadata tables. The scan node creates a Java scanner object that creates and scans the metadata table. The scanner uses the Iceberg API to scan the table after that the scan node fetches the rows one by one and materialises them into RowBatches. The Iceberg row reader on the backend does the translation between Iceberg and Impala types. There is only one fragment created to query the Iceberg metadata table which is supposed to be executed on the coordinator node that already has the Iceberg table loaded. This way there is no need for further table loading on the executor side. This change will not cover struct column types, these slots are set to NULL, it will be done in IMPALA-12205. Testing: - Added e2e tests for querying metadata tables - Updated planner tests Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 --- M be/CMakeLists.txt M be/src/exec/CMakeLists.txt M be/src/exec/exec-node.cc A be/src/exec/iceberg-metadata/CMakeLists.txt A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h A be/src/exec/iceberg-metadata/iceberg-row-reader.cc A be/src/exec/iceberg-metadata/iceberg-row-reader.h M be/src/scheduling/scheduler.cc M be/src/service/frontend.cc M be/src/service/frontend.h M be/src/service/impalad-main.cc M be/src/util/jni-util.cc M be/src/util/jni-util.h M common/thrift/PlanNodes.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M 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/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/IcebergMetadataScanNode.java M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/datasets/functional/functional_schema_template.sql M testdata/datasets/functional/schema_constraints.csv M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test M tests/query_test/test_iceberg.py 31 files changed, 1,358 insertions(+), 175 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/20010/10 -- To view, visit http://gerrit.cloudera.org:8080/20010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 Gerrit-Change-Number: 20010 Gerrit-PatchSet: 10 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying
Tamas Mate has uploaded a new patch set (#14). ( http://gerrit.cloudera.org:8080/20010 ) Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying .. IMPALA-11996: Scanner change for Iceberg metadata querying This commit adds a scan node for querying Iceberg metadata tables. The scan node creates a Java scanner object that creates and scans the metadata table. The scanner uses the Iceberg API to scan the table after that the scan node fetches the rows one by one and materialises them into RowBatches. The Iceberg row reader on the backend does the translation between Iceberg and Impala types. There is only one fragment created to query the Iceberg metadata table which is supposed to be executed on the coordinator node that already has the Iceberg table loaded. This way there is no need for further table loading on the executor side. This change will not cover nested column types, these slots are set to NULL, it will be done in IMPALA-12205. Testing: - Added e2e tests for querying metadata tables - Updated planner tests Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 --- M be/CMakeLists.txt M be/src/exec/CMakeLists.txt M be/src/exec/exec-node.cc A be/src/exec/iceberg-metadata/CMakeLists.txt A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h A be/src/exec/iceberg-metadata/iceberg-row-reader.cc A be/src/exec/iceberg-metadata/iceberg-row-reader.h M be/src/scheduling/scheduler.cc M be/src/service/frontend.cc M be/src/service/frontend.h M be/src/service/impalad-main.cc M be/src/util/jni-util.cc M be/src/util/jni-util.h M common/thrift/PlanNodes.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M 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/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/IcebergMetadataScanNode.java M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/datasets/functional/functional_schema_template.sql M testdata/datasets/functional/schema_constraints.csv M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test M tests/authorization/test_ranger.py M tests/query_test/test_iceberg.py 32 files changed, 1,393 insertions(+), 166 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/20010/14 -- To view, visit http://gerrit.cloudera.org:8080/20010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 Gerrit-Change-Number: 20010 Gerrit-PatchSet: 14 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying
Tamas Mate has posted comments on this change. (
http://gerrit.cloudera.org:8080/20010 )
Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..
Patch Set 14:
(8 comments)
Thank you for the review Zoltan, updated the change.
http://gerrit.cloudera.org:8080/#/c/20010/13/be/src/exec/iceberg-metadata/iceberg-row-reader.h
File be/src/exec/iceberg-metadata/iceberg-row-reader.h:
http://gerrit.cloudera.org:8080/#/c/20010/13/be/src/exec/iceberg-metadata/iceberg-row-reader.h@20
PS13, Line 20: #include
> Seems like we can remove this.
Done
http://gerrit.cloudera.org:8080/#/c/20010/13/be/src/exec/iceberg-metadata/iceberg-row-reader.cc
File be/src/exec/iceberg-metadata/iceberg-row-reader.cc:
http://gerrit.cloudera.org:8080/#/c/20010/13/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@104
PS13, Line 104: }
: }
: return Status::OK();
: }
:
: Status IcebergRowReader::WriteBooleanSlot(JNIEnv* env,
:
> These lines are common in all Read methods. It might make sense to put them
I moved it to MaterializeRow and renamed the Read methods.
http://gerrit.cloudera.org:8080/#/c/20010/13/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@111
PS13, Line 111: java_boolean_c
> Can we check somehow that accessed_value has the expected type?
I could DCHECK an IsInstanceOf call.
http://gerrit.cloudera.org:8080/#/c/20010/13/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
File fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java:
http://gerrit.cloudera.org:8080/#/c/20010/13/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java@414
PS13, Line 414: UNPARTITIONED
> Why are we using UNPARTITIONED instead of RANDOM?
This is for the scheduler, to execute the fragment on the coordinator.
scheduler.cc:171
http://gerrit.cloudera.org:8080/#/c/20010/13/fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java
File fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java:
http://gerrit.cloudera.org:8080/#/c/20010/13/fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java@76
PS13, Line 76:* Creates the Metadata{Table} which is a predifined Iceberg
{Table} object. This method
:* also starts an Iceberg {TableScan} to scan the {Table}.
After the scan is ready it
:* initializes the iterators, so the {GetNext} call can start
fetching the rows through
:* the Iceberg Api.
:*/
> nit: Almost the same as L102-L106. Can we refactor this?
Done
http://gerrit.cloudera.org:8080/#/c/20010/13/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/20010/13/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@6
PS13, Line 6: metadata t
> nit: metadata
Done
http://gerrit.cloudera.org:8080/#/c/20010/13/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@6
PS13, Line 6: Query all the metadata tables once
> Is there a list somewhere about all the metadata tables?
Only in the Iceberg codebase at the moment.
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/MetadataTableType.java
http://gerrit.cloudera.org:8080/#/c/20010/13/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:
http://gerrit.cloudera.org:8080/#/c/20010/13/tests/authorization/test_ranger.py@2048
PS13, Line 2048: the metad
> nit: the metadata tables
Done
--
To view, visit http://gerrit.cloudera.org:8080/20010
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7
Gerrit-Change-Number: 20010
Gerrit-PatchSet: 14
Gerrit-Owner: Tamas Mate
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Gabor Kaszab
Gerrit-Reviewer: Gergely Fürnstáhl
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Peter Rozsa
Gerrit-Reviewer: Tamas Mate
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Thu, 19 Oct 2023 12:55:01 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12308: DIRECTED distribution mode for V2 Iceberg tables
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20548 ) Change subject: IMPALA-12308: DIRECTED distribution mode for V2 Iceberg tables .. Patch Set 4: (6 comments) Great change Gabor, I left some nits. http://gerrit.cloudera.org:8080/#/c/20548/4/be/src/runtime/krpc-data-stream-sender.cc File be/src/runtime/krpc-data-stream-sender.cc: http://gerrit.cloudera.org:8080/#/c/20548/4/be/src/runtime/krpc-data-stream-sender.cc@1090 PS4, Line 1090: than nit: then http://gerrit.cloudera.org:8080/#/c/20548/4/be/src/runtime/query-state.h File be/src/runtime/query-state.h: http://gerrit.cloudera.org:8080/#/c/20548/4/be/src/runtime/query-state.h@123 PS4, Line 123: egzact nit: exact http://gerrit.cloudera.org:8080/#/c/20548/4/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java File fe/src/main/java/org/apache/impala/planner/ExchangeNode.java: http://gerrit.cloudera.org:8080/#/c/20548/4/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java@77 PS4, Line 77: give nit: return http://gerrit.cloudera.org:8080/#/c/20548/4/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java File fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java: http://gerrit.cloudera.org:8080/#/c/20548/4/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@74 PS4, Line 74: MultiAggregateInfo aggInfo, List fileDescs, : nit: indentation http://gerrit.cloudera.org:8080/#/c/20548/4/fe/src/main/java/org/apache/impala/planner/JoinNode.java File fe/src/main/java/org/apache/impala/planner/JoinNode.java: http://gerrit.cloudera.org:8080/#/c/20548/4/fe/src/main/java/org/apache/impala/planner/JoinNode.java@219 PS4, Line 219: nit: indentation http://gerrit.cloudera.org:8080/#/c/20548/4/testdata/workloads/functional-query/queries/QueryTest/set.test File testdata/workloads/functional-query/queries/QueryTest/set.test: http://gerrit.cloudera.org:8080/#/c/20548/4/testdata/workloads/functional-query/queries/QueryTest/set.test@135 PS4, Line 135: DIRECTED This can be confusing, because setting this has no effect on the distribution mode. Could we hide it or create doc jira to explain it for the user? -- To view, visit http://gerrit.cloudera.org:8080/20548 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I212afd7c9e94551a1c50a40ccb0e3c1f7ecdf3d2 Gerrit-Change-Number: 20548 Gerrit-PatchSet: 4 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 18 Oct 2023 12:28:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11853: Fix formatted docs query options CSS
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20556 ) Change subject: IMPALA-11853: Fix formatted docs query options CSS .. Patch Set 1: Code-Review+2 Thank you for the reviews! -- To view, visit http://gerrit.cloudera.org:8080/20556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icec787d7a2af848aaaff65be2ecf311a5ce8fe7f Gerrit-Change-Number: 20556 Gerrit-PatchSet: 1 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Wed, 18 Oct 2023 10:21:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11853: Fix formatted docs query options CSS
Tamas Mate has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20556 ) Change subject: IMPALA-11853: Fix formatted docs query options CSS .. IMPALA-11853: Fix formatted docs query options CSS The recent documentation formatting changes introduced the navigation panel on the left. However, due to the length of the query options navigation title these could overlap with the documentation paragraphs. This commit removes the underscores from the navigation titles of the query options, so browsers can break them into multiple lines. Additionally, the "SET" and "Query Options for the SET Statement" pages are merged to save some more space for the query option navigation titles. Testing: - Built the documentation and tested manually Change-Id: Icec787d7a2af848aaaff65be2ecf311a5ce8fe7f Reviewed-on: http://gerrit.cloudera.org:8080/20556 Tested-by: Impala Public Jenkins Reviewed-by: Jason Fehr Reviewed-by: Peter Rozsa Reviewed-by: Tamas Mate --- M docs/impala.ditamap M docs/impala_keydefs.ditamap M docs/topics/impala_abort_on_error.xml M docs/topics/impala_allow_erasure_coded_files.xml M docs/topics/impala_allow_unsupported_formats.xml M docs/topics/impala_appx_count_distinct.xml M docs/topics/impala_batch_size.xml M docs/topics/impala_broadcast_bytes_limit.xml M docs/topics/impala_buffer_pool_limit.xml M docs/topics/impala_compression_codec.xml M docs/topics/impala_compute_stats_min_sample_size.xml M docs/topics/impala_config_options.xml M docs/topics/impala_debug_action.xml M docs/topics/impala_decimal_v2.xml M docs/topics/impala_default_file_format.xml M docs/topics/impala_default_hints_insert_statement.xml M docs/topics/impala_default_join_distribution_mode.xml M docs/topics/impala_default_spillable_buffer_size.xml M docs/topics/impala_default_transactional_type.xml M docs/topics/impala_delete_stats_in_truncate.xml M docs/topics/impala_disable_codegen.xml M docs/topics/impala_disable_codegen_rows_threshold.xml M docs/topics/impala_disable_hbase_num_rows_estimate.xml M docs/topics/impala_disable_row_runtime_filtering.xml M docs/topics/impala_disable_streaming_preaggregations.xml M docs/topics/impala_disable_unsafe_spills.xml M docs/topics/impala_enable_expr_rewrites.xml M docs/topics/impala_exec_single_node_rows_threshold.xml M docs/topics/impala_exec_time_limit_s.xml M docs/topics/impala_expand_complex_types.xml M docs/topics/impala_explain_level.xml M docs/topics/impala_fetch_rows_timeout_ms.xml M docs/topics/impala_hbase_cache_blocks.xml M docs/topics/impala_hbase_caching.xml M docs/topics/impala_idle_session_timeout.xml M docs/topics/impala_join_rows_produced_limit.xml M docs/topics/impala_kudu_read_mode.xml M docs/topics/impala_live_progress.xml M docs/topics/impala_live_summary.xml M docs/topics/impala_max_errors.xml M docs/topics/impala_max_mem_estimate_for_admission.xml M docs/topics/impala_max_num_runtime_filters.xml M docs/topics/impala_max_result_spooling_mem.xml M docs/topics/impala_max_row_size.xml M docs/topics/impala_max_scan_range_length.xml M docs/topics/impala_max_spilled_result_spooling_mem.xml M docs/topics/impala_mem_limit.xml M docs/topics/impala_min_spillable_buffer_size.xml M docs/topics/impala_mt_dop.xml M docs/topics/impala_num_nodes.xml M docs/topics/impala_num_rows_produced_limit.xml M docs/topics/impala_num_scanner_threads.xml M docs/topics/impala_optimize_partition_key_scans.xml M docs/topics/impala_parquet_annotate_strings_utf8.xml M docs/topics/impala_parquet_array_resolution.xml M docs/topics/impala_parquet_compression_codec.xml M docs/topics/impala_parquet_dictionary_filtering.xml M docs/topics/impala_parquet_fallback_schema_resolution.xml M docs/topics/impala_parquet_file_size.xml M docs/topics/impala_parquet_object_store_split_size.xml M docs/topics/impala_parquet_page_row_count_limit.xml M docs/topics/impala_parquet_read_page_index.xml M docs/topics/impala_parquet_read_statistics.xml M docs/topics/impala_parquet_write_page_index.xml M docs/topics/impala_prefetch_mode.xml D docs/topics/impala_query_options.xml M docs/topics/impala_query_timeout_s.xml M docs/topics/impala_refresh_updated_hms.xml M docs/topics/impala_replica_preference.xml M docs/topics/impala_request_pool.xml M docs/topics/impala_resource_trace_ratio.xml M docs/topics/impala_retry_failed_queries.xml M docs/topics/impala_runtime_bloom_filter_size.xml M docs/topics/impala_runtime_filter_max_size.xml M docs/topics/impala_runtime_filter_min_size.xml M docs/topics/impala_runtime_filter_mode.xml M docs/topics/impala_runtime_filter_wait_time_ms.xml M docs/topics/impala_s3_skip_insert_staging.xml M docs/topics/impala_scan_bytes_limit.xml M docs/topics/impala_schedule_random_replica.xml M docs/topics/impala_scratch_limit.xml M docs/topics/impala_seq_compression_mode.xml M docs/topics/impala_set.xml M docs/topics/impala_shell_commands.xml M docs/topics/impala_shuffle_distinct_exprs.xml M docs/topics/impala_spool_quer
[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying
Tamas Mate has uploaded a new patch set (#13). ( http://gerrit.cloudera.org:8080/20010 ) Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying .. IMPALA-11996: Scanner change for Iceberg metadata querying This commit adds a scan node for querying Iceberg metadata tables. The scan node creates a Java scanner object that creates and scans the metadata table. The scanner uses the Iceberg API to scan the table after that the scan node fetches the rows one by one and materialises them into RowBatches. The Iceberg row reader on the backend does the translation between Iceberg and Impala types. There is only one fragment created to query the Iceberg metadata table which is supposed to be executed on the coordinator node that already has the Iceberg table loaded. This way there is no need for further table loading on the executor side. This change will not cover nested column types, these slots are set to NULL, it will be done in IMPALA-12205. Testing: - Added e2e tests for querying metadata tables - Updated planner tests Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 --- M be/CMakeLists.txt M be/src/exec/CMakeLists.txt M be/src/exec/exec-node.cc A be/src/exec/iceberg-metadata/CMakeLists.txt A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h A be/src/exec/iceberg-metadata/iceberg-row-reader.cc A be/src/exec/iceberg-metadata/iceberg-row-reader.h M be/src/scheduling/scheduler.cc M be/src/service/frontend.cc M be/src/service/frontend.h M be/src/service/impalad-main.cc M be/src/util/jni-util.cc M be/src/util/jni-util.h M common/thrift/PlanNodes.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M 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/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/IcebergMetadataScanNode.java M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/datasets/functional/functional_schema_template.sql M testdata/datasets/functional/schema_constraints.csv M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test M tests/authorization/test_ranger.py M tests/query_test/test_iceberg.py 32 files changed, 1,420 insertions(+), 166 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/20010/13 -- To view, visit http://gerrit.cloudera.org:8080/20010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 Gerrit-Change-Number: 20010 Gerrit-PatchSet: 13 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20010 ) Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying .. Patch Set 13: Fixed clang tidy errors. -- To view, visit http://gerrit.cloudera.org:8080/20010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 Gerrit-Change-Number: 20010 Gerrit-PatchSet: 13 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Sun, 15 Oct 2023 15:09:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying
Tamas Mate has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/20010 ) Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying .. IMPALA-11996: Scanner change for Iceberg metadata querying This commit adds a scan node for querying Iceberg metadata tables. The scan node creates a Java scanner object that creates and scans the metadata table. The scanner uses the Iceberg API to scan the table after that the scan node fetches the rows one by one and materialises them into RowBatches. The Iceberg row reader on the backend does the translation between Iceberg and Impala types. There is only one fragment created to query the Iceberg metadata table which is supposed to be executed on the coordinator node that already has the Iceberg table loaded. This way there is no need for further table loading on the executor side. This change will not cover nested column types, these slots are set to NULL, it will be done in IMPALA-12205. Testing: - Added e2e tests for querying metadata tables - Updated planner tests Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 --- M be/CMakeLists.txt M be/src/exec/CMakeLists.txt M be/src/exec/exec-node.cc A be/src/exec/iceberg-metadata/CMakeLists.txt A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h A be/src/exec/iceberg-metadata/iceberg-row-reader.cc A be/src/exec/iceberg-metadata/iceberg-row-reader.h M be/src/scheduling/scheduler.cc M be/src/service/frontend.cc M be/src/service/frontend.h M be/src/service/impalad-main.cc M be/src/util/jni-util.cc M be/src/util/jni-util.h M common/thrift/PlanNodes.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M 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/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/IcebergMetadataScanNode.java M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/datasets/functional/functional_schema_template.sql M testdata/datasets/functional/schema_constraints.csv M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test M tests/authorization/test_ranger.py M tests/query_test/test_iceberg.py 32 files changed, 1,419 insertions(+), 166 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/20010/12 -- To view, visit http://gerrit.cloudera.org:8080/20010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 Gerrit-Change-Number: 20010 Gerrit-PatchSet: 12 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20010 ) Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/20010/11/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/20010/11/tests/authorization/test_ranger.py@2060 PS11, Line 2060: = > flake8: E712 comparison to True should be 'if cond is True:' or 'if cond:' Done -- To view, visit http://gerrit.cloudera.org:8080/20010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 Gerrit-Change-Number: 20010 Gerrit-PatchSet: 11 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Sun, 15 Oct 2023 12:41:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying
Tamas Mate has posted comments on this change. (
http://gerrit.cloudera.org:8080/20010 )
Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying
..
Patch Set 11:
(59 comments)
Hi Peter and Gabor, thank you for the detailed review, updated the change. Let
me know your thoughts.
http://gerrit.cloudera.org:8080/#/c/20010/10//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20010/10//COMMIT_MSG@21
PS10, Line 21: nested column types
> You still only mention struct type, however, other nested types are affecte
Done
http://gerrit.cloudera.org:8080/#/c/20010/10//COMMIT_MSG@24
PS10, Line 24: Testing:
> I do recall something about a perf test to see how long it takes to query s
It was quite slow to build a table like that so I just skipped it for now.
http://gerrit.cloudera.org:8080/#/c/20010/10//COMMIT_MSG@25
PS10, Line 25: - Added e2e tests for querying metadata tables
> I'd really like to see some auth tests as well to see that changing privile
Done
http://gerrit.cloudera.org:8080/#/c/20010/9/be/src/exec/exec-node.cc
File be/src/exec/exec-node.cc:
http://gerrit.cloudera.org:8080/#/c/20010/9/be/src/exec/exec-node.cc@55
PS9, Line 55: #include "exec/unnest-node.h"
> nit: could go to L41
Done
http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
File be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h:
http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h@33
PS10, Line 33: /// Iceberg API provides predefined metadata tables, these
tables can be scanned through
> I really like this description of the ScanNode. Thanks!
Thanks!
http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h@35
PS10, Line 35: ner to scan Iceberg data, due to the virtua
> nit: "just like any other regular Iceberg tables"?
Done
http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h@39
PS10, Line 39: s
> nit: 'an'
Done
http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h@69
PS10, Line 69: SED_RESULT;
> Would it cause any harm to call this multiple times? If yes, can we return
Done
http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h@89
PS10, Line 89: scan_metadata_table_ = nullptr
> nit: this is the constructor, right? Can we name this 'iceberg_metadata_sca
Done
http://gerrit.cloudera.org:8080/#/c/20010/9/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h
File be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h:
http://gerrit.cloudera.org:8080/#/c/20010/9/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h@23
PS9, Line 23: #include
> Unused include
Done
http://gerrit.cloudera.org:8080/#/c/20010/9/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h@35
PS9, Line 35: /// its Parquet scanner to scan Iceberg data, due to the virtual
nature of the metadata
> nit: Parquet
Done
http://gerrit.cloudera.org:8080/#/c/20010/9/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
File be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc:
http://gerrit.cloudera.org:8080/#/c/20010/9/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc@41
PS9, Line 41:
> nit: ; not needed
Done
http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc
File be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc:
http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc@71
PS10, Line 71: if (tuple_desc_ == nullptr) {
> Could you include more info here for easier debugging, e.g. tuple Id?
Done
http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc@90
PS10, Line 90: d_global_ref;
> I see aboce that after calling this function you also called "RETURN_ERROR_
RETURN_ERROR_IF_EXC checks the environment, LocalToGlobalRef is a wrapper
around the JNI NewGlobalRef and env is checked in it. At this point, in case
there is an exception it is already an Impala Status.
http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc@131
PS10, Line 131: row_batch->tuple_data_pool(
> You've already checked for nullness few lines above.
Done
http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc@165
PS10, Line 165:
> You don't use 'env' in this function. Anyway, Frontend::GetCatalogTable() c
Done
http://gerrit.cloudera.org:8080/#/c/20010/10/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc@168
PS10, Line 168: RETURN_IF_ERROR(fe->GetCatalogTable(table_name_, jtable));
> Should we check if 'jtable' is nullptr after this call or would we get an e
Good point, added a DCHECK t
[Impala-ASF-CR] IMPALA-11996: Scanner change for Iceberg metadata querying
Tamas Mate has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/20010 ) Change subject: IMPALA-11996: Scanner change for Iceberg metadata querying .. IMPALA-11996: Scanner change for Iceberg metadata querying This commit adds a scan node for querying Iceberg metadata tables. The scan node creates a Java scanner object that creates and scans the metadata table. The scanner uses the Iceberg API to scan the table after that the scan node fetches the rows one by one and materialises them into RowBatches. The Iceberg row reader on the backend does the translation between Iceberg and Impala types. There is only one fragment created to query the Iceberg metadata table which is supposed to be executed on the coordinator node that already has the Iceberg table loaded. This way there is no need for further table loading on the executor side. This change will not cover nested column types, these slots are set to NULL, it will be done in IMPALA-12205. Testing: - Added e2e tests for querying metadata tables - Updated planner tests Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 --- M be/CMakeLists.txt M be/src/exec/CMakeLists.txt M be/src/exec/exec-node.cc A be/src/exec/iceberg-metadata/CMakeLists.txt A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.cc A be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h A be/src/exec/iceberg-metadata/iceberg-row-reader.cc A be/src/exec/iceberg-metadata/iceberg-row-reader.h M be/src/scheduling/scheduler.cc M be/src/service/frontend.cc M be/src/service/frontend.h M be/src/service/impalad-main.cc M be/src/util/jni-util.cc M be/src/util/jni-util.h M common/thrift/PlanNodes.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M 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/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/IcebergMetadataScanNode.java M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M testdata/datasets/functional/functional_schema_template.sql M testdata/datasets/functional/schema_constraints.csv M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-metadata-table-scan.test M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test M tests/authorization/test_ranger.py M tests/query_test/test_iceberg.py 32 files changed, 1,420 insertions(+), 168 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/20010/11 -- To view, visit http://gerrit.cloudera.org:8080/20010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e943cecd77f5ef7af7cd07e2b596f2c5b4331e7 Gerrit-Change-Number: 20010 Gerrit-PatchSet: 11 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Gergely Fürnstáhl Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-11853: Fix formatted docs query options CSS
Tamas Mate has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20556 Change subject: IMPALA-11853: Fix formatted docs query options CSS .. IMPALA-11853: Fix formatted docs query options CSS The recent documentation formatting changes introduced the navigation panel on the left. However, due to the length of the query options navigation title these could overlap with the documentation paragraphs. This commit removes the underscores from the navigation titles of the query options, so browsers can break them into multiple lines. Additionally, the "SET" and "Query Options for the SET Statement" pages are merged to save some more space for the query option navigation titles. Testing: - Built the documentation and tested manually Change-Id: Icec787d7a2af848aaaff65be2ecf311a5ce8fe7f --- M docs/impala.ditamap M docs/impala_keydefs.ditamap M docs/topics/impala_abort_on_error.xml M docs/topics/impala_allow_erasure_coded_files.xml M docs/topics/impala_allow_unsupported_formats.xml M docs/topics/impala_appx_count_distinct.xml M docs/topics/impala_batch_size.xml M docs/topics/impala_broadcast_bytes_limit.xml M docs/topics/impala_buffer_pool_limit.xml M docs/topics/impala_compression_codec.xml M docs/topics/impala_compute_stats_min_sample_size.xml M docs/topics/impala_config_options.xml M docs/topics/impala_debug_action.xml M docs/topics/impala_decimal_v2.xml M docs/topics/impala_default_file_format.xml M docs/topics/impala_default_hints_insert_statement.xml M docs/topics/impala_default_join_distribution_mode.xml M docs/topics/impala_default_spillable_buffer_size.xml M docs/topics/impala_default_transactional_type.xml M docs/topics/impala_delete_stats_in_truncate.xml M docs/topics/impala_disable_codegen.xml M docs/topics/impala_disable_codegen_rows_threshold.xml M docs/topics/impala_disable_hbase_num_rows_estimate.xml M docs/topics/impala_disable_row_runtime_filtering.xml M docs/topics/impala_disable_streaming_preaggregations.xml M docs/topics/impala_disable_unsafe_spills.xml M docs/topics/impala_enable_expr_rewrites.xml M docs/topics/impala_exec_single_node_rows_threshold.xml M docs/topics/impala_exec_time_limit_s.xml M docs/topics/impala_expand_complex_types.xml M docs/topics/impala_explain_level.xml M docs/topics/impala_fetch_rows_timeout_ms.xml M docs/topics/impala_hbase_cache_blocks.xml M docs/topics/impala_hbase_caching.xml M docs/topics/impala_idle_session_timeout.xml M docs/topics/impala_join_rows_produced_limit.xml M docs/topics/impala_kudu_read_mode.xml M docs/topics/impala_live_progress.xml M docs/topics/impala_live_summary.xml M docs/topics/impala_max_errors.xml M docs/topics/impala_max_mem_estimate_for_admission.xml M docs/topics/impala_max_num_runtime_filters.xml M docs/topics/impala_max_result_spooling_mem.xml M docs/topics/impala_max_row_size.xml M docs/topics/impala_max_scan_range_length.xml M docs/topics/impala_max_spilled_result_spooling_mem.xml M docs/topics/impala_mem_limit.xml M docs/topics/impala_min_spillable_buffer_size.xml M docs/topics/impala_mt_dop.xml M docs/topics/impala_num_nodes.xml M docs/topics/impala_num_rows_produced_limit.xml M docs/topics/impala_num_scanner_threads.xml M docs/topics/impala_optimize_partition_key_scans.xml M docs/topics/impala_parquet_annotate_strings_utf8.xml M docs/topics/impala_parquet_array_resolution.xml M docs/topics/impala_parquet_compression_codec.xml M docs/topics/impala_parquet_dictionary_filtering.xml M docs/topics/impala_parquet_fallback_schema_resolution.xml M docs/topics/impala_parquet_file_size.xml M docs/topics/impala_parquet_object_store_split_size.xml M docs/topics/impala_parquet_page_row_count_limit.xml M docs/topics/impala_parquet_read_page_index.xml M docs/topics/impala_parquet_read_statistics.xml M docs/topics/impala_parquet_write_page_index.xml M docs/topics/impala_prefetch_mode.xml D docs/topics/impala_query_options.xml M docs/topics/impala_query_timeout_s.xml M docs/topics/impala_refresh_updated_hms.xml M docs/topics/impala_replica_preference.xml M docs/topics/impala_request_pool.xml M docs/topics/impala_resource_trace_ratio.xml M docs/topics/impala_retry_failed_queries.xml M docs/topics/impala_runtime_bloom_filter_size.xml M docs/topics/impala_runtime_filter_max_size.xml M docs/topics/impala_runtime_filter_min_size.xml M docs/topics/impala_runtime_filter_mode.xml M docs/topics/impala_runtime_filter_wait_time_ms.xml M docs/topics/impala_s3_skip_insert_staging.xml M docs/topics/impala_scan_bytes_limit.xml M docs/topics/impala_schedule_random_replica.xml M docs/topics/impala_scratch_limit.xml M docs/topics/impala_seq_compression_mode.xml M docs/topics/impala_set.xml M docs/topics/impala_shell_commands.xml M docs/topics/impala_shuffle_distinct_exprs.xml M docs/topics/impala_spool_query_results.xml M docs/topics/impala_support_start_over.xml M docs/topics/impala_sync_ddl.xml M docs/topics/impala_thread_reservation_aggregate_limit.xml M docs/topics/impa
[Impala-ASF-CR] IMPALA-12243: Add support for DROP PARTITION for Iceberg tables
Tamas Mate has posted comments on this change. (
http://gerrit.cloudera.org:8080/20515 )
Change subject: IMPALA-12243: Add support for DROP PARTITION for Iceberg tables
..
Patch Set 2:
(6 comments)
Added some more comments.
http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
File
fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java:
http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java@131
PS2, Line 131: if (purgePartition_) {
: throw new AnalysisException("Iceberg tables can't purge
partitions");
: }
I couldn't find a test for this code path.
http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java@146
PS2, Line 146: if (icebergExpression == null) {
: throw new AnalysisException(
: "Invalid partition filtering expression: " +
expr.toSql());
: }
I think it would be cleaner to propagate an exception instead of relying on
null results.
http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java
File fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java:
http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/IcebergPartitionExpr.java@41
PS2, Line 41:
nit: empty line or missing comments
http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/common/IcebergPartitionPredicateConverter.java
File
fe/src/main/java/org/apache/impala/common/IcebergPartitionPredicateConverter.java:
http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/common/IcebergPartitionPredicateConverter.java@114
PS2, Line 114: }
nit: missing empty line
http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java
File fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java:
http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@152
PS2, Line 152:
nit: indentation
http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@246
PS2, Line 246: + "backend handle it.",
: ex);
nit: no need for line break.
--
To view, visit http://gerrit.cloudera.org:8080/20515
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a768ba2966f570454687e02e4e6d67df46741f9
Gerrit-Change-Number: 20515
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa
Gerrit-Reviewer: Gabor Kaszab
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Peter Rozsa
Gerrit-Reviewer: Tamas Mate
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Thu, 05 Oct 2023 12:37:23 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12279: Bump Iceberg lib version to 1.3
Tamas Mate has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20529 ) Change subject: IMPALA-12279: Bump Iceberg lib version to 1.3 .. IMPALA-12279: Bump Iceberg lib version to 1.3 This bumps the toolchain version to upgrade the Iceberg library. Testing: - Verified building Impala on Ubuntu 18 and ran the Iceberg tests. Change-Id: Ieece75dc9764c691917c8ed052d1a7f9cc950ec2 Reviewed-on: http://gerrit.cloudera.org:8080/20529 Tested-by: Impala Public Jenkins Reviewed-by: Zoltan Borok-Nagy --- M bin/impala-config.sh 1 file changed, 11 insertions(+), 11 deletions(-) Approvals: Impala Public Jenkins: Verified Zoltan Borok-Nagy: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/20529 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ieece75dc9764c691917c8ed052d1a7f9cc950ec2 Gerrit-Change-Number: 20529 Gerrit-PatchSet: 2 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12243: Add support for DROP PARTITION for Iceberg tables
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20515 ) Change subject: IMPALA-12243: Add support for DROP PARTITION for Iceberg tables .. Patch Set 2: (5 comments) Hi Peter, nice change! Just did a quick first check. I will be back later today. http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java File fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java: http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java@62 PS2, Line 62: /** :* Stores files selected by Iceberg's partition filtering :*/ nit: single-line comment http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java@66 PS2, Line 66: /** :* Statistics for selected Iceberg partitions :*/ nit: single-line comment http://gerrit.cloudera.org:8080/#/c/20515/2/fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java@114 PS2, Line 114: this.analyzer_ = analyzer; Do we need this for Alter statements? StmtBase does this but AlterTableStmt overwrites it. http://gerrit.cloudera.org:8080/#/c/20515/2/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/20515/2/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@401 PS2, Line 401: AnalysisException I believe, this is a bit confusing, we should not throw an AnalysisException during table load. I am still looking into what made this change necessary. http://gerrit.cloudera.org:8080/#/c/20515/2/tests/query_test/test_iceberg.py File tests/query_test/test_iceberg.py: http://gerrit.cloudera.org:8080/#/c/20515/2/tests/query_test/test_iceberg.py@1125 PS2, Line 1125: nit: empty line -- To view, visit http://gerrit.cloudera.org:8080/20515 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a768ba2966f570454687e02e4e6d67df46741f9 Gerrit-Change-Number: 20515 Gerrit-PatchSet: 2 Gerrit-Owner: Peter Rozsa Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 03 Oct 2023 10:09:24 + Gerrit-HasComments: Yes
