Skip to content

Commit

Permalink
Merge #48552 #48659
Browse files Browse the repository at this point in the history
48552: geo/geomfn: implement ST_Relate and ST_ContainsProperly r=sumeerbhola a=otan

Commit on top of #48522.

Added ST_ContainsProperly to the optimizer as well that calls it to use
Covers.

Also update the RFC to claim ST_ContainsProperly as indexed backed.

Release note (sql change): Implemented the geometry based builtins
`ST_Relate` and `ST_ContainsProperly`.

48659: colexec: fix type schema for LEFT SEMI and LEFT ANTI joins r=yuzefovich a=yuzefovich

LEFT SEMI and LEFT ANTI joins output only all the columns from the left,
however, we mistakenly put the columns from the right into
`result.ColumnTypes`.

Fixes: #48622.

Release note (bug fix): Previously, CockroachDB could encounter an
internal error when a query with LEFT SEMI or LEFT ANTI join was
performed via the vectorized execution engine in some cases, and now
this has been fixed. This is likely to occur only with `vectorize=on`
setting.

Co-authored-by: Oliver Tan <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
  • Loading branch information
3 people committed May 12, 2020
3 parents 91b2880 + 616821a + 4b18643 commit bb7a423
Show file tree
Hide file tree
Showing 15 changed files with 471 additions and 62 deletions.
7 changes: 7 additions & 0 deletions docs/RFCS/20200421_geospatial.md
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,7 @@ For 2D geometry and geography, these are:
* ST_Covers
* ST_CoveredBy
* ST_Contains (geometry only)
* ST_ContainsProperly (geometry only)
* ST_Crosses (geometry only)
* ST_DFullyWithin (geometry only)
* ST_DWithin
Expand Down Expand Up @@ -1039,6 +1040,8 @@ Functions map to the index functions:
contains(g, x)
* ST_Contains(g, x), ST_Contains(x, g): use contains(g, x) or
contained-by(g, x)
* ST_ContainsProperly(g, x), ST_ContainsProperly(x, g): use contains(g, x) or
contained-by(g, x)
* ST_Crosses: use intersects
* ST_DFullyWithin(g, x, d), ST_DFullyWithin(x, g, d): extend g by
distance d to produce a shape g’, and then use contains(g', x). The
Expand Down Expand Up @@ -1667,3 +1670,7 @@ good stretch option as well.
## Unresolved questions

None beyond what is already mentioned in earlier text.

# Updates
* 2020-05-07:
* added ST_ContainsProperly as an indexable function.
10 changes: 10 additions & 0 deletions docs/generated/sql/functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,10 @@ has no relationship with the commit order of concurrent transactions.</p>
<p>This function uses the GEOS module.</p>
<p>This function will automatically use any available index.</p>
</span></td></tr>
<tr><td><a name="st_containsproperly"></a><code>st_containsproperly(geometry_a: geometry, geometry_b: geometry) &rarr; <a href="bool.html">bool</a></code></td><td><span class="funcdesc"><p>Returns true if geometry_b intersects the interior of geometry_a but not the boundary or exterior of geometry_a.</p>
<p>This function uses the GEOS module.</p>
<p>This function will automatically use any available index.</p>
</span></td></tr>
<tr><td><a name="st_coveredby"></a><code>st_coveredby(geometry_a: geometry, geometry_b: geometry) &rarr; <a href="bool.html">bool</a></code></td><td><span class="funcdesc"><p>Returns true if no point in geometry_a is outside geometry_b.</p>
<p>This function uses the GEOS module.</p>
<p>This function will automatically use any available index.</p>
Expand Down Expand Up @@ -890,6 +894,12 @@ has no relationship with the commit order of concurrent transactions.</p>
</span></td></tr>
<tr><td><a name="st_polygonfromwkb"></a><code>st_polygonfromwkb(wkb: <a href="bytes.html">bytes</a>, srid: <a href="int.html">int</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Returns the Geometry from a WKB representation with an SRID. If the shape underneath is not Polygon, NULL is returned.</p>
</span></td></tr>
<tr><td><a name="st_relate"></a><code>st_relate(geometry_a: geometry, geometry_b: geometry) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Returns the DE-9IM spatial relation between geometry_a and geometry_b.</p>
<p>This function uses the GEOS module.</p>
</span></td></tr>
<tr><td><a name="st_relate"></a><code>st_relate(geometry_a: geometry, geometry_b: geometry, pattern: <a href="string.html">string</a>) &rarr; <a href="bool.html">bool</a></code></td><td><span class="funcdesc"><p>Returns whether the DE-9IM spatial relation between geometry_a and geometry_b matches the DE-9IM pattern.</p>
<p>This function uses the GEOS module.</p>
</span></td></tr>
<tr><td><a name="st_touches"></a><code>st_touches(geometry_a: geometry, geometry_b: geometry) &rarr; <a href="bool.html">bool</a></code></td><td><span class="funcdesc"><p>Returns true if the only points in common between geometry_a and geometry_b are on the boundary. Note points do not touch other points.</p>
<p>This function uses the GEOS module.</p>
<p>This function will automatically use any available index.</p>
Expand Down
10 changes: 10 additions & 0 deletions pkg/geo/geomfn/binary_predicates.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,16 @@ func Contains(a *geo.Geometry, b *geo.Geometry) (bool, error) {
return geos.Contains(a.EWKB(), b.EWKB())
}

// ContainsProperly returns whether geometry A properly contains geometry B.
func ContainsProperly(a *geo.Geometry, b *geo.Geometry) (bool, error) {
// No GEOS CAPI to call ContainsProperly; fallback to Relate.
relate, err := Relate(a, b)
if err != nil {
return false, err
}
return MatchesDE9IM(relate, "T**FF*FF*")
}

// Crosses returns whether geometry A crosses geometry B.
func Crosses(a *geo.Geometry, b *geo.Geometry) (bool, error) {
if a.SRID() != b.SRID() {
Expand Down
27 changes: 27 additions & 0 deletions pkg/geo/geomfn/binary_predicates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ func TestContains(t *testing.T) {
}{
{rightRect, rightRectPoint, true},
{rightRectPoint, rightRect, false},
{rightRectPoint, rightRectPoint, true},
{rightRect, rightRect, true},
{leftRect, rightRect, false},
}

Expand All @@ -102,6 +104,31 @@ func TestContains(t *testing.T) {
})
}

func TestContainsProperly(t *testing.T) {
testCases := []struct {
a *geo.Geometry
b *geo.Geometry
expected bool
}{
{rightRect, rightRect, false},
{rightRect, rightRectPoint, true},
{rightRectPoint, rightRectPoint, true},
}

for i, tc := range testCases {
t.Run(fmt.Sprintf("tc:%d", i), func(t *testing.T) {
g, err := ContainsProperly(tc.a, tc.b)
require.NoError(t, err)
require.Equal(t, tc.expected, g)
})
}

t.Run("errors if SRIDs mismatch", func(t *testing.T) {
_, err := ContainsProperly(mismatchingSRIDGeometryA, mismatchingSRIDGeometryB)
requireMismatchingSRIDError(t, err)
})
}

func TestCrosses(t *testing.T) {
testCases := []struct {
a *geo.Geometry
Expand Down
74 changes: 74 additions & 0 deletions pkg/geo/geomfn/de9im.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// Copyright 2020 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package geomfn

import (
"github.com/cockroachdb/cockroach/pkg/geo"
"github.com/cockroachdb/cockroach/pkg/geo/geos"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/errors"
)

// Relate returns the DE-9IM relation between A and B.
func Relate(a *geo.Geometry, b *geo.Geometry) (string, error) {
if a.SRID() != b.SRID() {
return "", geo.NewMismatchingSRIDsError(a, b)
}
return geos.Relate(a.EWKB(), b.EWKB())
}

// MatchesDE9IM checks whether the given DE-9IM relation matches the DE-91M pattern.
// Assumes the relation has been computed, and such has no 'T' and '*' characters.
// See: https://en.wikipedia.org/wiki/DE-9IM.
func MatchesDE9IM(relation string, pattern string) (bool, error) {
if len(relation) != 9 {
return false, errors.Newf("relation %q should be of length 9", relation)
}
if len(pattern) != 9 {
return false, errors.Newf("pattern %q should be of length 9", pattern)
}
for i := 0; i < len(relation); i++ {
matches, err := relationByteMatchesPatternByte(relation[i], pattern[i])
if err != nil {
return false, err
}
if !matches {
return false, nil
}
}
return true, nil
}

// relationByteMatchesPatternByte matches a single byte of a DE-9IM relation
// against the DE-9IM pattern.
// Pattern matches are as follows:
// * '*': allow anything.
// * 't'/'T': allow only if the relation is true. This means the relation must be
// '0' (point), '1' (line) or '2' (area) - which is the dimensionality of the
// intersection.
// * 'f'/'F': allow only if relation is also false, which is of the form 'f'/'F'.
func relationByteMatchesPatternByte(r byte, p byte) (bool, error) {
switch util.ToLowerSingleByte(p) {
case '*':
return true, nil
case 't':
if r < '0' || r > '2' {
return false, nil
}
case 'f':
if util.ToLowerSingleByte(r) != 'f' {
return false, nil
}
default:
return false, errors.Newf("unrecognized pattern character: %s", string(p))
}
return true, nil
}
72 changes: 72 additions & 0 deletions pkg/geo/geomfn/de9im_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// Copyright 2020 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package geomfn

import (
"fmt"
"testing"

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

func TestRelate(t *testing.T) {
testCases := []struct {
a *geo.Geometry
b *geo.Geometry
expected string
}{
{leftRect, rightRect, "FF2F11212"},
}

for i, tc := range testCases {
t.Run(fmt.Sprintf("tc:%d", i), func(t *testing.T) {
ret, err := Relate(tc.a, tc.b)
require.NoError(t, err)
require.Equal(t, tc.expected, ret)
})
}
}

func TestMatchesDE9IM(t *testing.T) {
testCases := []struct {
str string
pattern string
expected bool
expectedError string
}{
{"", "T**FF*FF*", false, `relation "" should be of length 9`},
{"TTTTTTTTT", "T**FF*FF*T", false, `pattern "T**FF*FF*T" should be of length 9`},
{"TTTTTTTTT", "T**FF*FF*T", false, `pattern "T**FF*FF*T" should be of length 9`},
{"000FFF000", "cTTFfFTTT", false, `unrecognized pattern character: c`},
{"120FFF021", "TTTFfFTTT", true, ""},
{"02FFFF000", "T**FfFTTT", true, ""},
{"020F1F010", "TTTFFFTtT", false, ""},
{"020FFF0f0", "TTTFFFTtT", false, ""},
}

for _, tc := range testCases {
t.Run(fmt.Sprintf("%s has pattern %s", tc.str, tc.pattern), func(t *testing.T) {
ret, err := MatchesDE9IM(tc.str, tc.pattern)
if tc.expectedError == "" {
require.NoError(t, err)
require.Equal(t, tc.expected, ret)
} else {
require.EqualError(t, err, tc.expectedError)
}
})
}

t.Run("errors if SRIDs mismatch", func(t *testing.T) {
_, err := Relate(mismatchingSRIDGeometryA, mismatchingSRIDGeometryB)
requireMismatchingSRIDError(t, err)
})
}
40 changes: 39 additions & 1 deletion pkg/geo/geos/geos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ typedef void (*CR_GEOS_finish_r)(CR_GEOS_Handle);
typedef CR_GEOS_MessageHandler (*CR_GEOS_Context_setErrorMessageHandler_r)(CR_GEOS_Handle,
CR_GEOS_MessageHandler,
void*);

typedef void (*CR_GEOS_Free_r)(CR_GEOS_Handle, void* buffer);
typedef void (*CR_GEOS_SetSRID_r)(CR_GEOS_Handle, CR_GEOS_Geometry, int);
typedef int (*CR_GEOS_GetSRID_r)(CR_GEOS_Handle, CR_GEOS_Geometry);
typedef void (*CR_GEOS_GeomDestroy_r)(CR_GEOS_Handle, CR_GEOS_Geometry);
Expand Down Expand Up @@ -78,6 +78,8 @@ typedef char (*CR_GEOS_Overlaps_r)(CR_GEOS_Handle, CR_GEOS_Geometry, CR_GEOS_Geo
typedef char (*CR_GEOS_Touches_r)(CR_GEOS_Handle, CR_GEOS_Geometry, CR_GEOS_Geometry);
typedef char (*CR_GEOS_Within_r)(CR_GEOS_Handle, CR_GEOS_Geometry, CR_GEOS_Geometry);

typedef char* (*CR_GEOS_Relate_r)(CR_GEOS_Handle, CR_GEOS_Geometry, CR_GEOS_Geometry);

typedef CR_GEOS_WKBWriter (*CR_GEOS_WKBWriter_create_r)(CR_GEOS_Handle);
typedef char* (*CR_GEOS_WKBWriter_write_r)(CR_GEOS_Handle, CR_GEOS_WKBWriter, CR_GEOS_Geometry,
size_t*);
Expand All @@ -99,6 +101,7 @@ struct CR_GEOS {
CR_GEOS_init_r GEOS_init_r;
CR_GEOS_finish_r GEOS_finish_r;
CR_GEOS_Context_setErrorMessageHandler_r GEOSContext_setErrorMessageHandler_r;
CR_GEOS_Free_r GEOSFree_r;

CR_GEOS_SetSRID_r GEOSSetSRID_r;
CR_GEOS_GetSRID_r GEOSGetSRID_r;
Expand Down Expand Up @@ -127,6 +130,8 @@ struct CR_GEOS {
CR_GEOS_Touches_r GEOSTouches_r;
CR_GEOS_Within_r GEOSWithin_r;

CR_GEOS_Relate_r GEOSRelate_r;

CR_GEOS_WKBWriter_create_r GEOSWKBWriter_create_r;
CR_GEOS_WKBWriter_destroy_r GEOSWKBWriter_destroy_r;
CR_GEOS_WKBWriter_setByteOrder_r GEOSWKBWriter_setByteOrder_r;
Expand Down Expand Up @@ -154,6 +159,7 @@ struct CR_GEOS {

INIT(GEOS_init_r);
INIT(GEOS_finish_r);
INIT(GEOSFree_r);
INIT(GEOSContext_setErrorMessageHandler_r);
INIT(GEOSGeom_destroy_r);
INIT(GEOSSetSRID_r);
Expand All @@ -170,6 +176,7 @@ struct CR_GEOS {
INIT(GEOSOverlaps_r);
INIT(GEOSTouches_r);
INIT(GEOSWithin_r);
INIT(GEOSRelate_r);
INIT(GEOSWKTReader_create_r);
INIT(GEOSWKTReader_destroy_r);
INIT(GEOSWKTReader_read_r);
Expand Down Expand Up @@ -431,3 +438,34 @@ CR_GEOS_Status CR_GEOS_Touches(CR_GEOS* lib, CR_GEOS_Slice a, CR_GEOS_Slice b, c
CR_GEOS_Status CR_GEOS_Within(CR_GEOS* lib, CR_GEOS_Slice a, CR_GEOS_Slice b, char *ret) {
return CR_GEOS_BinaryPredicate(lib, lib->GEOSWithin_r, a, b, ret);
}

//
// DE-9IM related
// See: https://en.wikipedia.org/wiki/DE-9IM.
//

CR_GEOS_Status CR_GEOS_Relate(CR_GEOS* lib, CR_GEOS_Slice a, CR_GEOS_Slice b, CR_GEOS_String *ret) {
std::string error;
auto handle = initHandleWithErrorBuffer(lib, &error);

auto wkbReader = lib->GEOSWKBReader_create_r(handle);
auto geomA = lib->GEOSWKBReader_read_r(handle, wkbReader, a.data, a.len);
auto geomB = lib->GEOSWKBReader_read_r(handle, wkbReader, b.data, b.len);
lib->GEOSWKBReader_destroy_r(handle, wkbReader);

if (geomA != nullptr && geomB != nullptr) {
auto r = lib->GEOSRelate_r(handle, geomA, geomB);
if (r != NULL) {
*ret = toGEOSString(r, strlen(r));
lib->GEOSFree_r(handle, r);
}
}
if (geomA != nullptr) {
lib->GEOSGeom_destroy_r(handle, geomA);
}
if (geomB != nullptr) {
lib->GEOSGeom_destroy_r(handle, geomB);
}
lib->GEOS_finish_r(handle);
return toGEOSString(error.data(), error.length());
}
20 changes: 20 additions & 0 deletions pkg/geo/geos/geos.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,3 +399,23 @@ func Within(a geopb.EWKB, b geopb.EWKB) (bool, error) {
}
return ret == 1, nil
}

//
// DE-9IM related
//

// Relate returns the DE-9IM relation between A and B.
func Relate(a geopb.EWKB, b geopb.EWKB) (string, error) {
g, err := ensureInitInternal()
if err != nil {
return "", err
}
var ret C.CR_GEOS_String
if err := statusToError(C.CR_GEOS_Relate(g, goToCSlice(a), goToCSlice(b), &ret)); err != nil {
return "", err
}
if ret.data == nil {
return "", errors.Newf("expected DE-9IM string but found nothing")
}
return string(cStringToSafeGoBytes(ret)), nil
}
5 changes: 5 additions & 0 deletions pkg/geo/geos/geos.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ CR_GEOS_Status CR_GEOS_Overlaps(CR_GEOS* lib, CR_GEOS_Slice a, CR_GEOS_Slice b,
CR_GEOS_Status CR_GEOS_Touches(CR_GEOS* lib, CR_GEOS_Slice a, CR_GEOS_Slice b, char *ret);
CR_GEOS_Status CR_GEOS_Within(CR_GEOS* lib, CR_GEOS_Slice a, CR_GEOS_Slice b, char *ret);

//
// DE-9IM related
//

CR_GEOS_Status CR_GEOS_Relate(CR_GEOS *lib, CR_GEOS_Slice a, CR_GEOS_Slice b, CR_GEOS_String *ret);
#ifdef __cplusplus
} // extern "C"
#endif
Loading

0 comments on commit bb7a423

Please sign in to comment.