Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24549 )
Change subject: IMPALA-15160: Add WKB_EXPERIMENTAL geospatial serialization mode ...................................................................... Patch Set 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/24549/3/fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java File fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java: http://gerrit.cloudera.org:8080/#/c/24549/3/fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java@86 PS3, Line 86: new ArrayList<> > bump This is needed because not addNatives is not always true, so line 113 will actually append to the list, which is not possibe with fixed size list returned by Arrays.asList() http://gerrit.cloudera.org:8080/#/c/24549/6/fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java File fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java: http://gerrit.cloudera.org:8080/#/c/24549/6/fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java@125 PS6, Line 125: boolean isWkb > This arg pushdown could be eliminated by reading GeometryUtils.getFormat at This will be used more widely in a later patch https://gerrit.cloudera.org/#/c/24540/9/fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java http://gerrit.cloudera.org:8080/#/c/24549/6/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/24549/6/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java@118 PS6, Line 118: if (!geoLib.equals(TGeospatialLibrary.NONE)) { : HiveEsriGeospatialBuiltins.initBuiltins(this); > nit:Not NONE Done http://gerrit.cloudera.org:8080/#/c/24549/7/fe/src/main/java/org/apache/impala/service/JniFrontend.java File fe/src/main/java/org/apache/impala/service/JniFrontend.java: http://gerrit.cloudera.org:8080/#/c/24549/7/fe/src/main/java/org/apache/impala/service/JniFrontend.java@158 PS7, Line 158: // Initialize the geo serialization format on all impalads (coordinator and : // executor). On executor-only impalads the Java Frontend is not instantiated : // so BuiltinsDb/HiveEsriGeospatialBuiltins.initBuiltins() never runs, but : // Hive geo UDFs still execute via HiveUdfCall and therefore require the : // correct GeometryUtils.format to deserialize geometry values. : TGeospatialLibrary geoLib = BackendConfig.INSTANCE.getGeospatialLibrary(); : if (!geoLib.equals(TGeospatialLibrary.NONE)) { : GeometryUtils.setFormat(geoLib.equals(TGeospatialLibrary.WKB_EXPERIMENTAL) : ? GeometryUtils.SerializationFormat.WKB : : GeometryUtils.SerializationFormat.ESRI_SHAPE); : } Realized that my patch was wrong and only worked because all impalads were coordinators, pure executors would not have been affected by the flag -- To view, visit http://gerrit.cloudera.org:8080/24549 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifbb1b91171bf9a8834a55443d4a19518b38b759e Gerrit-Change-Number: 24549 Gerrit-PatchSet: 7 Gerrit-Owner: Csaba Ringhofer <[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: Thu, 30 Jul 2026 16:40:39 +0000 Gerrit-HasComments: Yes
