Skip to content

Commit

Permalink
opt: add rule to replace ST_Distance with ST_DWithin
Browse files Browse the repository at this point in the history 
`ST_DWithin` is equivalent to the expression `ST_Distance <= x`.
Similar reformulations can be made for different comparison operators
(<, >, >=). This commit adds two rules that replace expressions of the
latter form with either `ST_DWithin` or `ST_DWithinExclusive`. This
replacement is desirable because the `ST_DWithin` function can exit early.
`ST_DWithin` can also be used to generate an inverted index scan.

Fixes cockroachdb#52028

Release note: None
  • Loading branch information
@DrewKimball
DrewKimball committed Aug 19, 2020
1 parent 2088d55 commit 1d5fe90
Show file tree
Hide file tree
Showing 14 changed files with 481 additions and 65 deletions.
44 changes: 31 additions & 13 deletions docs/generated/sql/functions.md
View file

Large diffs are not rendered by default.

14 changes: 14 additions & 0 deletions pkg/geo/geo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,20 @@ const (
EmptyBehaviorOmit EmptyBehavior = 1
)

// FnExclusivity is used to indicate whether a geo function should have
// inclusive or exclusive semantics. For example, DWithin == (Distance <= x),
// while DWithinExclusive == (Distance < x).
type FnExclusivity bool

const (
// FnExclusive indicates that the corresponding geo function should have
// exclusive semantics.
FnExclusive FnExclusivity = true
// FnInclusive indicates that the corresponding geo function should have
// inclusive semantics.
FnInclusive FnExclusivity = false
)

//
// Geospatial Type
//
Expand Down
14 changes: 6 additions & 8 deletions pkg/geo/geogfn/distance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ func Distance(
aRegions,
bRegions,
a.BoundingRect().Intersects(b.BoundingRect()),
0, /* stopAfter */
false, /* exclusive */
0, /* stopAfter */
geo.FnInclusive,
)
}

Expand Down Expand Up @@ -201,7 +201,7 @@ func distanceGeographyRegions(
bRegions []s2.Region,
boundingBoxIntersects bool,
stopAfter float64,
exclusive bool,
exclusive geo.FnExclusivity,
) (float64, error) {
minDistance := math.MaxFloat64
for _, aRegion := range aRegions {
Expand Down Expand Up @@ -248,7 +248,7 @@ type geographyMinDistanceUpdater struct {
minEdge s2.Edge
minD s1.ChordAngle
stopAfter s1.ChordAngle
exclusive bool
exclusive geo.FnExclusivity
}

var _ geodist.DistanceUpdater = (*geographyMinDistanceUpdater)(nil)
Expand All @@ -259,7 +259,7 @@ func newGeographyMinDistanceUpdater(
spheroid *geographiclib.Spheroid,
useSphereOrSpheroid UseSphereOrSpheroid,
stopAfter float64,
exclusive bool,
exclusive geo.FnExclusivity,
) *geographyMinDistanceUpdater {
multiplier := 1.0
if useSphereOrSpheroid == UseSpheroid {
Expand Down Expand Up @@ -302,9 +302,7 @@ func (u *geographyMinDistanceUpdater) Update(aPoint geodist.Point, bPoint geodis
// If we have a threshold, determine if we can stop early.
// If the sphere distance is within range of the stopAfter, we can
// definitively say we've reach the close enough point.
if !u.exclusive && u.minD <= u.stopAfter {
return true
} else if u.exclusive && u.minD < u.stopAfter {
if (!u.exclusive && u.minD <= u.stopAfter) || (u.exclusive && u.minD < u.stopAfter) {
return true
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/geo/geogfn/dwithin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ import (
)

// DWithin returns whether a is within distance d of b. If A or B contains empty
// Geography objects, this will return false. If exclusive is false, DWithin is
// Geography objects, this will return false. If inclusive, DWithin is
// equivalent to Distance(a, b) <= d. Otherwise, DWithin is instead equivalent
// to Distance(a, b) < d.
func DWithin(
a *geo.Geography,
b *geo.Geography,
distance float64,
useSphereOrSpheroid UseSphereOrSpheroid,
exclusive bool,
exclusive geo.FnExclusivity,
) (bool, error) {
if a.SRID() != b.SRID() {
return false, geo.NewMismatchingSRIDsError(a, b)
Expand Down
40 changes: 20 additions & 20 deletions pkg/geo/geogfn/dwithin_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,26 +48,26 @@ func TestDWithin(t *testing.T) {
}
for _, val := range []float64{zeroValue, 1, 10, 10000} {
t.Run(fmt.Sprintf("dwithin:%f", val), func(t *testing.T) {
dwithin, err := DWithin(a, b, val, subTC.useSphereOrSpheroid, false /* exclusive */)
dwithin, err := DWithin(a, b, val, subTC.useSphereOrSpheroid, geo.FnInclusive)
require.NoError(t, err)
require.True(t, dwithin)

dwithin, err = DWithin(b, a, val, subTC.useSphereOrSpheroid, false /* exclusive */)
dwithin, err = DWithin(b, a, val, subTC.useSphereOrSpheroid, geo.FnInclusive)
require.NoError(t, err)
require.True(t, dwithin)
})
t.Run(fmt.Sprintf("dwithinexclusive:%f", val), func(t *testing.T) {
t.Run(fmt.Sprintf("FnExclusive:%f", val), func(t *testing.T) {
exclusiveExpected := true
if val == subTC.expected {
exclusiveExpected = false
}
dwithin, err := DWithin(a, b, val, subTC.useSphereOrSpheroid, true /* exclusive */)
dwithin, err := DWithin(a, b, val, subTC.useSphereOrSpheroid, geo.FnExclusive)
require.NoError(t, err)
require.Equal(t, dwithin, exclusiveExpected)
require.Equal(t, exclusiveExpected, dwithin)

dwithin, err = DWithin(b, a, val, subTC.useSphereOrSpheroid, true /* exclusive */)
dwithin, err = DWithin(b, a, val, subTC.useSphereOrSpheroid, geo.FnExclusive)
require.NoError(t, err)
require.Equal(t, dwithin, exclusiveExpected)
require.Equal(t, exclusiveExpected, dwithin)
})
}
} else {
Expand All @@ -78,20 +78,20 @@ func TestDWithin(t *testing.T) {
subTC.expected * 2,
} {
t.Run(fmt.Sprintf("dwithin:%f", val), func(t *testing.T) {
dwithin, err := DWithin(a, b, val, subTC.useSphereOrSpheroid, false /* exclusive */)
dwithin, err := DWithin(a, b, val, subTC.useSphereOrSpheroid, geo.FnInclusive)
require.NoError(t, err)
require.True(t, dwithin)

dwithin, err = DWithin(b, a, val, subTC.useSphereOrSpheroid, false /* exclusive */)
dwithin, err = DWithin(b, a, val, subTC.useSphereOrSpheroid, geo.FnInclusive)
require.NoError(t, err)
require.True(t, dwithin)
})
t.Run(fmt.Sprintf("dwithinexclusive:%f", val), func(t *testing.T) {
dwithin, err := DWithin(a, b, val, subTC.useSphereOrSpheroid, true /* exclusive */)
t.Run(fmt.Sprintf("FnExclusive:%f", val), func(t *testing.T) {
dwithin, err := DWithin(a, b, val, subTC.useSphereOrSpheroid, geo.FnExclusive)
require.NoError(t, err)
require.True(t, dwithin)

dwithin, err = DWithin(b, a, val, subTC.useSphereOrSpheroid, true /* exclusive */)
dwithin, err = DWithin(b, a, val, subTC.useSphereOrSpheroid, geo.FnExclusive)
require.NoError(t, err)
require.True(t, dwithin)
})
Expand All @@ -104,20 +104,20 @@ func TestDWithin(t *testing.T) {
subTC.expected / 2,
} {
t.Run(fmt.Sprintf("dwithin:%f", val), func(t *testing.T) {
dwithin, err := DWithin(a, b, val, subTC.useSphereOrSpheroid, false /* exclusive */)
dwithin, err := DWithin(a, b, val, subTC.useSphereOrSpheroid, geo.FnInclusive)
require.NoError(t, err)
require.False(t, dwithin)

dwithin, err = DWithin(b, a, val, subTC.useSphereOrSpheroid, false /* exclusive */)
dwithin, err = DWithin(b, a, val, subTC.useSphereOrSpheroid, geo.FnInclusive)
require.NoError(t, err)
require.False(t, dwithin)
})
t.Run(fmt.Sprintf("dwithinexclusive:%f", val), func(t *testing.T) {
dwithin, err := DWithin(a, b, val, subTC.useSphereOrSpheroid, true /* exclusive */)
t.Run(fmt.Sprintf("FnExclusive:%f", val), func(t *testing.T) {
dwithin, err := DWithin(a, b, val, subTC.useSphereOrSpheroid, geo.FnExclusive)
require.NoError(t, err)
require.False(t, dwithin)

dwithin, err = DWithin(b, a, val, subTC.useSphereOrSpheroid, true /* exclusive */)
dwithin, err = DWithin(b, a, val, subTC.useSphereOrSpheroid, geo.FnExclusive)
require.NoError(t, err)
require.False(t, dwithin)
})
Expand All @@ -129,12 +129,12 @@ func TestDWithin(t *testing.T) {
}

t.Run("errors if SRIDs mismatch", func(t *testing.T) {
_, err := DWithin(mismatchingSRIDGeographyA, mismatchingSRIDGeographyB, 0, UseSpheroid, false /* exclusive */)
_, err := DWithin(mismatchingSRIDGeographyA, mismatchingSRIDGeographyB, 0, UseSpheroid, geo.FnInclusive)
requireMismatchingSRIDError(t, err)
})

t.Run("errors if distance < 0", func(t *testing.T) {
_, err := DWithin(geo.MustParseGeography("POINT(1.0 2.0)"), geo.MustParseGeography("POINT(3.0 4.0)"), -0.01, UseSpheroid, false /* exclusive */)
_, err := DWithin(geo.MustParseGeography("POINT(1.0 2.0)"), geo.MustParseGeography("POINT(3.0 4.0)"), -0.01, UseSpheroid, geo.FnInclusive)
require.Error(t, err)
})

Expand All @@ -156,7 +156,7 @@ func TestDWithin(t *testing.T) {
require.NoError(t, err)
b, err := geo.ParseGeography(tc.b)
require.NoError(t, err)
dwithin, err := DWithin(a, b, 0, useSphereOrSpheroid, false /* exclusive */)
dwithin, err := DWithin(a, b, 0, useSphereOrSpheroid, geo.FnInclusive)
require.NoError(t, err)
require.False(t, dwithin)
})
Expand Down
1 change: 1 addition & 0 deletions pkg/geo/geoindex/geoindex.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ var RelationshipMap = map[string]RelationshipType{
"st_overlaps": Intersects,
"st_touches": Intersects,
"st_within": CoveredBy,
"st_dwithinexclusive": DWithin,
}

// RelationshipReverseMap contains a default function for each of the
Expand Down
12 changes: 8 additions & 4 deletions pkg/geo/geomfn/distance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ func MaxDistance(a *geo.Geometry, b *geo.Geometry) (float64, error) {
// DWithin determines if any part of geometry A is within D units of geometry B.
// If exclusive is false, DWithin is equivalent to Distance(a, b) <= d.
// Otherwise, DWithin is equivalent to Distance(a, b) < d.
func DWithin(a *geo.Geometry, b *geo.Geometry, d float64, exclusive bool) (bool, error) {
func DWithin(
a *geo.Geometry, b *geo.Geometry, d float64, exclusive geo.FnExclusivity,
) (bool, error) {
if a.SRID() != b.SRID() {
return false, geo.NewMismatchingSRIDsError(a, b)
}
Expand Down Expand Up @@ -167,7 +169,7 @@ func minDistanceInternal(
b *geo.Geometry,
stopAfter float64,
emptyBehavior geo.EmptyBehavior,
exclusive bool,
exclusive geo.FnExclusivity,
) (float64, error) {
u := newGeomMinDistanceUpdater(stopAfter, exclusive)
c := &geomDistanceCalculator{updater: u, boundingBoxIntersects: a.CartesianBoundingBox().Intersects(b.CartesianBoundingBox())}
Expand Down Expand Up @@ -391,7 +393,7 @@ func (c *geomGeodistEdgeCrosser) ChainCrossing(p geodist.Point) (bool, geodist.P
type geomMinDistanceUpdater struct {
currentValue float64
stopAfter float64
exclusive bool
exclusive geo.FnExclusivity
// coordA represents the first vertex of the edge that holds the maximum distance.
coordA geom.Coord
// coordB represents the second vertex of the edge that holds the maximum distance.
Expand All @@ -404,7 +406,9 @@ var _ geodist.DistanceUpdater = (*geomMinDistanceUpdater)(nil)

// newGeomMinDistanceUpdater returns a new geomMinDistanceUpdater with the
// correct arguments set up.
func newGeomMinDistanceUpdater(stopAfter float64, exclusive bool) *geomMinDistanceUpdater {
func newGeomMinDistanceUpdater(
stopAfter float64, exclusive geo.FnExclusivity,
) *geomMinDistanceUpdater {
return &geomMinDistanceUpdater{
currentValue: math.MaxFloat64,
stopAfter: stopAfter,
Expand Down
22 changes: 11 additions & 11 deletions pkg/geo/geomfn/distance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -457,11 +457,11 @@ func TestDWithin(t *testing.T) {
tc.expectedMinDistance * 2,
} {
t.Run(fmt.Sprintf("dwithin:%f", val), func(t *testing.T) {
dwithin, err := DWithin(a, b, val, false /* exclusive */)
dwithin, err := DWithin(a, b, val, geo.FnInclusive)
require.NoError(t, err)
require.Equal(t, expected, dwithin)

dwithin, err = DWithin(b, a, val, false /* exclusive */)
dwithin, err = DWithin(b, a, val, geo.FnInclusive)
require.NoError(t, err)
require.Equal(t, expected, dwithin)
})
Expand All @@ -470,11 +470,11 @@ func TestDWithin(t *testing.T) {
if val == tc.expectedMinDistance {
exclusiveExpected = false
}
dwithin, err := DWithin(a, b, val, true /* exclusive */)
dwithin, err := DWithin(a, b, val, geo.FnExclusive)
require.NoError(t, err)
require.Equal(t, exclusiveExpected, dwithin)

dwithin, err = DWithin(b, a, val, true /* exclusive */)
dwithin, err = DWithin(b, a, val, geo.FnExclusive)
require.NoError(t, err)
require.Equal(t, exclusiveExpected, dwithin)
})
Expand All @@ -487,20 +487,20 @@ func TestDWithin(t *testing.T) {
} {
if val > 0 {
t.Run(fmt.Sprintf("dwithin:%f", val), func(t *testing.T) {
dwithin, err := DWithin(a, b, val, false /* exclusive */)
dwithin, err := DWithin(a, b, val, geo.FnInclusive)
require.NoError(t, err)
require.False(t, dwithin)

dwithin, err = DWithin(b, a, val, false /* exclusive */)
dwithin, err = DWithin(b, a, val, geo.FnInclusive)
require.NoError(t, err)
require.False(t, dwithin)
})
t.Run(fmt.Sprintf("dwithinexclusive:%f", val), func(t *testing.T) {
dwithin, err := DWithin(a, b, val, true /* exclusive */)
dwithin, err := DWithin(a, b, val, geo.FnExclusive)
require.NoError(t, err)
require.False(t, dwithin)

dwithin, err = DWithin(b, a, val, true /* exclusive */)
dwithin, err = DWithin(b, a, val, geo.FnExclusive)
require.NoError(t, err)
require.False(t, dwithin)
})
Expand All @@ -516,7 +516,7 @@ func TestDWithin(t *testing.T) {
require.NoError(t, err)
b, err := geo.ParseGeometry(tc.b)
require.NoError(t, err)
dwithin, err := DWithin(a, b, 0, false /* exclusive */)
dwithin, err := DWithin(a, b, 0, geo.FnInclusive)
require.NoError(t, err)
require.False(t, dwithin)
})
Expand All @@ -529,7 +529,7 @@ func TestDWithin(t *testing.T) {
})

t.Run("errors if distance < 0", func(t *testing.T) {
_, err := DWithin(geo.MustParseGeometry("POINT(1.0 2.0)"), geo.MustParseGeometry("POINT(3.0 4.0)"), -0.01, false /* exclusive */)
_, err := DWithin(geo.MustParseGeometry("POINT(1.0 2.0)"), geo.MustParseGeometry("POINT(3.0 4.0)"), -0.01, geo.FnInclusive)
require.Error(t, err)
})
}
Expand Down Expand Up @@ -591,7 +591,7 @@ func TestDFullyWithin(t *testing.T) {
})

t.Run("errors if distance < 0", func(t *testing.T) {
_, err := DWithin(geo.MustParseGeometry("POINT(1.0 2.0)"), geo.MustParseGeometry("POINT(3.0 4.0)"), -0.01, false /* exclusive */)
_, err := DWithin(geo.MustParseGeometry("POINT(1.0 2.0)"), geo.MustParseGeometry("POINT(3.0 4.0)"), -0.01, geo.FnInclusive)
require.Error(t, err)
})
}
Expand Down
Loading

0 comments on commit 1d5fe90

Please sign in to comment.