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 24, 2022
1 parent 96d102a commit 8c610ca
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, upper case characters are parsed as lower case characters.
{"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 8c610ca

Please sign in to comment.