Merge #61515 #61523 #61528

61515: sql: prevent tables with PARTITION BY clauses from becoming multi-region r=ajstorm a=otan

Release justification: low-risk update to new functionality
Release note: None

Refs #59719

61523: geo/geomfn: implement ST_Snap r=otan a=alsohas

This patch implements ST_Snap.

Resolves #49040

Release justification: low risk, high benefit changes to existing functionality
Release note (sql change): ST_Snap is now available.

61528: roachtest: bump activerecord adapter tests to 6.1.0beta2 r=rafiss a=otan

Release justification: non-production code change

Release note: None

Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
Co-authored-by: Abdullah Islam <abdullah_islam@uri.edu>

docs/generated/sql/functions.md

@@ -2217,6 +2217,10 @@ The paths themselves are given in the direction of the first geometry.</p>
 </span></td></tr>
 <tr><td><a name="st_simplifypreservetopology"></a><code>st_simplifypreservetopology(geometry: geometry, tolerance: <a href="float.html">float</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Simplifies the given geometry using the Douglas-Peucker algorithm, avoiding the creation of invalid geometries.</p>
 </span></td></tr>
+<tr><td><a name="st_snap"></a><code>st_snap(input: geometry, target: geometry, tolerance: <a href="float.html">float</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Snaps the vertices and segments of input geometry the target geometry’s vertices.
+Tolerance is used to control where snapping is performed. The result geometry is the input geometry with the vertices snapped.
+If no snapping occurs then the input geometry is returned unchanged.</p>
+</span></td></tr>
 <tr><td><a name="st_snaptogrid"></a><code>st_snaptogrid(geometry: geometry, origin_x: <a href="float.html">float</a>, origin_y: <a href="float.html">float</a>, size_x: <a href="float.html">float</a>, size_y: <a href="float.html">float</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Snap a geometry to a grid of with X coordinates snapped to size_x and Y coordinates snapped to size_y based on an origin of (origin_x, origin_y).</p>
 </span></td></tr>
 <tr><td><a name="st_snaptogrid"></a><code>st_snaptogrid(geometry: geometry, size: <a href="float.html">float</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Snap a geometry to a grid of the given size.</p>

pkg/ccl/logictestccl/testdata/logic_test/multi_region

@@ -844,9 +844,34 @@ TABLE t_global  ALTER TABLE t_global CONFIGURE ZONE USING
                 lease_preferences = '[[+region=ca-central-1]]'
 
 statement ok
-CREATE DATABASE no_initial_region
+CREATE DATABASE no_initial_region;
+USE no_initial_region
 
 statement ok
+CREATE TABLE table_with_partition_by (
+  id INT PRIMARY KEY
+) PARTITION BY LIST (id) (
+  PARTITION "one" VALUES IN (1)
+)
+
+statement error cannot convert database no_initial_region to a multi-region database\nDETAIL: cannot convert table table_with_partition_by to a multi-region table as it is partitioned
+ALTER DATABASE no_initial_region SET PRIMARY REGION "us-east-1"
+
+statement ok
+DROP TABLE table_with_partition_by;
+CREATE TABLE idx_with_partition_by (
+  id INT PRIMARY KEY,
+  a INT,
+  INDEX(a) PARTITION BY LIST (a) (
+    PARTITION "one" VALUES IN (1)
+  )
+)
+
+statement error cannot convert database no_initial_region to a multi-region database\nDETAIL: cannot convert table idx_with_partition_by to a multi-region table as it has index/constraint idx_with_partition_by_a_idx with partitioning
+ALTER DATABASE no_initial_region SET PRIMARY REGION "us-east-1"
+
+statement ok
+DROP TABLE idx_with_partition_by;
 CREATE TABLE no_initial_region.t(k INT)
 
 statement ok

pkg/cmd/roachtest/activerecord.go

@@ -20,8 +20,8 @@ import (
 
 var activerecordResultRegex = regexp.MustCompile(`^(?P<test>[^\s]+#[^\s]+) = (?P<timing>\d+\.\d+ s) = (?P<result>.)$`)
 var railsReleaseTagRegex = regexp.MustCompile(`^v(?P<major>\d+)\.(?P<minor>\d+)\.(?P<point>\d+)\.?(?P<subpoint>\d*)$`)
-var supportedRailsVersion = "6.0.3.4"
-var adapterVersion = "v6.0.0beta3"
+var supportedRailsVersion = "6.1"
+var adapterVersion = "v6.1.0beta2"
 
 // This test runs activerecord's full test suite against a single cockroach node.
 

pkg/geo/geomfn/BUILD.bazel

@@ -26,6 +26,7 @@ go_library(
         "reverse.go",
         "segmentize.go",
         "shift_longitude.go",
+        "snap.go",
         "snap_to_grid.go",
         "subdivide.go",
         "swap_ordinates.go",
@@ -80,6 +81,7 @@ go_test(
         "reverse_test.go",
         "segmentize_test.go",
         "shift_longitude_test.go",
+        "snap_test.go",
         "snap_to_grid_test.go",
         "subdivide_test.go",
         "swap_ordinates_test.go",

pkg/geo/geomfn/snap.go

@@ -0,0 +1,27 @@
+// Copyright 2021 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"
+)
+
+// Snap returns the input geometry with the vertices snapped to the target
+// geometry. Tolerance is used to control where snapping is performed.
+// If no snapping occurs then the input geometry is returned unchanged.
+func Snap(input, target geo.Geometry, tolerance float64) (geo.Geometry, error) {
+	snappedEWKB, err := geos.Snap(input.EWKB(), target.EWKB(), tolerance)
+	if err != nil {
+		return geo.Geometry{}, err
+	}
+	return geo.ParseGeometryFromEWKB(snappedEWKB)
+}

pkg/geo/geomfn/snap_test.go

@@ -0,0 +1,87 @@
+// Copyright 2021 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 (
+	"testing"
+
+	"github.com/cockroachdb/cockroach/pkg/geo"
+	"github.com/stretchr/testify/require"
+	"github.com/twpayne/go-geom"
+)
+
+func TestSnap(t *testing.T) {
+	testCases := []struct {
+		desc      string
+		input     geom.T
+		target    geom.T
+		tolerance float64
+		expected  geom.T
+	}{
+		{
+			desc:      "Test snap on two linestrings with tolerance of 0",
+			input:     geom.NewLineStringFlat(geom.XY, []float64{20, 25, 30, 35}),
+			target:    geom.NewLineStringFlat(geom.XY, []float64{10, 15, 20, 25}),
+			tolerance: 0,
+			expected:  geom.NewLineStringFlat(geom.XY, []float64{20, 25, 30, 35}),
+		},
+		{
+			desc:      "Test snapping a polygon on a linestring with tolerance of 150",
+			input:     geom.NewLineStringFlat(geom.XY, []float64{10, 55, 78, 84, 100, 200}),
+			target:    geom.NewPolygonFlat(geom.XY, []float64{26, 125, 26, 200, 126, 200, 126, 125, 26, 125}, []int{10}),
+			tolerance: 150,
+			expected:  geom.NewLineStringFlat(geom.XY, []float64{10, 55, 26, 125, 26, 200, 126, 200, 126, 125}),
+		},
+		{
+			desc:      "Test snapping a linestring on a polygon with tolerance of 150",
+			target:    geom.NewLineStringFlat(geom.XY, []float64{10, 55, 78, 84, 100, 200}),
+			input:     geom.NewPolygonFlat(geom.XY, []float64{26, 125, 26, 200, 126, 200, 126, 125, 26, 125}, []int{10}),
+			tolerance: 150,
+			expected:  geom.NewPolygonFlat(geom.XY, []float64{10, 55, 26, 200, 100, 200, 78, 84, 10, 55}, []int{10}),
+		},
+		{
+			desc:      "Test snapping a linestring on a multipolygon with tolerance of 200",
+			input:     geom.NewLineStringFlat(geom.XY, []float64{5, 107, 54, 84, 101, 100}),
+			target:    geom.NewMultiPolygonFlat(geom.XY, []float64{1, 1, 2, 2, 3, 3, 1, 1, 4, 4, 5, 5, 6, 6, 4, 4}, [][]int{{8}, {16}}),
+			tolerance: 200,
+			expected:  geom.NewLineStringFlat(geom.XY, []float64{5, 107, 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6, 101, 100}),
+		},
+		{
+			desc:      "Test snapping a multipolygon on a linestring with tolerance of 200",
+			input:     geom.NewMultiPolygonFlat(geom.XY, []float64{1, 1, 2, 2, 3, 3, 1, 1, 4, 4, 5, 5, 6, 6, 4, 4}, [][]int{{8}, {16}}),
+			target:    geom.NewLineStringFlat(geom.XY, []float64{5, 107, 54, 84, 101, 100}),
+			tolerance: 200,
+			expected:  geom.NewMultiPolygonFlat(geom.XY, []float64{1, 1, 2, 2, 5, 107, 54, 84, 101, 100, 1, 1, 4, 4, 5, 5, 5, 107, 54, 84, 101, 100, 4, 4}, [][]int{{8}, {16}}),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			input, err := geo.MakeGeometryFromGeomT(tc.input)
+			require.NoError(t, err)
+			target, err := geo.MakeGeometryFromGeomT(tc.target)
+			require.NoError(t, err)
+
+			actual, err := Snap(input, target, tc.tolerance)
+			require.NoError(t, err)
+
+			// Compare FlatCoords and assert they are within epsilon.
+			// This is because they exact matches may encounter rounding issues.
+			actualGeomT, err := actual.AsGeomT()
+
+			require.NoError(t, err)
+			require.Equal(t, tc.expected.SRID(), actualGeomT.SRID())
+			require.Equal(t, tc.expected.Layout(), actualGeomT.Layout())
+			require.IsType(t, tc.expected, actualGeomT)
+			require.InEpsilonSlice(t, tc.expected.FlatCoords(), actualGeomT.FlatCoords(), 0.00001)
+		})
+	}
+}

pkg/geo/geos/geos.cc

@@ -174,6 +174,8 @@ typedef char (*CR_GEOS_EqualsExact_r)(CR_GEOS_Handle, CR_GEOS_Geometry,
 
 typedef CR_GEOS_Geometry (*CR_GEOS_MinimumRotatedRectangle_r)(CR_GEOS_Handle, CR_GEOS_Geometry);
 
+typedef CR_GEOS_Geometry (*CR_GEOS_Snap_r)(CR_GEOS_Handle, CR_GEOS_Geometry, CR_GEOS_Geometry, double);
+
 std::string ToString(CR_GEOS_Slice slice) { return std::string(slice.data, slice.len); }
 
 }  // namespace
@@ -283,6 +285,8 @@ struct CR_GEOS {
 
   CR_GEOS_Node_r GEOSNode_r;
 
+  CR_GEOS_Snap_r GEOSSnap_r;
+
   CR_GEOS(dlhandle geoscHandle, dlhandle geosHandle)
       : geoscHandle(geoscHandle), geosHandle(geosHandle) {}
 
@@ -385,6 +389,7 @@ struct CR_GEOS {
     INIT(GEOSWKBWriter_write_r);
     INIT(GEOSClipByRect_r);
     INIT(GEOSNode_r);
+    INIT(GEOSSnap_r);
     return nullptr;
 
 #undef INIT
@@ -1523,3 +1528,27 @@ CR_GEOS_Status CR_GEOS_MinimumRotatedRectangle(CR_GEOS* lib, CR_GEOS_Slice g, CR
   lib->GEOS_finish_r(handle);
   return toGEOSString(error.data(), error.length());
 }
+
+CR_GEOS_Status CR_GEOS_Snap(CR_GEOS* lib, CR_GEOS_Slice input, CR_GEOS_Slice target, double tolerance, CR_GEOS_String* snappedEWKB) {
+  std::string error;
+  auto handle = initHandleWithErrorBuffer(lib, &error);
+  auto gGeomInput = CR_GEOS_GeometryFromSlice(lib, handle, input);
+  auto gGeomTarget = CR_GEOS_GeometryFromSlice(lib, handle, target);
+  *snappedEWKB = {.data = NULL, .len = 0};
+  if (gGeomInput != nullptr && gGeomTarget != nullptr) {
+    auto r = lib->GEOSSnap_r(handle, gGeomInput, gGeomTarget, tolerance);
+    if (r != NULL) {
+      auto srid = lib->GEOSGetSRID_r(handle, r);
+      CR_GEOS_writeGeomToEWKB(lib, handle, r, snappedEWKB, srid);
+      lib->GEOSGeom_destroy_r(handle, r);
+    }
+  }
+  if (gGeomInput != nullptr) {
+    lib->GEOSGeom_destroy_r(handle, gGeomInput);
+  }
+  if (gGeomTarget != nullptr) {
+    lib->GEOSGeom_destroy_r(handle, gGeomTarget);
+  }
+  lib->GEOS_finish_r(handle);
+  return toGEOSString(error.data(), error.length());
+}

pkg/geo/geos/geos.go

@@ -1073,3 +1073,20 @@ func MinimumRotatedRectangle(ewkb geopb.EWKB) (geopb.EWKB, error) {
 	}
 	return cStringToSafeGoBytes(cEWKB), nil
 }
+
+// Snap returns the input EWKB with the vertices snapped to the target
+// EWKB. Tolerance is used to control where snapping is performed.
+// If no snapping occurs then the input geometry is returned unchanged.
+func Snap(input, target geopb.EWKB, tolerance float64) (geopb.EWKB, error) {
+	g, err := ensureInitInternal()
+	if err != nil {
+		return nil, err
+	}
+	var cEWKB C.CR_GEOS_String
+	if err := statusToError(
+		C.CR_GEOS_Snap(g, goToCSlice(input), goToCSlice(target), C.double(tolerance), &cEWKB),
+	); err != nil {
+		return nil, err
+	}
+	return cStringToSafeGoBytes(cEWKB), nil
+}

pkg/geo/geos/geos.h

@@ -192,6 +192,8 @@ CR_GEOS_Status CR_GEOS_RelatePattern(CR_GEOS* lib, CR_GEOS_Slice a, CR_GEOS_Slic
 CR_GEOS_Status CR_GEOS_VoronoiDiagram(CR_GEOS* lib, CR_GEOS_Slice g, CR_GEOS_Slice env,
                                       double tolerance, int onlyEdges, CR_GEOS_String* ret);
 
+CR_GEOS_Status CR_GEOS_Snap(CR_GEOS* lib, CR_GEOS_Slice input, CR_GEOS_Slice target, double tolerance, CR_GEOS_String* ret);
+
 #ifdef __cplusplus
 }  // extern "C"
 #endif

pkg/sql/alter_database.go

@@ -546,16 +546,16 @@ func (n *alterDatabasePrimaryRegionNode) switchPrimaryRegion(params runParams) e
 // that the table is a REGIONAL BY TABLE table homed in the primary region of
 // the database.
 func addDefaultLocalityConfigToAllTables(
-	ctx context.Context, p *planner, desc *dbdesc.Immutable, regionEnumID descpb.ID,
+	ctx context.Context, p *planner, dbDesc *dbdesc.Immutable, regionEnumID descpb.ID,
 ) error {
-	if !desc.IsMultiRegion() {
+	if !dbDesc.IsMultiRegion() {
 		return errors.AssertionFailedf(
 			"cannot add locality config to tables in non multi-region database with ID %d",
-			desc.GetID(),
+			dbDesc.GetID(),
 		)
 	}
 	b := p.Txn().NewBatch()
-	if err := forEachTableDesc(ctx, p, desc, hideVirtual,
+	if err := forEachTableDesc(ctx, p, dbDesc, hideVirtual,
 		func(immutable *dbdesc.Immutable, _ string, desc catalog.TableDescriptor) error {
 			mutDesc, err := p.Descriptors().GetMutableTableByID(
 				ctx, p.txn, desc.GetID(), tree.ObjectLookupFlags{},
@@ -564,6 +564,10 @@ func addDefaultLocalityConfigToAllTables(
 				return err
 			}
 
+			if err := checkCanConvertTableToMultiRegion(dbDesc, mutDesc); err != nil {
+				return err
+			}
+
 			if err := p.alterTableDescLocalityToRegionalByTable(
 				ctx, tree.PrimaryRegionNotSpecifiedName, mutDesc, regionEnumID,
 			); err != nil {
@@ -580,6 +584,40 @@ func addDefaultLocalityConfigToAllTables(
 	return p.Txn().Run(ctx, b)
 }
 
+// checkCanConvertTableToMultiRegion checks whether a given table can be converted
+// to a multi-region table.
+func checkCanConvertTableToMultiRegion(
+	dbDesc catalog.DatabaseDescriptor, tableDesc catalog.TableDescriptor,
+) error {
+	if tableDesc.GetPrimaryIndex().GetPartitioning().NumColumns > 0 {
+		return errors.WithDetailf(
+			pgerror.Newf(
+				pgcode.ObjectNotInPrerequisiteState,
+				"cannot convert database %s to a multi-region database",
+				dbDesc.GetName(),
+			),
+			"cannot convert table %s to a multi-region table as it is partitioned",
+			tableDesc.GetName(),
+		)
+	}
+	for _, idx := range tableDesc.NonDropIndexes() {
+		if idx.GetPartitioning().NumColumns > 0 {
+			return errors.WithDetailf(
+				pgerror.Newf(
+					pgcode.ObjectNotInPrerequisiteState,
+					"cannot convert database %s to a multi-region database",
+					dbDesc.GetName(),
+				),
+				"cannot convert table %s to a multi-region table as it has index/constraint %s with partitioning",
+				tableDesc.GetName(),
+				idx.GetName(),
+			)
+		}
+	}
+	// TODO(#57668): check zone configurations are not set here
+	return nil
+}
+
 // setInitialPrimaryRegion sets the primary region in cases where the database
 // is already a multi-region database.
 func (n *alterDatabasePrimaryRegionNode) setInitialPrimaryRegion(params runParams) error {

pkg/sql/logictest/testdata/logic_test/geospatial

@@ -5768,3 +5768,17 @@ SRID=4326;POINT (359 90)
 SRID=4326;POINT (-179 -23)
 SRID=4326;LINESTRING (2 2, 5 -60, -160 0)
 SRID=4326;POLYGON ((354 -10, 6 -10, 0 20, 354 -10), (3 2, 359 -1, 1 -5, 3 2))
+
+query T
+SELECT ST_AsText(
+    ST_Snap(poly,line, ST_Distance(poly, line)*1.25)
+) AS polysnapped
+FROM (SELECT
+    ST_GeomFromText('MULTIPOLYGON(
+      (( 26 125, 26 200, 126 200, 126 125, 26 125 ),
+      ( 51 150, 101 150, 76 175, 51 150 )),
+      (( 151 100, 151 200, 176 175, 151 100 )))') As poly,
+    ST_GeomFromText('LINESTRING (5 107, 54 84, 101 100)') As line
+	) tbl(poly, line);
+----
+MULTIPOLYGON (((5 107, 26 200, 126 200, 126 125, 101 100, 54 84, 5 107), (51 150, 101 150, 76 175, 51 150)), ((151 100, 151 200, 176 175, 151 100)))