Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/23783 )
Change subject: IMPALA-14321: Add BINARY partition transform to Iceberg tables ...................................................................... Patch Set 10: (4 comments) Thanks for working on this! http://gerrit.cloudera.org:8080/#/c/23783/10/be/src/exprs/iceberg-functions-test.cc File be/src/exprs/iceberg-functions-test.cc: http://gerrit.cloudera.org:8080/#/c/23783/10/be/src/exprs/iceberg-functions-test.cc@237 PS10, Line 237: int width = 8; : StringVal ret_val = IcebergFunctions::TruncatePartitionTransform<true>( : ctx, input, IntVal(width)); : EXPECT_EQ(ret_val.len, width); : EXPECT_EQ(memcmp((char*)input.ptr, (char*)ret_val.ptr, width), 0); You could write a for loop with width 1..sizeof(binary_data). http://gerrit.cloudera.org:8080/#/c/23783/10/fe/src/main/java/org/apache/impala/util/IcebergUtil.java File fe/src/main/java/org/apache/impala/util/IcebergUtil.java: http://gerrit.cloudera.org:8080/#/c/23783/10/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@862 PS10, Line 862: StandardCharsets.ISO_8859_1 nit: we could have a static variable CHARSET_FOR_BINARY = StandardCharsets.ISO_8859_1; To make sure we always use the same at all places. http://gerrit.cloudera.org:8080/#/c/23783/10/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@1220 PS10, Line 1220: Convert ByteBuffer to String using Base64 encoding Elsewhere we use StandardCharsets.ISO_8859_1. Any reason to use Base64 encoding here? http://gerrit.cloudera.org:8080/#/c/23783/10/testdata/workloads/functional-query/queries/QueryTest/iceberg-show-files-partition.test File testdata/workloads/functional-query/queries/QueryTest/iceberg-show-files-partition.test: http://gerrit.cloudera.org:8080/#/c/23783/10/testdata/workloads/functional-query/queries/QueryTest/iceberg-show-files-partition.test@315 PS10, Line 315: / Can we include the partition transform value here as well? -- To view, visit http://gerrit.cloudera.org:8080/23783 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5fd1ef382aa064dad55445dea00fbd39caeca1d3 Gerrit-Change-Number: 23783 Gerrit-PatchSet: 10 Gerrit-Owner: Daniel Vanko <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Daniel Vanko <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 13 Jan 2026 16:46:22 +0000 Gerrit-HasComments: Yes
