Daniel Becker 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:

(6 comments)

Thanks, I finished PS3.

http://gerrit.cloudera.org:8080/#/c/21061/3/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/21061/3/fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java@119
PS3, Line 119: Object
Why isn't this type 'StructLike', like in GetValueByPosition()?


http://gerrit.cloudera.org:8080/#/c/21061/3/fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java@127
PS3, Line 127: classTypeClass
Wouldn't "classType" be enough for the name?


http://gerrit.cloudera.org:8080/#/c/21061/3/fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java@145
PS3, Line 145: iterator != null
Can 'iterator' be NULL? If not, this could be a Preconditions check instead.


http://gerrit.cloudera.org:8080/#/c/21061/3/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/3/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@a566
PS3, Line 566:
Why did we delete this test?


http://gerrit.cloudera.org:8080/#/c/21061/3/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@566
PS3, Line 566: ####
We could have a test that tries to query a MAP.


http://gerrit.cloudera.org:8080/#/c/21061/3/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@611
PS3, Line 611: select equality_ids from (select equality_ids from 
functional_parquet.iceberg_v2_delete_both_eq_and_pos.all_files) s;
We could also have an inline view with a WITH clause.



--
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 <[email protected]>
Gerrit-Reviewer: Daniel Becker <[email protected]>
Gerrit-Reviewer: Gabor Kaszab <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Tue, 27 Feb 2024 10:41:21 +0000
Gerrit-HasComments: Yes

Reply via email to