Skip to content

Commit

Permalink
Merge #52940 #52958 #52961 #52963 #52965
Browse files Browse the repository at this point in the history 
52940: sql: allow DEALLOCATE ALL with a prepared statement r=asubiotto a=rafiss

PR #48842 added logic to exhaust portals after executing them. This had
issues when the portal being executed closes itself, which happens when
using DEALLOCATE in a prepared statement. Now we check if the portal
still exists before exhausting it.

There is no release note as this fixes a bug that only exists in
unreleased versions.

fixes #52915
fixes #52220
fixes #52880
fixes #52506

Release note: None

52958: RFCs: update geospatial RFC r=otan a=otan

Update geospatial RFC to be in line with updated offerings and commands.

Release note: None

52961: builtins: implement ST_MakeBox2D r=sumeerbhola a=otan

Resolves #52900.

Release note (sql change): Implement ST_MakeBox2D.

52963: builtins: implement the PostGIS_GetBBox builtin r=sumeerbhola a=otan

Release note (sql change): Implement the PostGIS_GetBBox builtin.

52965: tree: implement casts between box2d and geometry r=rytaft,sumeerbhola a=otan

Release note (sql change): Implement the ability to cast between box2d
and geometry types.

Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Oliver Tan <[email protected]>
  • Loading branch information
@rafiss @otan
3 people committed Aug 18, 2020
6 parents d1afb84 + cb23025 + 0d8594c + 6429dda + c8b6fbe + 51b89db commit f41ff14
Show file tree
Hide file tree
Showing 10 changed files with 311 additions and 56 deletions.
77 changes: 62 additions & 15 deletions docs/RFCS/20200421_geospatial.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,25 +266,22 @@ If we decide to support user-adjustable S2 parameters, we could
support a syntax like the following:

```
CREATE INVERTED INDEX [indexname] ON [tablename] ( [geospatial_column] ) WITH COVERING (min_level = 0, max_level = 30, ….)
CREATE INVERTED INDEX [indexname] ON [tablename] ( [geospatial_column] ) WITH (s2_max_level = 30, ….)
-- or
CREATE INDEX [indexname] ON [tablename] USING GIST ( [geospatial_column] ) WITH (s2_max_level = 30, ….)
```

For 2D planar geometry there will be an axis-aligned rectangular bound
that all shapes that want index acceleration should fit in. The bound
will be specified at index creation time. We could adopt a syntax
similar to SQL Server, which also requires a rectangular [bounding
box](https://docs.microsoft.com/en-us/sql/relational-databases/spatial/spatial-indexes-overview?view=sql-server-ver15#the-bounding-box).
Shapes can exceed this bounding box, but those shapes will not benefit
from index acceleration. To support this syntax, we will introduce a
WITH:
will be specified at index creation time. By default this will use
the bounds defined by the SRID.

Bounds can be overwritten using WITH:

```
CREATE INVERTED INDEX [indexname] ON [tablename] ( [geospatial_column] ) WITH BOUNDING BOX (x_min, y_min, x_max, y_max)
CREATE INVERTED INDEX [indexname] ON [tablename] ( [geospatial_column] ) WITH (geometry_min_x = X, ...)
```

The default bounding box for 2D geometry will be decided after
consultation with users.

#### Disk Serialization

For Geometry and Geography types, we will serialize the data as
Expand Down Expand Up @@ -403,7 +400,21 @@ Example changed installation instructions:
$ wget -qO- https://binaries.cockroachdb.com/cockroach-v20.2.0.linux-amd64.tgz | tar xvz
$ cp -i cockroach-v20.2.0.linux-amd64/cockroach /usr/local/bin/
# Optional step.
$ cp -i cockroach-v20.2.0.linux-amd64/libgeos_c.so /usr/local/lib/
$ cp -i cockroach-v20.2.0.linux-amd64/lib/*.so /usr/local/lib/cockroach/
```

However, GEOS will still be loaded provided it is in the same directory under `lib`
in which CockroachDB was executed from, e.g.:
```
$ wget -qO- https://binaries.cockroachdb.com/cockroach-v20.2.0.linux-amd64.tgz | tar xvz
$ cd cockroach-v20.2.0.linux-amd64
$ ls
lib
cockroach
$ ls lib
libgeos.so
libgeos_c.so
$ ./cockroach # this should have GEOS loaded.
```

If a user attempts to use a geospatial function without having the library installed,
Expand Down Expand Up @@ -488,8 +499,7 @@ PostGIS has a set of [bounding box
operators](https://postgis.net/docs/reference.html#idm9874) that are
not part of the OGC or SQL/MM specs. As mentioned above, our different
indexing approach prevents these from being index
accelerated. Therefore we have no plans to support these in the
initial implementation.
accelerated. We may decide to support these at a later date.


#### Distance Operators
Expand Down Expand Up @@ -524,7 +534,7 @@ For 2D geometry and geography, these are:
* ST_Within (geometry only)

These functions have an equivalent with an `_` prefix
(e.g.. _ST_Within) that avoids using the indexes. We will similarly
(e.g. `_ST_Within`) that avoids using the indexes. We will similarly
have such functions, in which the optimizer will know not to use the
indexes when doing these operations.

Expand Down Expand Up @@ -584,6 +594,38 @@ builtins for parsing and encoding them):
[RFC7946](https://tools.ietf.org/html/rfc7946). These are all
supported by twpayne/go-geom libraries.

Furthermore, we will aim to be compatible with importing and exporting
other data external file types using the [`ogr2ogr`](https://gdal.org/programs/ogr2ogr.html)
tool.

Initially, these imports can be directly ingested as PGDUMPs using the
`IMPORT PGDUMP` syntax. An example workflow:

```
$ ogr2ogr -f PGDUMP cockroach-data/import.sql import.gpkg -skipfailures
$ echo "IMPORT PGDUMP 'nodelocal://0/import.sql" | cockroach sql
```

For exports, a user can transform the dataset
into GeoJSON before using `ogr2ogr` to massage them into a different format.
An example workflow:

```
$ echo "SELECT json_build_object(
'type', 'FeatureCollection',
'name', 'nyc_subway_stations',
'features', json_agg(st_asgeojson(nyc_subway_stations.*)::jsonb)
) FROM nyc_subway_stations;" | cockroach sql --format=raw | head -n 4 | tail -n 1 > ~/subway.json
$ ogr2ogr -f GPKG ~/subway.gpkg ~/subway.json
```

Re-using `ogr2ogr` allows us to support file types such as:

* ESRI Shapefiles
* PGDUMPs
* Geodatabase
* Geopackage
* [and vector drivers mentioned here](https://gdal.org/drivers/vector/index.html)

### Spatial Reference Identifiers (SRIDs) and Projections

Expand Down Expand Up @@ -1674,3 +1716,8 @@ None beyond what is already mentioned in earlier text.
# Updates
* 2020-05-07:
* added ST_ContainsProperly as an indexable function.
* 2020-08-17:
* update install instructions
* update bbox notes
* update index specification
* added notes on ogr2ogr
4 changes: 4 additions & 0 deletions docs/generated/sql/functions.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,8 @@ has no relationship with the commit order of concurrent transactions.</p>
</span></td></tr>
<tr><td><a name="postgis_geos_version"></a><code>postgis_geos_version() &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Compatibility placeholder function with PostGIS. Returns a fixed string based on PostGIS 3.0.1, with minor edits.</p>
</span></td></tr>
<tr><td><a name="postgis_getbbox"></a><code>postgis_getbbox(geometry: geometry) &rarr; box2d</code></td><td><span class="funcdesc"><p>Returns a box2d encapsulating the given Geometry</p>
</span></td></tr>
<tr><td><a name="postgis_hasbbox"></a><code>postgis_hasbbox(geometry: geometry) &rarr; <a href="bool.html">bool</a></code></td><td><span class="funcdesc"><p>Returns whether a given Geometry has a bounding box. False for points and empty geometries; always true otherwise.</p>
</span></td></tr>
<tr><td><a name="postgis_lib_build_date"></a><code>postgis_lib_build_date() &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Compatibility placeholder function with PostGIS. Returns a fixed string based on PostGIS 3.0.1, with minor edits.</p>
Expand Down Expand Up @@ -1330,6 +1332,8 @@ calculated, the result is transformed back into a Geography with SRID 4326.</p>
<tr><td><a name="st_longestline"></a><code>st_longestline(geometry_a: geometry, geometry_b: geometry) &rarr; geometry</code></td><td><span class="funcdesc"><p>Returns the LineString corresponds to the max distance across every pair of points comprising the given geometries.</p>
<p>Note if geometries are the same, it will return the LineString with the maximum distance between the geometry’s vertexes. The function will return the longest line that was discovered first when comparing maximum distances if more than one is found.</p>
</span></td></tr>
<tr><td><a name="st_makebox2d"></a><code>st_makebox2d(geometry_a: geometry, geometry_b: geometry) &rarr; box2d</code></td><td><span class="funcdesc"><p>Creates a box2d from two points. Errors if arguments are not two non-empty points.</p>
</span></td></tr>
<tr><td><a name="st_makepoint"></a><code>st_makepoint(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_makepolygon"></a><code>st_makepolygon(geometry: geometry) &rarr; geometry</code></td><td><span class="funcdesc"><p>Returns a new Polygon with the given outer LineString.</p>
Expand Down
21 changes: 21 additions & 0 deletions pkg/geo/bbox.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,27 @@ func (b *CartesianBoundingBox) Covers(o *CartesianBoundingBox) bool {
b.LoY <= o.HiY && o.HiY <= b.HiY
}

// ToGeomT converts a BoundingBox to a GeomT.
func (b *CartesianBoundingBox) ToGeomT() geom.T {
if b.LoX == b.HiX && b.LoY == b.HiY {
return geom.NewPointFlat(geom.XY, []float64{b.LoX, b.LoY})
}
if b.LoX == b.HiX || b.LoY == b.HiY {
return geom.NewLineStringFlat(geom.XY, []float64{b.LoX, b.LoY, b.HiX, b.HiY})
}
return geom.NewPolygonFlat(
geom.XY,
[]float64{
b.LoX, b.LoY,
b.LoX, b.HiY,
b.HiX, b.HiY,
b.HiX, b.LoY,
b.LoX, b.LoY,
},
[]int{10},
)
}

// boundingBoxFromGeomT returns a bounding box from a given geom.T.
// Returns nil if no bounding box was found.
func boundingBoxFromGeomT(g geom.T, soType geopb.SpatialObjectType) (*geopb.BoundingBox, error) {
Expand Down
40 changes: 11 additions & 29 deletions pkg/geo/geomfn/envelope.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ package geomfn

import (
"github.com/cockroachdb/cockroach/pkg/geo"
"github.com/cockroachdb/errors"
"github.com/twpayne/go-geom"
)

Expand All @@ -22,34 +23,15 @@ func Envelope(g *geo.Geometry) (*geo.Geometry, error) {
if g.Empty() {
return g, nil
}
bbox := g.CartesianBoundingBox()
if bbox.LoX == bbox.HiX && bbox.LoY == bbox.HiY {
return geo.NewGeometryFromGeomT(
geom.NewPointFlat(geom.XY, []float64{bbox.LoX, bbox.LoY}).SetSRID(int(g.SRID())),
)
t := g.CartesianBoundingBox().ToGeomT()
switch t := t.(type) {
case *geom.Point:
return geo.NewGeometryFromGeomT(t.SetSRID(int(g.SRID())))
case *geom.LineString:
return geo.NewGeometryFromGeomT(t.SetSRID(int(g.SRID())))
case *geom.Polygon:
return geo.NewGeometryFromGeomT(t.SetSRID(int(g.SRID())))
default:
return nil, errors.Newf("unknown geom type: %T", t)
}
if bbox.LoX == bbox.HiX || bbox.LoY == bbox.HiY {
return geo.NewGeometryFromGeomT(
geom.NewLineStringFlat(
geom.XY,
[]float64{
bbox.LoX, bbox.LoY,
bbox.HiX, bbox.HiY,
},
).SetSRID(int(g.SRID())),
)
}
return geo.NewGeometryFromGeomT(
geom.NewPolygonFlat(
geom.XY,
[]float64{
bbox.LoX, bbox.LoY,
bbox.LoX, bbox.HiY,
bbox.HiX, bbox.HiY,
bbox.HiX, bbox.LoY,
bbox.LoX, bbox.LoY,
},
[]int{10},
).SetSRID(int(g.SRID())),
)
}
6 changes: 5 additions & 1 deletion pkg/sql/conn_executor_exec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,11 @@ func (ex *connExecutor) execPortal(
// Note that the portal is considered exhausted regardless of
// the fact whether an error occurred or not - if it did, we
// still don't want to re-execute the portal from scratch.
ex.exhaustPortal(portalName)
// The current statement may have just closed and deleted the portal,
// so only exhaust it if it still exists.
if _, ok := ex.extraTxnState.prepStmtsNamespace.portals[portalName]; ok {
ex.exhaustPortal(portalName)
}
}
default:
ev, payload, err = ex.execStmt(stmtCtx, curStmt, stmtRes, pinfo)
Expand Down
35 changes: 24 additions & 11 deletions pkg/sql/logictest/testdata/logic_test/geospatial
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,6 @@ CREATE TABLE public.geog_table_negative_values (
statement error SRID 3857 cannot be used for geography as it is not in a lon/lat coordinate system
SELECT 'SRID=3857;POINT(1.0 2.0)'::geography

query BTT
SELECT
PostGIS_HasBBox(geom),
PostGIS_AddBBox(geom),
PostGIS_DropBBox(geom)
FROM geo_table
ORDER BY id ASC
----
false 010100000000000000000000400000000000000040 010100000000000000000000400000000000000040
false 0101000000000000000000F03F000000000000F03F 0101000000000000000000F03F000000000000F03F

query ITTT rowsort
SELECT * FROM geo_table
----
Expand Down Expand Up @@ -881,6 +870,30 @@ Square (left) 1
Square (right) 1
Square overlapping left and right square 1.1

# *BBox operators
query TBTTT
SELECT
dsc,
PostGIS_HasBBox(geom),
ST_AsEWKT(PostGIS_AddBBox(geom)),
PostGIS_GetBBox(geom),
ST_AsEWKT(PostGIS_DropBBox(geom))
FROM geom_operators_test
ORDER BY dsc ASC
----
Empty GeometryCollection false GEOMETRYCOLLECTION EMPTY NULL GEOMETRYCOLLECTION EMPTY
Empty LineString false LINESTRING EMPTY NULL LINESTRING EMPTY
Empty Point false POINT EMPTY NULL POINT EMPTY
Faraway point false POINT (5 5) BOX(5 5,5 5) POINT (5 5)
Line going through left and right square true LINESTRING (-0.5 0.5, 0.5 0.5) BOX(-0.5 0.5,0.5 0.5) LINESTRING (-0.5 0.5, 0.5 0.5)
NULL NULL NULL NULL NULL
Nested Geometry Collection true GEOMETRYCOLLECTION (GEOMETRYCOLLECTION (POINT (0 0))) BOX(0 0,0 0) GEOMETRYCOLLECTION (GEOMETRYCOLLECTION (POINT (0 0)))
Point middle of Left Square false POINT (-0.5 0.5) BOX(-0.5 0.5,-0.5 0.5) POINT (-0.5 0.5)
Point middle of Right Square false POINT (0.5 0.5) BOX(0.5 0.5,0.5 0.5) POINT (0.5 0.5)
Square (left) true POLYGON ((-1 0, 0 0, 0 1, -1 1, -1 0)) BOX(-1 0,0 1) POLYGON ((-1 0, 0 0, 0 1, -1 1, -1 0))
Square (right) true POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0)) BOX(0 0,1 1) POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))
Square overlapping left and right square true POLYGON ((-0.1 0, 1 0, 1 1, -0.1 1, -0.1 0)) BOX(-0.1 0,1 1) POLYGON ((-0.1 0, 1 0, 1 1, -0.1 1, -0.1 0))

# Validity checking.
query TBBBTTTT
SELECT
Expand Down
60 changes: 60 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/geospatial_bbox
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,32 @@ SELECT * FROM box2d_encoding_test ORDER BY id ASC
1 BOX(1 2,3 4) BOX(3 4,5 6) {"BOX(-1 -2,-3 -4)"}
2 BOX(10.1 20.1,30.5 40.6) BOX(30 40,50 60) {"BOX(-1 -2,-3 -4)","BOX(3 -4,5 -6)"}

subtest st_makebox2d

statement error first argument is not a POINT
SELECT ST_MakeBox2D('LINESTRING(0 0, 1 1)', 'POINT(1 0)')

statement error second argument is not a POINT
SELECT ST_MakeBox2D('POINT(1 0)', 'LINESTRING(0 0, 1 1)')

statement error cannot use POINT EMPTY
SELECT ST_MakeBox2D('POINT(1 0)', 'POINT EMPTY')

statement error cannot use POINT EMPTY
SELECT ST_MakeBox2D('POINT EMPTY', 'POINT (1 0)')

statement error operation on mixed SRIDs forbidden
SELECT ST_MakeBox2D('SRID=4326;POINT(1 0)', 'SRID=3857;POINT(1 0)')

query T
SELECT ST_MakeBox2D(a::geometry, b::geometry) FROM ( VALUES
('POINT (1 0)', 'POINT(5 6)'),
('POINT (1 0)', 'POINT(1 0)')
) tbl(a, b)
----
BOX(1 0,5 6)
BOX(1 0,1 0)

subtest comparison_ops

statement ok
Expand Down Expand Up @@ -135,3 +161,37 @@ NULL
BOX(-1 -1,1 1)
BOX(4 -5,4 -5)
BOX(-1 -5,4 1)

subtest cast_test

query T
SELECT
ST_AsEWKT(b::box2d::geometry)
FROM ( VALUES
(NULL),
('box(-1 -1,-1 -1)'),
('box(1 3,20 3)'),
('box(5.5 -10, 5.5 60)'),
('box(-1 -2, 4 6)')
) tbl(b)
----
NULL
POINT (-1 -1)
LINESTRING (1 3, 20 3)
LINESTRING (5.5 -10, 5.5 60)
POLYGON ((-1 -2, -1 6, 4 6, 4 -2, -1 -2))

query T
SELECT
g::geometry::box2d
FROM ( VALUES
('point empty'),
(null),
('point(5 5)'),
('linestring(4 5, 9 10)')
) tbl(g)
----
NULL
NULL
BOX(5 5,5 5)
BOX(4 5,9 10)
Loading

0 comments on commit f41ff14

Please sign in to comment.