Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/24238 )
Change subject: IMPALA-14589: [Part 2] Add comprehensive type support for Iceberg initial-default values ...................................................................... Patch Set 6: Code-Review+1 (5 comments) Looks good! Only found nits and asked for a bit more test coverage for specific values. http://gerrit.cloudera.org:8080/#/c/24238/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24238/6//COMMIT_MSG@23 PS6, Line 23: TIMESTAMPTZ This is not supported anymore. http://gerrit.cloudera.org:8080/#/c/24238/6//COMMIT_MSG@25 PS6, Line 25: BINARY and TIMESTAMPTZ http://gerrit.cloudera.org:8080/#/c/24238/6/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java: http://gerrit.cloudera.org:8080/#/c/24238/6/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@1176 PS6, Line 1176: field.type() instanceof Types.TimestampType It's enough to check this as it already implies isPrimitiveType() http://gerrit.cloudera.org:8080/#/c/24238/6/testdata/data/iceberg_test/iceberg_v3/iceberg_v3_all_types/metadata/v4.metadata.json File testdata/data/iceberg_test/iceberg_v3/iceberg_v3_all_types/metadata/v4.metadata.json: http://gerrit.cloudera.org:8080/#/c/24238/6/testdata/data/iceberg_test/iceberg_v3/iceberg_v3_all_types/metadata/v4.metadata.json@72 PS6, Line 72: "123.45" What if decimal literal is stored as '1E+3'? Can we have another column for that? http://gerrit.cloudera.org:8080/#/c/24238/6/testdata/data/iceberg_test/iceberg_v3/iceberg_v3_all_types/metadata/v4.metadata.json@93 PS6, Line 93: "1969-12-31T23:59:59.123456" Can we have another two column for: - 1970-01-01T00:00:00 - 2024-01-15T10:30:45.123456 -- To view, visit http://gerrit.cloudera.org:8080/24238 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c591d5dbf305176dd802c6f5f811c0de98aac0c Gerrit-Change-Number: 24238 Gerrit-PatchSet: 6 Gerrit-Owner: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Daniel Vanko <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Fri, 08 May 2026 14:43:09 +0000 Gerrit-HasComments: Yes
