Zoltan Borok-Nagy 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 5: Code-Review+1 (3 comments) The change LGTM, left some minor comments. http://gerrit.cloudera.org:8080/#/c/20759/5/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/20759/5/be/src/exec/iceberg-metadata/iceberg-metadata-scan-node.h@127 PS5, Line 127: see CollectStructFieldAccessors for : /// more details. Please update this comment. http://gerrit.cloudera.org:8080/#/c/20759/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/20759/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@496 PS2, Line 496: row_regex:[1-9]\d*|0,1 > Good point, but unfortunately it is not possible with this patch, because b I see, thanks. Maybe we could take a look at what other engines do and implement the best one in a follow-up Jira. Until that probably we could set BINARY values to NULLs (in a follow-up Jira). http://gerrit.cloudera.org:8080/#/c/20759/5/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/5/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@529 PS5, Line 529: Do any of the metadata tables have ARRAYs or MAPs? What happens if the user tries to query them? Can they unnest them with the INNER JOIN syntax? Would be nice to have negative tests for them and proper error messages if such queries are not working. -- 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: 5 Gerrit-Owner: Tamas Mate <tma...@apache.org> 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: Fri, 15 Dec 2023 13:37:53 +0000 Gerrit-HasComments: Yes
