Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24547 )
Change subject: IMPALA-15154: Copy Hive ESRI geospatial UDFs into Impala ...................................................................... Patch Set 10: (5 comments) http://gerrit.cloudera.org:8080/#/c/24547/8/java/hive-geospatial-functions/pom.xml File java/hive-geospatial-functions/pom.xml: http://gerrit.cloudera.org:8080/#/c/24547/8/java/hive-geospatial-functions/pom.xml@29 PS8, Line 29: <artifactId>hive-geospatial-functions</artifactId> > It's inherited from the parent, not needed Done http://gerrit.cloudera.org:8080/#/c/24547/8/java/hive-geospatial-functions/pom.xml@36 PS8, Line 36: <groupId>com.esri.geometry</groupId> > Is this required? no, the parent sets this property ... twice I deduplicated this to set it at a single place and removed from the 2 projects that used it. http://gerrit.cloudera.org:8080/#/c/24547/8/java/hive-geospatial-functions/pom.xml@43 PS8, Line 43: </dependency> > Please externalize the version to the parent pom Done http://gerrit.cloudera.org:8080/#/c/24547/8/java/shaded-deps/hive-exec/pom.xml File java/shaded-deps/hive-exec/pom.xml: http://gerrit.cloudera.org:8080/#/c/24547/8/java/shaded-deps/hive-exec/pom.xml@137 PS8, Line 137: <include>org/apache/hadoop/hive/ql/exec/Description.class</include> > Is this really required? We could remove the Description annotations from t not really required, it is just there to minimize changes from Hive and it can be useful as documentation this class is minimal and didn't change in the last 6 years, so I don't expect issues from this: https://github.com/apache/hive/blob/master/udf/src/java/org/apache/hadoop/hive/ql/exec/Description.java I can remove the descriptions if you still prefer that. Or can also copy the file to have our own version. http://gerrit.cloudera.org:8080/#/c/24547/8/testdata/workloads/functional-query/queries/QueryTest/geospatial-esri-extra.test File testdata/workloads/functional-query/queries/QueryTest/geospatial-esri-extra.test: http://gerrit.cloudera.org:8080/#/c/24547/8/testdata/workloads/functional-query/queries/QueryTest/geospatial-esri-extra.test@157 PS8, Line 157: lect > nit: it could be a ticket for future work oops, this TODO shouldn't be here, the original commit was split in 2, this is from the second one there is a WIP patch about extending tests to cover missing functions https://gerrit.cloudera.org/#/c/24535/ this specific round trip is is likely to be simple, I just need to focus on it a bit -- To view, visit http://gerrit.cloudera.org:8080/24547 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7d078e6ee961c76b10915da6be5b86966ad9cefb Gerrit-Change-Number: 24547 Gerrit-PatchSet: 10 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: Peter Rozsa <[email protected]> Gerrit-Comment-Date: Thu, 09 Jul 2026 13:51:10 +0000 Gerrit-HasComments: Yes
