Skip to content

Commit

Permalink
sql, geo: Fix upper case geohash parsing and allow NULL arguments
Browse files Browse the repository at this point in the history
Release note (sql change): ST_Box2DFromGeoHash now accepts NULL arguments,
the precision being NULL is the same as no precision being passed in at all.

Upper case characters are now parsed as lower case characters for geohash,
this matches Postgis behaviour.
  • Loading branch information
RichardJCai committed Feb 25, 2022
1 parent 96d102a commit 537f89d
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 3 deletions.
3 changes: 3 additions & 0 deletions pkg/geo/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,9 @@ func parseGeoHash(g string, precision int) (geohash.Box, error) {
return geohash.Box{}, pgerror.Newf(pgcode.InvalidParameterValue, "length of GeoHash must be greater than 0")
}

// In PostGIS the parsing is case-insensitive.
g = strings.ToLower(g)

// If precision is more than the length of the geohash
// or if precision is less than 0 then set
// precision equal to length of geohash.
Expand Down
11 changes: 11 additions & 0 deletions pkg/geo/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,17 @@ func TestParseHash(t *testing.T) {
Max: -120.9375,
},
}},
// In PostGIS the parsing is case-insensitive.
{"F", 1, geohash.Box{
Lat: geohash.Range{
Min: 45,
Max: 90,
},
Lon: geohash.Range{
Min: -90,
Max: -45,
},
}},
}
for _, tc := range testCases {
t.Run(fmt.Sprintf("%s[:%d]", tc.h, tc.p), func(t *testing.T) {
Expand Down
30 changes: 30 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/geospatial_bbox
Original file line number Diff line number Diff line change
Expand Up @@ -243,3 +243,33 @@ BOX(0 0,0.00000000032741809263825417 0.00000000016370904631912708)
BOX(20.012344998976914 -20.012345000286587,20.012345000286587 -20.012344998976914)
BOX(90 0,90.00000000032742 0.00000000016370904631912708)
BOX(90 0,90.0439453125 0.0439453125)

query T
SELECT ST_Box2DFromGeoHash('F'::TEXT::TEXT::TEXT, NULL::INT4::INT4)::BOX2D;
----
BOX(-90 45,-45 90)

query T
SELECT ST_Box2DFromGeoHash('kkqnpkue9ktbpe5', NULL)::BOX2D;
----
BOX(20.012344998976914 -20.012345000286587,20.012345000286587 -20.012344998976914)

query T
SELECT ST_Box2DFromGeoHash('KKQNPKUE9KTBPE5', NULL)::BOX2D;
----
BOX(20.012344998976914 -20.012345000286587,20.012345000286587 -20.012344998976914)

query T
SELECT ST_Box2DFromGeoHash('kKqNpKuE9KtBpE5', NULL)::BOX2D;
----
BOX(20.012344998976914 -20.012345000286587,20.012345000286587 -20.012344998976914)

query T
SELECT ST_Box2DFromGeoHash(NULL)::BOX2D;
----
NULL

query T
SELECT ST_Box2DFromGeoHash(NULL, NULL)::BOX2D;
----
NULL
20 changes: 17 additions & 3 deletions pkg/sql/sem/builtins/geo_builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -1185,17 +1185,28 @@ SELECT ST_S2Covering(geography, 's2_max_level=15,s2_level_mod=3').
},
),
"st_box2dfromgeohash": makeBuiltin(
defProps(),
tree.FunctionProperties{NullableArgs: true},
tree.Overload{
Types: tree.ArgTypes{
{"geohash", types.String},
{"precision", types.Int},
},
ReturnType: tree.FixedReturnType(types.Box2D),
Fn: func(ctx *tree.EvalContext, args tree.Datums) (tree.Datum, error) {
if args[0] == tree.DNull {
return tree.DNull, nil
}

g := tree.MustBeDString(args[0])
p := tree.MustBeDInt(args[1])
bbox, err := geo.ParseCartesianBoundingBoxFromGeoHash(string(g), int(p))

// Precision is allowed to be NULL, in that case treat it as if the
// argument had not been passed in at all
p := len(string(g))
if args[1] != tree.DNull {
p = int(tree.MustBeDInt(args[1]))
}

bbox, err := geo.ParseCartesianBoundingBoxFromGeoHash(string(g), p)
if err != nil {
return nil, err
}
Expand All @@ -1212,6 +1223,9 @@ SELECT ST_S2Covering(geography, 's2_max_level=15,s2_level_mod=3').
},
ReturnType: tree.FixedReturnType(types.Box2D),
Fn: func(ctx *tree.EvalContext, args tree.Datums) (tree.Datum, error) {
if args[0] == tree.DNull {
return tree.DNull, nil
}
g := tree.MustBeDString(args[0])
p := len(string(g))
bbox, err := geo.ParseCartesianBoundingBoxFromGeoHash(string(g), p)
Expand Down

0 comments on commit 537f89d

Please sign in to comment.