Merge #47097 #47446

47097: geo/geoindex: interface for geometry and geography indexing, and r=sumeerbhola a=sumeerbhola

implementation of those interfaces by s2GMIndex and s2GGIndex

Both implementations use the S2 library to map 2D space to a
single dimension.

The implementations have TODOs regarding heuristics that will
be addressed after we have end-to-end benchmarks. The geometry
index abuses S2 by using face 0 of the unit cube that will be
projected to the unit sphere, to index the rectangular bounding
box of the index. This introduces two distortions:
rectangle => square, and square side of unit cube => unit sphere.
Geometries that exceed the bounding box are clipped, and indexed
both as clipped, and under a special cellid that is unused by S2.

Index acceleration is restricted to three relationships:
Covers, CoveredBy, Intersection. Other relationships will need
to be expressed using these. CoveredBy uses an "inner covering"
to deal with the fact that coverings do not have the strong
properties of bounding boxes. The CoveredBy produces an expression
involving set unions and intersections that is factored to
eliminate common sub-expressions and output in Reverse Polish
notation. How to compute this expression during query execution is
an open topic.

Release note: None

47446: sql: fix handling of errors in schema change rollbacks r=lucy-zhang a=lucy-zhang

This PR does two main things to fix error handling for a schema change
being rolled back in the job's `OnFailOrCancel` hook: (1) errors from
`runStateMachineAndBackfill` (acting on the reversed schema change) are
now returned instead of being logged and swallowed; and (2) we now
return the error and cause the the job to fail permanently when we
encounter any error that isn't one of the specific whitelisted
"retriable" errors for the schema changer. Transient "retriable" errors
in the schema changer now correctly lead to retries (which are handled
by the job registry).

The reason for implementing (2) in addition to (1) was that I discovered
some pre-existing instances of erroneous behavior in 19.2 and earlier
versions where the "expected" behavior was that we would never be able
to clean up a failed schema change due to inadequacies in the schema
changer and backfiller. (One notable example is that because the
column backfiller can't recover previous values, if a DROP COLUMN schema
change is being rolled back, trying to backfill a default value on a
column with constraints won't work in general.) Given this, it seemed
best to just return a result to the client right away instead of
pointlessly retrying and blocking indefinitely. This seemed like the
best quick fix that isn't a regression from 19.2 and that somewhat
reproduces the existing behavior.

This change also required a few changes to testing: There's now a
sentinel for job retry errors used with the errors package, and a few
testing knobs in the schema change job migration tests that were relying
on the old erroneous behavior have now been corrected.

See #46541 (comment) for an example of a schema change that can't be successfully rolled back.
Closes #47324.

Release note (bug fix): Fixed a bug introduced with the new schema
change job implementation in beta.3 that caused errors when rolling back
a schema change to be swallowed.

Co-authored-by: sumeerbhola <[email protected]>
Co-authored-by: Lucy Zhang <[email protected]>

pkg/geo/geo.go

@@ -122,6 +122,11 @@ func (g *Geometry) AsGeography() (*Geography, error) {
 	return NewGeography(geopb.EWKB(ret)), nil
 }
 
+// AsGeomT returns the geometry as a geom.T object.
+func (g *Geometry) AsGeomT() (geom.T, error) {
+	return ewkb.Unmarshal(g.ewkb)
+}
+
 // Geography is a spherical spatial object.
 type Geography struct {
 	spatialObjectBase

pkg/geo/geo_test.go

@@ -14,7 +14,6 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/cockroachdb/cockroach/pkg/geo/geopb"
 	"github.com/cockroachdb/cockroach/pkg/geo/geos"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/datadriven"
@@ -187,7 +186,7 @@ func TestClipEWKBByRect(t *testing.T) {
 			d.ScanArgs(t, "xmax", &xMax)
 			d.ScanArgs(t, "ymax", &yMax)
 			ewkb, err := geos.ClipEWKBByRect(
-				geopb.WKB(g.ewkb), float64(xMin), float64(yMin), float64(xMax), float64(yMax))
+				g.ewkb, float64(xMin), float64(yMin), float64(xMax), float64(yMax))
 			if err != nil {
 				return err.Error()
 			}