Arnab Karmakar has posted comments on this change. ( http://gerrit.cloudera.org:8080/24505 )
Change subject: IMPALA-15200: Add Parquet UUID read support for Iceberg tables ...................................................................... Patch Set 7: (9 comments) http://gerrit.cloudera.org:8080/#/c/24505/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24505/6//COMMIT_MSG@26 PS6, Line 26: - Trino > Now it's also possible to add interop tests with Trino, see Done. Added new interop tests for reading uuid type. http://gerrit.cloudera.org:8080/#/c/24505/6/be/src/exec/parquet/parquet-column-readers.cc File be/src/exec/parquet/parquet-column-readers.cc: http://gerrit.cloudera.org:8080/#/c/24505/6/be/src/exec/parquet/parquet-column-readers.cc@325 PS6, Line 325: NeedsConversionInline() const { > Since we switched to C++17 this TODO can be applied. Done http://gerrit.cloudera.org:8080/#/c/24505/6/be/src/exec/parquet/parquet-column-readers.cc@887 PS6, Line 887: rquet::Type::INT64, : true>::DecodeValue<Encoding::PLAIN>(uint8_t** > It would be cleaner to introduce a UuidValue object with an internal std::a Done http://gerrit.cloudera.org:8080/#/c/24505/6/be/src/exec/parquet/parquet-column-readers.cc@895 PS6, Line 895: return false; > fixed_len_size_ is set on Parquet metadata (node.element->type_length). Whi We've now removed the specialization itself. http://gerrit.cloudera.org:8080/#/c/24505/6/be/src/exec/parquet/parquet-data-converter.h File be/src/exec/parquet/parquet-data-converter.h: http://gerrit.cloudera.org:8080/#/c/24505/6/be/src/exec/parquet/parquet-data-converter.h@104 PS6, Line 104: } : if (col_type_->type == TYPE_DECIMAL) { : > Instead of doing a conversion, we could just copy the bytes directly. UUID Done http://gerrit.cloudera.org:8080/#/c/24505/6/be/src/runtime/raw-value.cc File be/src/runtime/raw-value.cc: http://gerrit.cloudera.org:8080/#/c/24505/6/be/src/runtime/raw-value.cc@86 PS6, Line 86: break; : case TYPE_DECIMAL: > This could be merged with TYPE_CHAR: Done http://gerrit.cloudera.org:8080/#/c/24505/6/be/src/service/fe-support.cc File be/src/service/fe-support.cc: http://gerrit.cloudera.org:8080/#/c/24505/6/be/src/service/fe-support.cc@156 PS6, Line 156: // Thrift has no uuid_val, so binary_val holds the 16 raw slot : // bytes. string_val holds the cano > Why is it needed? Please also add a code comment with the reason. Done http://gerrit.cloudera.org:8080/#/c/24505/6/testdata/data/README File testdata/data/README: http://gerrit.cloudera.org:8080/#/c/24505/6/testdata/data/README@1015 PS6, Line 1015: Iceberg V3 Parquet table for UUID read-path tests. > Mention what engine/version you used to create the table. Done http://gerrit.cloudera.org:8080/#/c/24505/6/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/24505/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-uuid-type.test@5 PS6, Line 5: DESCRIBE iceberg_uuid_test > Add test for DESCRIBE FORMATTED as well. Done -- To view, visit http://gerrit.cloudera.org:8080/24505 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4157c002e80677d27d8fd060c7bfa07b95d7c78f Gerrit-Change-Number: 24505 Gerrit-PatchSet: 7 Gerrit-Owner: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Thu, 30 Jul 2026 09:25:05 +0000 Gerrit-HasComments: Yes
