Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24534 )
Change subject: IMPALA-15161: Add GEOMETRY type + make WKB_EXPERIMENTAL default ...................................................................... Patch Set 32: (8 comments) http://gerrit.cloudera.org:8080/#/c/24534/31//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24534/31//COMMIT_MSG@56 PS31, Line 56: because > typo Done http://gerrit.cloudera.org:8080/#/c/24534/31/fe/src/compat-hive-3/java/org/apache/impala/compat/GeospatialBuiltins.java File fe/src/compat-hive-3/java/org/apache/impala/compat/GeospatialBuiltins.java: http://gerrit.cloudera.org:8080/#/c/24534/31/fe/src/compat-hive-3/java/org/apache/impala/compat/GeospatialBuiltins.java@61 PS31, Line 61: > logger not used anywhere Done http://gerrit.cloudera.org:8080/#/c/24534/31/fe/src/compat-hive-3/java/org/apache/impala/compat/GeospatialBuiltins.java@82 PS31, Line 82: ivate static v > nit: No gap between if and parentheses. Done http://gerrit.cloudera.org:8080/#/c/24534/31/fe/src/compat-hive-3/java/org/apache/impala/compat/GeospatialBuiltins.java@107 PS31, Line 107: g(), > nit: no gap between comma and new. Done http://gerrit.cloudera.org:8080/#/c/24534/31/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java File fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java: http://gerrit.cloudera.org:8080/#/c/24534/31/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java@119 PS31, Line 119: > nit: No indentation. Done http://gerrit.cloudera.org:8080/#/c/24534/31/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java@1748 PS31, Line 1748: ) > nit: extra space. Done http://gerrit.cloudera.org:8080/#/c/24534/31/java/hive-geospatial-functions/src/main/java/org/apache/impala/hive/geospatial/esri/README.md File java/hive-geospatial-functions/src/main/java/org/apache/impala/hive/geospatial/esri/README.md: http://gerrit.cloudera.org:8080/#/c/24534/31/java/hive-geospatial-functions/src/main/java/org/apache/impala/hive/geospatial/esri/README.md@48 PS31, Line 48: ementation and testing (IMPALA-15168). : 5. **Relational functions are GEOMETRY-only** — `ST_Intersect > I think this is stale as (GEOMETRY, GEOMETRY) is now also registered in WKB thanks for spotting! also reordered the differences to start with the most significant one (GEOMETRY instead of BINARY) http://gerrit.cloudera.org:8080/#/c/24534/31/testdata/workloads/functional-query/queries/QueryTest/geospatial-esri.test File testdata/workloads/functional-query/queries/QueryTest/geospatial-esri.test: http://gerrit.cloudera.org:8080/#/c/24534/31/testdata/workloads/functional-query/queries/QueryTest/geospatial-esri.test@a2455 PS31, Line 2455: > I might be missing something, but I dont understand why was this removed. T This is intentional - this test runs both in HIVE_ESRI and WKB_EXPERIMENTAL mode, and the error message is different due to the BINARY->GEOMETRY switch. Removing the argument types seemed the simples fix. -- To view, visit http://gerrit.cloudera.org:8080/24534 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ff29cb15ab45f3899bf453041e605a32f195c27 Gerrit-Change-Number: 24534 Gerrit-PatchSet: 32 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Aleksandr Efimov <[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-Reviewer: Peter Rozsa <[email protected]> Gerrit-Comment-Date: Mon, 21 Sep 2026 08:54:33 +0000 Gerrit-HasComments: Yes
