Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/24817 )
Change subject: IMPALA-15202: Add explicit CAST support between UUID and STRING ...................................................................... Patch Set 1: (9 comments) Only had comments about testing, otherwise the code LGTM! http://gerrit.cloudera.org:8080/#/c/24817/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-uuid-type.test File testdata/workloads/functional-query/queries/QueryTest/iceberg-uuid-type.test: http://gerrit.cloudera.org:8080/#/c/24817/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-uuid-type.test@a71 PS1, Line 71: : Please keep a couple old queries (or maybe all) as well, as they test different code paths. http://gerrit.cloudera.org:8080/#/c/24817/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-uuid-type.test@a145 PS1, Line 145: : : : : : : : : : : : : : : : : : : : Can we keep these tests? http://gerrit.cloudera.org:8080/#/c/24817/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-uuid-type.test@a251 PS1, Line 251: : : : : : : : : : : : : : : : : : : : : : : : : : Can we keep these tests? http://gerrit.cloudera.org:8080/#/c/24817/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-uuid-type.test@205 PS1, Line 205: WHERE uuid_col IS NOT DISTINCT FROM (SELECT uuid_col FROM iceberg_uuid_test WHERE id = 4) You could also test it with CAST(NULL AS UUID) http://gerrit.cloudera.org:8080/#/c/24817/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-uuid-type.test@245 PS1, Line 245: SELECT CAST('12345678-1234-5678-1234-567812345678' AS UUID) Can you add tests for casting non-constant strings to UUID? Or maybe: WHERE CAST(CAST(uuid_col AS STRING) AS UUID) = uuid_col http://gerrit.cloudera.org:8080/#/c/24817/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-uuid-type.test@246 PS1, Line 246: ---- RESULTS Uppercase input should be accepted and come back lowercase, e.g. CAST(CAST('ABCDEFAB-…' AS UUID) AS STRING) http://gerrit.cloudera.org:8080/#/c/24817/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-uuid-type.test@248 PS1, Line 248: ---- TYPES Also add test for rejected strings like * no dashes * {…} braces * leading or trailing whitespace * empty string. http://gerrit.cloudera.org:8080/#/c/24817/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-uuid-type.test@249 PS1, Line 249: UUID Add negative tests for: * INSERT INTO iceberg_uuid_test VALUES (10, CAST('…' AS UUID), 'x') * CREATE TABLE t STORED AS PARQUET AS SELECT CAST('…' AS UUID) u http://gerrit.cloudera.org:8080/#/c/24817/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-uuid-type.test@250 PS1, Line 250: ---- HS2_TYPES Can you add positive/negative test for: * CAST(uuid_col AS CHAR(36)) * CAST(uuid_col AS VARCHAR(36)) and vice versa? -- To view, visit http://gerrit.cloudera.org:8080/24817 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I51af55c6e343f7155cae22bf3714e293f31561d5 Gerrit-Change-Number: 24817 Gerrit-PatchSet: 1 Gerrit-Owner: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 15 Sep 2026 09:41:17 +0000 Gerrit-HasComments: Yes
