geo/geomfn: implement ST_Relate and ST_ContainsProperly

[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.

[cockroach-teamcity]

This change is 

[maddyblue]

opt parts lgtm

[blathers-crl]

❌ The GitHub CI (Cockroach) build has failed on 6484ec48.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[sumeerbhola]

only minor comments -- looks good overall

Reviewed 13 of 13 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @otan and @sumeerbhola)

---

docs/RFCS/20200421_geospatial.md, line 1043 at r2 (raw file):

* 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): see above.

nit: please repeat the text from the previous bullet

---

pkg/geo/geomfn/binary_predicates.go, line 49 at r2 (raw file):

T**FF*FF*

There is some redundancy in DE-9IM -- when I tried to construct I came up with TF*FF*FF*, but I think it is equivalent to what you wrote above.

---

pkg/geo/geomfn/de9im.go, line 31 at r2 (raw file):

// MatchesDE9IM checks whether the given DE-9IM relation matches the DE-91M pattern.
// See: https://en.wikipedia.org/wiki/DE-9IM.
func MatchesDE9IM(str string, pattern string) (bool, error) {

I am assuming str cannot have * since it represents the actual computed relation.
If yes, can you add that to this function comment.

---

pkg/geo/geomfn/de9im.go, line 43 at r2 (raw file):

			continue
		case 't':
			if str[i] < '0' || str[i] > '2' {

when do we expect < 0?
Can you add a comment that f represents false and that 0, 1, 2 are true?
I think it would be easier to read if we added a conversion function to a bool.

---

pkg/geo/geos/geos.cc, line 430 at r2 (raw file):

//
// DE-9IM related

Please add the wikipedia link

[otan]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @sumeerbhola)

---

pkg/geo/geomfn/binary_predicates.go, line 49 at r2 (raw file):

Previously, sumeerbhola wrote…

T**FF*FF*

There is some redundancy in DE-9IM -- when I tried to construct I came up with TF*FF*FF*, but I think it is equivalent to what you wrote above.

I cheated and used what was on https://postgis.net/docs/ST_ContainsProperly.html.

---

pkg/geo/geomfn/de9im.go, line 31 at r2 (raw file):

Previously, sumeerbhola wrote…

I am assuming str cannot have * since it represents the actual computed relation.
If yes, can you add that to this function comment.

Done.

---

pkg/geo/geomfn/de9im.go, line 43 at r2 (raw file):

Previously, sumeerbhola wrote…

when do we expect < 0?
Can you add a comment that f represents false and that 0, 1, 2 are true?
I think it would be easier to read if we added a conversion function to a bool.

Done.

---

pkg/geo/geos/geos.cc, line 430 at r2 (raw file):

Previously, sumeerbhola wrote…

Please add the wikipedia link

Done.

[blathers-crl]

❌ The GitHub CI (Cockroach) build has failed on ff2a00be.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[sumeerbhola]

Reviewed 7 of 7 files at r3.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @otan and @sumeerbhola)

---

pkg/geo/geomfn/de9im.go, line 29 at r3 (raw file):

// MatchesDE9IM checks whether the given DE-9IM relation matches the DE-91M pattern.
// Assumes the relation has been computed, and such has no 'T' characters.

did you mean '*'?

---

pkg/geo/geomfn/de9im.go, line 54 at r3 (raw file):

// Pattern matches are as follows:
// * '*': allow anything.
// * 'T': allow only if the relation is '0' (point), '1' (line) or '2' (polygon).

'T' or 't': allow if the relation is true, which is represented by any of the characters '0' (point), '1' (line) or '2' (area) (representing the dimensionality of the intersection).

---

pkg/geo/geomfn/de9im.go, line 55 at r3 (raw file):

// * '*': allow anything.
// * 'T': allow only if the relation is '0' (point), '1' (line) or '2' (polygon).
// * 'f': allow only if relation is also false.

'F' or 'f': allow only if the relation is false, which is represented by the characters 'f' or 'F'.

---

pkg/geo/geomfn/de9im.go, line 62 at r3 (raw file):

	case 't':
		// True should only match characters 0-2. 'T' should never exist
		// in a relation.

I think one can drop this last sentence "'T' should never exist ..."

[otan]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @sumeerbhola)

---

pkg/geo/geomfn/de9im.go, line 29 at r3 (raw file):

Previously, sumeerbhola wrote…

did you mean '*'?

both, actually.

---

pkg/geo/geomfn/de9im.go, line 54 at r3 (raw file):

Previously, sumeerbhola wrote…

'T' or 't': allow if the relation is true, which is represented by any of the characters '0' (point), '1' (line) or '2' (area) (representing the dimensionality of the intersection).

Done.

---

pkg/geo/geomfn/de9im.go, line 55 at r3 (raw file):

Previously, sumeerbhola wrote…

'F' or 'f': allow only if the relation is false, which is represented by the characters 'f' or 'F'.

Done.

---

pkg/geo/geomfn/de9im.go, line 62 at r3 (raw file):

Previously, sumeerbhola wrote…

I think one can drop this last sentence "'T' should never exist ..."

Done.

[blathers-crl]

❌ The GitHub CI (Cockroach) build has failed on 060cf58b.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[sumeerbhola]

Reviewed 13 of 16 files at r1, 2 of 2 files at r4.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @otan and @sumeerbhola)

---

pkg/geo/geomfn/de9im.go, line 54 at r4 (raw file):

// Pattern matches are as follows:
// * '*': allow anything.
// * 't'/'T': allow only if the relation is tryem which is represented any of

s/tryem/true/

... is represented by any of ...

---

pkg/geo/geomfn/de9im.go, line 55 at r4 (raw file):

// * '*': allow anything.
// * 't'/'T': allow only if the relation is tryem which is represented any of
//   '0' (point), '1' (line) or '2' (polygon) which are the dimensionality of the

s/polygon/area

... which is the dimensionality ...

[otan]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @sumeerbhola)

---

pkg/geo/geomfn/de9im.go, line 54 at r4 (raw file):

Previously, sumeerbhola wrote…

s/tryem/true/

... is represented by any of ...

Done.

---

pkg/geo/geomfn/de9im.go, line 55 at r4 (raw file):

Previously, sumeerbhola wrote…

s/polygon/area

... which is the dimensionality ...

Done.

[sumeerbhola]

Reviewed 3 of 3 files at r5.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @sumeerbhola)

[otan]

bors r=sumeerbhola

[craig]

Build failed (retrying...)

• GitHub CI (Cockroach)