Mihaly Szjatinya has posted comments on this change. ( http://gerrit.cloudera.org:8080/21761 )
Change subject: IMPALA-13364: Schema resolution doesn't work for migrated partitioned Iceberg tables that have complex types ...................................................................... Patch Set 6: (3 comments) Checked the code and run tests. Left mostly minor comments for now. Just one thing not directly related to this patch, but exploring the code around schema resolver I found a lot of iceberg (table format) specific ramifications across parquet and orc (file format) code. This would definitely require better familiarity with the area, so take it for what it is, but I wonder, would it be feasible to think in the direction of decoupling iceberg related code at some point? (which would largely affect this patch among other things). http://gerrit.cloudera.org:8080/#/c/21761/6/be/src/exec/file-metadata-utils.h File be/src/exec/file-metadata-utils.h: http://gerrit.cloudera.org:8080/#/c/21761/6/be/src/exec/file-metadata-utils.h@69 PS6, Line 69: , in any other cases nit: ". In any other case" ? http://gerrit.cloudera.org:8080/#/c/21761/6/be/src/exec/file-metadata-utils.h@87 PS6, Line 87: Only valid to call : /// for Iceberg tables. Can this be reflected in function's name? As it is for other Iceberg specific functions around. http://gerrit.cloudera.org:8080/#/c/21761/6/be/src/exec/file-metadata-utils.h@89 PS6, Line 89: auto Is there a consensus on auto usage? It doesn't seem to be used much around these files. -- To view, visit http://gerrit.cloudera.org:8080/21761 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie32952021b63d6b55b8820489e434bfc2a91580b Gerrit-Change-Number: 21761 Gerrit-PatchSet: 6 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Mihaly Szjatinya <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Sun, 29 Sep 2024 14:08:46 +0000 Gerrit-HasComments: Yes
