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 <tma...@apache.org> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tamas Mate <tma...@apache.org> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Thu, 29 Feb 2024 15:23:24 +0000 Gerrit-HasComments: Yes
