Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/24536 )
Change subject: IMPALA-15162: Add GEOMETRY column support for Iceberg and Parquet ...................................................................... Patch Set 12: (8 comments) http://gerrit.cloudera.org:8080/#/c/24536/12/be/src/exec/parquet/hdfs-parquet-table-writer.cc File be/src/exec/parquet/hdfs-parquet-table-writer.cc: http://gerrit.cloudera.org:8080/#/c/24536/12/be/src/exec/parquet/hdfs-parquet-table-writer.cc@191 PS12, Line 191: meta_data->__set_statistics(stats); Is this where the issue is with incorrectly written stats? Do they still need to be encoded to thrift? http://gerrit.cloudera.org:8080/#/c/24536/12/fe/src/main/java/org/apache/impala/catalog/ColumnStats.java File fe/src/main/java/org/apache/impala/catalog/ColumnStats.java: http://gerrit.cloudera.org:8080/#/c/24536/12/fe/src/main/java/org/apache/impala/catalog/ColumnStats.java@682 PS12, Line 682: // representation (no NDV; null count + byte size). Should we consider adding NDV stats at a later time? http://gerrit.cloudera.org:8080/#/c/24536/12/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java: http://gerrit.cloudera.org:8080/#/c/24536/12/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@4378 PS12, Line 4378: AnalysisError("create table new_table (g geometry) stored as iceberg", : "GEOMETRY columns require Iceberg format version 3 or higher."); This DDL should specify an Iceberg table version of 2 so it does not start failing when the default switches to 3. http://gerrit.cloudera.org:8080/#/c/24536/12/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@4469 PS12, Line 4469: // GEOMETRY requires Iceberg format-version >= 3 (both explicit and via CTAS). : AnalysisError("create table geom_ice (id int, g geometry) stored as iceberg", : "GEOMETRY columns require Iceberg format version 3 or higher."); : AnalysisError("create table geom_ice_ctas stored as iceberg " : + "as select cast(NULL as geometry) as g", : "GEOMETRY columns require Iceberg format version 3 or higher."); These DDLs should specify an Iceberg table version of 2 so it does not start failing when the default switches to 3. http://gerrit.cloudera.org:8080/#/c/24536/12/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@4484 PS12, Line 4484: // ALTER TABLE ADD COLUMNS on an Iceberg table routes through the same : // format-version check: the existing Iceberg fixtures are below v3, so GEOMETRY is : // rejected with the format-version message (not the non-Iceberg message). : AnalysisError("alter table functional_parquet.iceberg_non_partitioned " : + "add columns (g geometry)", : "GEOMETRY columns require Iceberg format version 3 or higher."); This DDL should specify an Iceberg table version of 2 so it does not start failing when the default switches to 3. http://gerrit.cloudera.org:8080/#/c/24536/12/fe/src/test/java/org/apache/impala/util/IcebergHiveSchemaUtilTest.java File fe/src/test/java/org/apache/impala/util/IcebergHiveSchemaUtilTest.java: http://gerrit.cloudera.org:8080/#/c/24536/12/fe/src/test/java/org/apache/impala/util/IcebergHiveSchemaUtilTest.java@1 PS12, Line 1: // Licensed to the Apache Software Foundation (ASF) under one Should this class be moved to the java/shaded-deps/impala-iceberg-runtime/ directory since the patched HiveSchemaUtil class should eventually go away? http://gerrit.cloudera.org:8080/#/c/24536/12/java/shaded-deps/impala-iceberg-runtime/src/main/java/org/apache/iceberg/hive/HiveSchemaUtil.java File java/shaded-deps/impala-iceberg-runtime/src/main/java/org/apache/iceberg/hive/HiveSchemaUtil.java: http://gerrit.cloudera.org:8080/#/c/24536/12/java/shaded-deps/impala-iceberg-runtime/src/main/java/org/apache/iceberg/hive/HiveSchemaUtil.java@116 PS12, Line 116: case BINARY: Nit: trailing space http://gerrit.cloudera.org:8080/#/c/24536/12/testdata/workloads/functional-query/queries/QueryTest/iceberg-geometry.test File testdata/workloads/functional-query/queries/QueryTest/iceberg-geometry.test: http://gerrit.cloudera.org:8080/#/c/24536/12/testdata/workloads/functional-query/queries/QueryTest/iceberg-geometry.test@206 PS12, Line 206: CREATE TABLE iceberg_geom_no_v3 (id INT, g GEOMETRY) STORED AS ICEBERG Hardcode table version here to prevent test case failure when Iceberg default table version switches to 3. -- To view, visit http://gerrit.cloudera.org:8080/24536 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32d8c12a6b71708646fbfc06a6dd8f7aaf4f5e6b Gerrit-Change-Number: 24536 Gerrit-PatchSet: 12 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Balazs Hevele <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Comment-Date: Fri, 25 Sep 2026 18:22:47 +0000 Gerrit-HasComments: Yes
