Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21236 )
Change subject: IMPALA-12612: Expand complex type columns from Iceberg metadata tables ...................................................................... Patch Set 1: (5 comments) Thanks Gábor, it looks good, I have only a few remarks. http://gerrit.cloudera.org:8080/#/c/21236/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21236/1//COMMIT_MSG@7 PS1, Line 7: IMPALA-12612: Expand complex type columns from Iceberg metadata tables The title could include "select *", otherwise it's not clear what this refers to. http://gerrit.cloudera.org:8080/#/c/21236/1//COMMIT_MSG@9 PS1, Line 9: , Nit: should come after "behave". http://gerrit.cloudera.org:8080/#/c/21236/1//COMMIT_MSG@14 PS1, Line 14: Note, the behavior of handling nested columns from regular tables We could mention that although this is technically a breaking change, metadata tables are a very recent feature so it is not problematic. http://gerrit.cloudera.org:8080/#/c/21236/1/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/21236/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@25 PS1, Line 25: hdfs://localhost:20500 Is it intentional that these are changed from "$NAMENODE" to "hdfs://localhost:20500"? Applies also to some of the other queries. http://gerrit.cloudera.org:8080/#/c/21236/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@1110 PS1, Line 1110: select readable_metrics.* from functional_parquet.iceberg_query_metadata.`files`; This example can be confusing at first because the 'readable_metrics' struct itself only contains a single struct. I thought first that it failed to expand to the columns "column_size", "value_count" etc. We could either mention it in a comment and/or expand "readable_metrics.i" instead. -- To view, visit http://gerrit.cloudera.org:8080/21236 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia298705ba54411cc439e99d5cb27184093541f02 Gerrit-Change-Number: 21236 Gerrit-PatchSet: 1 Gerrit-Owner: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Wed, 03 Apr 2024 21:52:04 +0000 Gerrit-HasComments: Yes
