Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24570 )
Change subject: IMPALA-12675: Set PARQUET_ANNOTATE_STRINGS_UTF8 to true by default and turn on validation ...................................................................... Patch Set 17: Code-Review+1 (6 comments) lgtm, only nits http://gerrit.cloudera.org:8080/#/c/24570/15//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24570/15//COMMIT_MSG@23 PS15, Line 23: CHAR/VARCHAR (always text) > My guess there's no specific reason for this. do the engines above treat string and char/varchar differently? I agree that changing char/varchar can go to a separate patch - can you create a ticket? http://gerrit.cloudera.org:8080/#/c/24570/17/common/thrift/Query.thrift File common/thrift/Query.thrift: http://gerrit.cloudera.org:8080/#/c/24570/17/common/thrift/Query.thrift@273 PS17, Line 273: // Enabled by default since Impala 5.0 (IMPALA-12675). When enabled, string values nit: bit verbose http://gerrit.cloudera.org:8080/#/c/24570/17/tests/query_test/test_datasketches.py File tests/query_test/test_datasketches.py: http://gerrit.cloudera.org:8080/#/c/24570/17/tests/query_test/test_datasketches.py@34 PS17, Line 34: # Sketches are serialized binary data stored in STRING columns, so they are not valid nit: IMPALA-9821 could be mentioned in the comments, as setting the query option won't be needed after it is merged http://gerrit.cloudera.org:8080/#/c/24570/17/tests/query_test/test_iceberg.py File tests/query_test/test_iceberg.py: http://gerrit.cloudera.org:8080/#/c/24570/17/tests/query_test/test_iceberg.py@100 PS17, Line 100: """IMPALA-12675: Iceberg always annotates STRING columns as UTF-8 (a spec requirement nit: bit verbose http://gerrit.cloudera.org:8080/#/c/24570/17/tests/query_test/test_scanners.py File tests/query_test/test_scanners.py: http://gerrit.cloudera.org:8080/#/c/24570/17/tests/query_test/test_scanners.py@933 PS17, Line 933: """Returns the SchemaElement for column `col_name` in the Parquet file of nit: bit verbose http://gerrit.cloudera.org:8080/#/c/24570/17/tests/query_test/test_scanners.py@966 PS17, Line 966: def converted_type(col_name): : return self._get_parquet_schema_element( : tmpdir, unique_database, TABLE_NAME, col_name).converted_type optional: it seems a bit wasteful to copy and parse the same time 4 times a function like _get_parquet_schema_columns could return a dictionary or list of columns -- To view, visit http://gerrit.cloudera.org:8080/24570 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia95bcba978863ffb5d603fdf81bb2c75ba06f7da Gerrit-Change-Number: 24570 Gerrit-PatchSet: 17 Gerrit-Owner: Daniel Vanko <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Daniel Vanko <[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: Thu, 17 Sep 2026 08:17:20 +0000 Gerrit-HasComments: Yes
