HIVE-29353:Add basic support for encoding GeoHash & decoding it to Geometry by ramitg254 · Pull Request #6813 · apache/hive

ramitg254 · 2026-09-22T18:29:18Z

Change-Id: I3a88b45b021ec8d40bd6a479b6f17ed15a411d38

What changes were proposed in this pull request?

Add basic support for encoding a geohash for a 2d point and decoding it to corresponding bounding box.

Why are the changes needed?

geohash encoding and decoding is not supported

Does this PR introduce any user-facing change?

Yes, user will be able to use st_geohash and st_geomfromgeohash spatial functions.

How was this patch tested?

unit test

InvisibleProgrammer · 2026-09-25T07:35:21Z

+   * @param characterPrecision number of leading characters to use (1–12, at most {@code geohash}
+   *     length)
+   */
+  public static Polygon geohashCellPolygon(String geohash, int characterPrecision) {


I wonder why this is the only public method in this class that got javadoc. I would suggest adding documentation to the other public methods as well.

for other methods their names are suffieciently self explainatory, this one required some explaination regarding type of polygon returned and geohash string kind needed so added javadoc for it only. I think we can avoid javadoc for others wdyt?

InvisibleProgrammer · 2026-09-25T07:40:05Z

+        characterPrecision > geohash.length()) {
+      return null;
+    }
+    String hashPrefix = geohash.substring(0, characterPrecision);


I'm trying to understand what happens here: we receive a geohash and a precision.

And we only use the first part of the hash.
That suggests a geohash actually contains geo coordinates only in the first n character, according to precision. What is in the remaining part of the geohash string?

this is postgis compliant behaviour
a geohash while converting to geometry implies only to a rectangular polygon basically so size of rectangle depends on the no. of characters used from geohash string so if you use entire string then that rectangle contains less other points than the one from it is constructed, and if you use subset prefix of it then it will contain more other points tahn the one from it is constructed

InvisibleProgrammer · 2026-09-25T07:45:35Z

+    return evaluate(geomref, null);
+  }
+
+  public Text evaluate(BytesWritable geomref, IntWritable precisionArg) {


nit: I think geomref is something like geometric reference. I would suggest using geomRef instead. Both here and at the other evaluate method.

the same variable name is used in almost all the other geospatial udfs so used same for consistency.
changing it will require changing it in all other places as well

InvisibleProgrammer · 2026-09-25T07:49:20Z

+  }
+
+  public Text evaluate(BytesWritable geomref, IntWritable precisionArg) {
+    if (geomref == null || geomref.getLength() == 0) {


Tiny suggestion about readability: commons-lang3 has utility methods that helps a lot around strings, like StringUtils.isEmpty().

done added StringUtils.isEmpty() wherever it seemed correct

InvisibleProgrammer · 2026-09-25T07:53:46Z

+  }
+
+  public Text evaluate(BytesWritable geomref, IntWritable precisionArg) {
+    if (geomref == null || geomref.getLength() == 0) {


Question about performance vs readability: GeometryUtils.getType() starts with the following check:

if (geomref == null || geomref.getLength() < 5) { return OGCType.UNKNOWN; }

This is basically almost the exact same check as the check here. Does it worth to do both checks or one of them is enough?

the checks are cheap and I think keeping it is reasonable based on package logging style and consistency

InvisibleProgrammer · 2026-09-25T08:05:16Z

+      return null;
+    }
+    Point point = (Point) geom;
+    return geohashText(point.getX(), point.getY(), precisionArg);


GeometryUtils.geometryFromEsriShape(geomref) can return with an empty point. Empty point has no coordinates and point.getX() and pont.getY() can return with IllegalStateException if there is no coordinates.
It would worth to do double checking and return with null not only if geometryFromEsriShape but if the point is empty as well (Point.isEmpty()).

InvisibleProgrammer · 2026-09-25T08:08:52Z

+  private Text geohashText(double longitude, double latitude, IntWritable precisionArg) {
+    int precision =
+        GeoHashUtils.resolveEncodePrecision(precisionArg == null ? null : precisionArg.get());
+    if (precision < 0) {


nit about readability:
Now we have to navigate to GeoHashUtils and check the method what precision < 0 actually means if we want to understand it.

What if adding an extra contstant to GeoHashUtils, something like INVALID_PRECISION so that this expression can be rewritten to an easier to understand format, like if (precision == GeoHashUtils.INVALID_PRECISION).

InvisibleProgrammer · 2026-09-25T08:41:38Z

+        geohash.length());
+    if (characterPrecision < 0) {
+      LogUtils.Log_InvalidPrecision(LOG, GeoHashUtils.MIN_CHARACTER_PRECISION,
+          Math.min(geohash.length(), GeoHashUtils.MAX_CHARACTER_PRECISION));


If the geohash.length() is 10, there will be the log message:

Invalid precision - Precision must be between 1 and 10

It is not true because the precision should be between 1 and 12.

when we are construct geometry from geoshash string, specifying precision beyond string length doesn't yield anything and will be wrong as per behaviour: #6813 (comment)
so for string of length 10 precison 12 is wrong will be wrong as construction of geometry depends on the no. of chars in the string we are gonna use which are at max 10 in that case

InvisibleProgrammer · 2026-09-25T08:48:47Z

+      <groupId>ch.hsr</groupId>
+      <artifactId>geohash</artifactId>
+      <version>${geohash.version}</version>
+      <scope>compile</scope>


compile is the default dependency scope: https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html

…ometry Change-Id: I3a88b45b021ec8d40bd6a479b6f17ed15a411d38

Change-Id: I5f36faa114e8ad927eb6c18d62b2bf92538abc94

sonarqubecloud · 2026-09-28T12:03:10Z

Quality Gate passed

Issues
8 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

InvisibleProgrammer · 2026-09-28T13:02:25Z

There are two questions that I want to bring up:

Firstly, I couldn't make the tests running on my computer. With our private discussion, I was able to run the test with adding the content of this other change: #6797. I wonder, would it worth merge those PRs as one? The other PR is a one liner that also helps running the test case for this one.

Secondly, I was curious about logging so I created a test case that simulates a typical user error when they accidentally just pick a wrong column writing their select statement:

create table geohash_wrong(name string);

insert into geohash_wrong(name) values ('Joe'), ('Jack'), ('Jill');

select ST_GeoHash(ST_Point(name, name), 10) as geohash
from geohash_wrong;

I was wondered how the logs look like. Do they have a single log entry? No log entry at all and just null values in the result set? Multiple log entries, one for each failure?

With this test, I expected the execution stopping at the ST_Point function calls. The output was what I expected. But the log is not.
Output:

POSTHOOK: query: select ST_GeoHash(ST_Point(name, name), 10) as geohash
from geohash_wrong
POSTHOOK: type: QUERY
POSTHOOK: Input: default@geohash_wrong
#### A masked pattern was here ####
NULL
NULL
NULL

Log:

2026-09-28T05:56:43,907 ERROR [e0c7c9aa-d722-4880-8124-07b523932ee5 main] esri.ST_GeoHash: Invalid arguments - one or more arguments are null.
2026-09-28T05:56:43,907 ERROR [e0c7c9aa-d722-4880-8124-07b523932ee5 main] esri.ST_GeoHash: Invalid arguments - one or more arguments are null.
2026-09-28T05:56:43,907 ERROR [e0c7c9aa-d722-4880-8124-07b523932ee5 main] esri.ST_GeoHash: Invalid arguments - one or more arguments are null.

As you can see the select result contains NULL values only. As it expected. no error log entry from ST_GeoHash (actually no log entry at all). I assumed a log entry about the failure at parsing ST_Point as Jack is not a valid geometric coordinate. No log entry for ST_Point at all. But got 3 error log entries for ST_GeoHash.

My problem is with this approach that I fear what happens if a user runs a query with a similar mistake but the table contains 1M rows. I'm pretty sure their log file would explode.

My suggestion is to reconsider the logging strategy and just return with null values in similar cases.

ramitg254 · 2026-10-01T11:33:46Z

As per the private discussion, @csringhofer has better solution for this issue, so closing this pr.
Thanks @InvisibleProgrammer for the inputs so far

csringhofer · 2026-10-01T12:18:28Z

As per the private discussion, @csringhofer has better solution for this issue, so closing this pr. Thanks @InvisibleProgrammer for the inputs so far

Thx @ramitg254 ! Sorry for the not signaling that I had a patch and causing extra work.

asf-ci-hive added the tests pending label Sep 22, 2026

ramitg254 changed the title ~~HIVE-29353:Add basic support for encoding GeoHash & decoding it to Ge…~~ [WIP] Sep 22, 2026

asf-ci-hive added tests unstable and removed tests pending labels Sep 22, 2026

ramitg254 force-pushed the HIVE-29353 branch from dd40686 to fc23fb3 Compare September 23, 2026 05:55

asf-ci-hive added tests pending and removed tests unstable labels Sep 23, 2026

ramitg254 changed the title ~~[WIP]~~ HIVE-29353:Add basic support for encoding GeoHash & decoding it to cell polygon Geometry Sep 23, 2026

ramitg254 changed the title ~~HIVE-29353:Add basic support for encoding GeoHash & decoding it to cell polygon Geometry~~ HIVE-29353:Add basic support for encoding GeoHash & decoding it to rectangular polygon Geometry Sep 23, 2026

ramitg254 changed the title ~~HIVE-29353:Add basic support for encoding GeoHash & decoding it to rectangular polygon Geometry~~ HIVE-29353:Add basic support for encoding GeoHash & decoding it to corresponding bounding box Sep 23, 2026

ramitg254 changed the title ~~HIVE-29353:Add basic support for encoding GeoHash & decoding it to corresponding bounding box~~ HIVE-29353:Add basic support for encoding GeoHash & decoding it to Geometry Sep 23, 2026

asf-ci-hive added tests failed tests pending tests passed and removed tests pending tests failed labels Sep 23, 2026

InvisibleProgrammer reviewed Sep 25, 2026

View reviewed changes

HIVE-29353:Add basic support for encoding GeoHash & decoding it to Ge…

6b32e3b

…ometry Change-Id: I3a88b45b021ec8d40bd6a479b6f17ed15a411d38

ramitg254 force-pushed the HIVE-29353 branch from fc23fb3 to 8f5fdd9 Compare September 28, 2026 09:42

asf-ci-hive added tests pending and removed tests passed labels Sep 28, 2026

refractored and corrected

2343498

Change-Id: I5f36faa114e8ad927eb6c18d62b2bf92538abc94

ramitg254 force-pushed the HIVE-29353 branch from 8f5fdd9 to 2343498 Compare September 28, 2026 10:50

asf-ci-hive added tests failed tests pending and removed tests pending tests failed labels Sep 28, 2026

asf-ci-hive added tests passed and removed tests pending labels Sep 28, 2026

ramitg254 closed this Oct 1, 2026

Conversation

ramitg254 commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Sep 28, 2026

Quality Gate passed

Uh oh!

InvisibleProgrammer commented Sep 28, 2026

Uh oh!

ramitg254 commented Oct 1, 2026

Uh oh!

csringhofer commented Oct 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ramitg254 commented Sep 22, 2026 •

edited

Loading