Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
53127: builtins: use ST_Union as aggregate r=rytaft,sumeerbhola a=otan

Add the ST_Union aggregate, removing the two-argument version
temporarily as we cannot currently have an aggregate and non-aggregate
at the same time. This is ok since we haven't released yet, and from
reading it seems more likely people will use the aggregate version.

Resolves #51248

Release note (sql change): Implement the ST_Union builtin as an
aggregate. The previous alpha-available ST_Union for two arguments is
deprecated.

53162: builtins: implement ST_GeomFromGeoHash/ST_Box2DFromGeoHash r=sumeerbhola a=otan

Resolves #48806

Release note (sql change): Implemented the ST_GeomFromGeoHash and
ST_Box2DFromGeoHash builtins.

53180: opt: add rule to reduce IS DISTINCT FROM NULL in join filters r=DrewKimball a=DrewKimball

This PR introduces a normalization rule that removes an `IS NOT NULL`
condition on a not-null column from the ON condition of a left or full
join.

Ex:
```
SELECT * FROM abc FULL JOIN abc AS abc2 ON abc.a IS NOT NULL;
=>
SELECT * FROM abc FULL JOIN abc AS abc2 ON True;
```

Fixes #48173

Release note: None

Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
Co-authored-by: Drew Kimball <drewk@cockroachlabs.com>
  • Loading branch information
3 people committed Aug 22, 2020
4 parents 5d1428a + 455ae38 + b8678a2 + 26dabe7 commit 320eb2e
Show file tree
Hide file tree
Showing 20 changed files with 766 additions and 498 deletions.
2 changes: 2 additions & 0 deletions docs/generated/sql/aggregates.md
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,8 @@
</span></td></tr>
<tr><td><a name="st_makeline"></a><code>st_makeline(arg1: geometry) &rarr; geometry</code></td><td><span class="funcdesc"><p>Forms a LineString from Point, MultiPoint or LineStrings. Other shapes will be ignored.</p>
</span></td></tr>
<tr><td><a name="st_union"></a><code>st_union(arg1: geometry) &rarr; geometry</code></td><td><span class="funcdesc"><p>Applies a spatial union to the geometries provided.</p>
</span></td></tr>
<tr><td><a name="stddev"></a><code>stddev(arg1: <a href="decimal.html">decimal</a>) &rarr; <a href="decimal.html">decimal</a></code></td><td><span class="funcdesc"><p>Calculates the standard deviation of the selected values.</p>
</span></td></tr>
<tr><td><a name="stddev"></a><code>stddev(arg1: <a href="float.html">float</a>) &rarr; <a href="float.html">float</a></code></td><td><span class="funcdesc"><p>Calculates the standard deviation of the selected values.</p>
Expand Down
15 changes: 10 additions & 5 deletions docs/generated/sql/functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -974,6 +974,10 @@ has no relationship with the commit order of concurrent transactions.</p>
<tr><td><a name="st_azimuth"></a><code>st_azimuth(geometry_a: geometry, geometry_b: geometry) &rarr; <a href="float.html">float</a></code></td><td><span class="funcdesc"><p>Returns the azimuth in radians of the segment defined by the given point geometries, or NULL if the two points are coincident.</p>
<p>The azimuth is angle is referenced from north, and is positive clockwise: North = 0; East = π/2; South = π; West = 3π/2.</p>
</span></td></tr>
<tr><td><a name="st_box2dfromgeohash"></a><code>st_box2dfromgeohash(geohash: <a href="string.html">string</a>) &rarr; box2d</code></td><td><span class="funcdesc"><p>Return a Box2D from a GeoHash string with max precision.</p>
</span></td></tr>
<tr><td><a name="st_box2dfromgeohash"></a><code>st_box2dfromgeohash(geohash: <a href="string.html">string</a>, precision: <a href="int.html">int</a>) &rarr; box2d</code></td><td><span class="funcdesc"><p>Return a Box2D from a GeoHash string with supplied precision.</p>
</span></td></tr>
<tr><td><a name="st_buffer"></a><code>st_buffer(geography: geography, distance: <a href="float.html">float</a>) &rarr; geography</code></td><td><span class="funcdesc"><p>Returns a Geometry that represents all points whose distance is less than or equal to the given distance
from the given Geometry.</p>
<p>This function utilizes the GEOS module.</p>
Expand Down Expand Up @@ -1264,6 +1268,10 @@ Bottom Left.</p>
</span></td></tr>
<tr><td><a name="st_geomfromewkt"></a><code>st_geomfromewkt(val: <a href="string.html">string</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Returns the Geometry from an EWKT representation.</p>
</span></td></tr>
<tr><td><a name="st_geomfromgeohash"></a><code>st_geomfromgeohash(geohash: <a href="string.html">string</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Return a POLYGON Geometry from a GeoHash string with max precision.</p>
</span></td></tr>
<tr><td><a name="st_geomfromgeohash"></a><code>st_geomfromgeohash(geohash: <a href="string.html">string</a>, precision: <a href="int.html">int</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Return a POLYGON Geometry from a GeoHash string with supplied precision.</p>
</span></td></tr>
<tr><td><a name="st_geomfromgeojson"></a><code>st_geomfromgeojson(val: <a href="string.html">string</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Returns the Geometry from an GeoJSON representation.</p>
</span></td></tr>
<tr><td><a name="st_geomfromgeojson"></a><code>st_geomfromgeojson(val: jsonb) &rarr; geometry</code></td><td><span class="funcdesc"><p>Returns the Geometry from an GeoJSON representation.</p>
Expand Down Expand Up @@ -1490,9 +1498,9 @@ calculated, the result is transformed back into a Geography with SRID 4326.</p>
</span></td></tr>
<tr><td><a name="st_point"></a><code>st_point(x: <a href="float.html">float</a>, y: <a href="float.html">float</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Returns a new Point with the given X and Y coordinates.</p>
</span></td></tr>
<tr><td><a name="st_pointfromgeohash"></a><code>st_pointfromgeohash(geohash: <a href="string.html">string</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Return a Geometry point from a GeoHash string with max precision.</p>
<tr><td><a name="st_pointfromgeohash"></a><code>st_pointfromgeohash(geohash: <a href="string.html">string</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Return a POINT Geometry from a GeoHash string with max precision.</p>
</span></td></tr>
<tr><td><a name="st_pointfromgeohash"></a><code>st_pointfromgeohash(geohash: <a href="string.html">string</a>, precision: <a href="int.html">int</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Return a Geometry point from a GeoHash string with supplied precision.</p>
<tr><td><a name="st_pointfromgeohash"></a><code>st_pointfromgeohash(geohash: <a href="string.html">string</a>, precision: <a href="int.html">int</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Return a POINT Geometry from a GeoHash string with supplied precision.</p>
</span></td></tr>
<tr><td><a name="st_pointfromtext"></a><code>st_pointfromtext(str: <a href="string.html">string</a>, srid: <a href="int.html">int</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Returns the Geometry from a WKT or EWKT representation with an SRID. If the shape underneath is not Point, NULL is returned. If the SRID is present in both the EWKT and the argument, the argument value is used.</p>
</span></td></tr>
Expand Down Expand Up @@ -1605,9 +1613,6 @@ Negative azimuth values and values greater than 2π (360 degrees) are supported.
</span></td></tr>
<tr><td><a name="st_translate"></a><code>st_translate(g: geometry, deltaX: <a href="float.html">float</a>, deltaY: <a href="float.html">float</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Returns a modified Geometry translated by the given deltas</p>
</span></td></tr>
<tr><td><a name="st_union"></a><code>st_union(geometry_a: geometry, geometry_b: geometry) &rarr; geometry</code></td><td><span class="funcdesc"><p>Returns the union of the given geometries as a single Geometry object.</p>
<p>This function utilizes the GEOS module.</p>
</span></td></tr>
<tr><td><a name="st_within"></a><code>st_within(geometry_a: geometry, geometry_b: geometry) &rarr; <a href="bool.html">bool</a></code></td><td><span class="funcdesc"><p>Returns true if geometry_a is completely inside geometry_b.</p>
<p>This function utilizes the GEOS module.</p>
<p>This function variant will attempt to utilize any available geospatial index.</p>
Expand Down
41 changes: 0 additions & 41 deletions pkg/geo/geohash.go

This file was deleted.

51 changes: 51 additions & 0 deletions pkg/geo/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"github.com/cockroachdb/cockroach/pkg/geo/geopb"
"github.com/cockroachdb/cockroach/pkg/geo/geos"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/errors"
"github.com/pierrre/geohash"
"github.com/twpayne/go-geom"
"github.com/twpayne/go-geom/encoding/ewkb"
"github.com/twpayne/go-geom/encoding/ewkbhex"
Expand Down Expand Up @@ -209,3 +211,52 @@ func hasPrefixIgnoreCase(str string, prefix string) bool {
}
return true
}

// ParseGeometryPointFromGeoHash converts a GeoHash to a Geometry Point
// using a Lng/Lat Point representation of the GeoHash.
func ParseGeometryPointFromGeoHash(g string, precision int) (*Geometry, error) {
box, err := parseGeoHash(g, precision)
if err != nil {
return nil, err
}
point := box.Center()
geom, gErr := NewGeometryFromPointCoords(point.Lon, point.Lat)
if gErr != nil {
return nil, gErr
}
return geom, nil
}

// ParseCartesianBoundingBoxFromGeoHash converts a GeoHash to a CartesianBoundingBox.
func ParseCartesianBoundingBoxFromGeoHash(g string, precision int) (CartesianBoundingBox, error) {
box, err := parseGeoHash(g, precision)
if err != nil {
return CartesianBoundingBox{}, err
}
return CartesianBoundingBox{
BoundingBox: geopb.BoundingBox{
LoX: box.Lon.Min,
HiX: box.Lon.Max,
LoY: box.Lat.Min,
HiY: box.Lat.Max,
},
}, nil
}

func parseGeoHash(g string, precision int) (geohash.Box, error) {
if len(g) == 0 {
return geohash.Box{}, errors.Newf("length of GeoHash must be greater than 0")
}

// 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.
if precision > len(g) || precision < 0 {
precision = len(g)
}
box, err := geohash.Decode(g[:precision])
if err != nil {
return geohash.Box{}, err
}
return box, nil
}
82 changes: 82 additions & 0 deletions pkg/geo/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
package geo

import (
"fmt"
"testing"

"github.com/cockroachdb/cockroach/pkg/geo/geopb"
"github.com/pierrre/geohash"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -589,3 +591,83 @@ func TestParseGeography(t *testing.T) {
})
}
}

func TestParseHash(t *testing.T) {
testCases := []struct {
h string
p int
expected geohash.Box
}{
{"123", 2, geohash.Box{
Lat: geohash.Range{
Min: -90,
Max: -84.375,
},
Lon: geohash.Range{
Min: -123.75,
Max: -112.5,
},
}},
{"123", 3, geohash.Box{
Lat: geohash.Range{
Min: -88.59375,
Max: -87.1875,
},
Lon: geohash.Range{
Min: -122.34375,
Max: -120.9375,
},
}},
{"123", 4, geohash.Box{
Lat: geohash.Range{
Min: -88.59375,
Max: -87.1875,
},
Lon: geohash.Range{
Min: -122.34375,
Max: -120.9375,
},
}},
{"123", -1, geohash.Box{
Lat: geohash.Range{
Min: -88.59375,
Max: -87.1875,
},
Lon: geohash.Range{
Min: -122.34375,
Max: -120.9375,
},
}},
}
for _, tc := range testCases {
t.Run(fmt.Sprintf("%s[:%d]", tc.h, tc.p), func(t *testing.T) {
ret, err := parseGeoHash(tc.h, tc.p)
require.NoError(t, err)
require.Equal(t, tc.expected, ret)
})
}

errorCases := []struct {
h string
p int
err string
}{
{
"",
10,
"length of GeoHash must be greater than 0",
},
{
"-",
10,
`geohash decode '-': invalid character at index 0`,
},
}
for _, tc := range errorCases {
t.Run(fmt.Sprintf("%s[:%d]", tc.h, tc.p), func(t *testing.T) {
_, err := parseGeoHash(tc.h, tc.p)
require.Error(t, err)
require.EqualError(t, err, tc.err)
})
}
}
10 changes: 6 additions & 4 deletions pkg/sql/distsql/columnar_operators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ var aggregateFuncToNumArguments = map[execinfrapb.AggregatorSpec_Func]int{
execinfrapb.AggregatorSpec_STDDEV_POP: 1,
execinfrapb.AggregatorSpec_ST_MAKELINE: 1,
execinfrapb.AggregatorSpec_ST_EXTENT: 1,
execinfrapb.AggregatorSpec_ST_UNION: 1,
}

// TestAggregateFuncToNumArguments ensures that all aggregate functions are
Expand Down Expand Up @@ -183,9 +184,9 @@ func TestAggregatorAgainstProcessor(t *testing.T) {
for j := range aggFnInputTypes {
aggFnInputTypes[j] = sqlbase.RandType(rng)
}
// There is a special case for concat_agg, string_agg,
// st_makeline, and st_extent when at least one argument is a
// tuple. Such cases pass GetAggregateInfo check below,
// There is a special case for some functions when at
// least one argument is a tuple.
// Such cases pass GetAggregateInfo check below,
// but they are actually invalid, and during normal
// execution it is caught during type-checking.
// However, we don't want to do fully-fledged type
Expand All @@ -195,7 +196,8 @@ func TestAggregatorAgainstProcessor(t *testing.T) {
case execinfrapb.AggregatorSpec_CONCAT_AGG,
execinfrapb.AggregatorSpec_STRING_AGG,
execinfrapb.AggregatorSpec_ST_MAKELINE,
execinfrapb.AggregatorSpec_ST_EXTENT:
execinfrapb.AggregatorSpec_ST_EXTENT,
execinfrapb.AggregatorSpec_ST_UNION:
for _, typ := range aggFnInputTypes {
if typ.Family() == types.TupleFamily {
invalid = true
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/execinfra/server_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ import (
//
// ATTENTION: When updating these fields, add to version_history.txt explaining
// what changed.
const Version execinfrapb.DistSQLVersion = 32
const Version execinfrapb.DistSQLVersion = 33

// MinAcceptedVersion is the oldest version that the server is
// compatible with; see above.
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/execinfra/version_history.txt
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,6 @@
- Version: 32 (MinAcceptedVersion: 30)
- Added an aggregator for ST_Extent. The change is backwards compatible
(mixed versions will prevent parallelization).
- Version: 33 (MinAcceptedVersion: 30)
- Added an aggregator for ST_Union. The change is backwards compatible
(mixed versions will prevent parallelization).
Loading

0 comments on commit 320eb2e

Please sign in to comment.