Merge #53556 #53600 #53604 #53643 #53656

53556: builtins: add in unimplemented spatial builtins r=sumeerbhola a=otan

This adds telemetry to remaining unused functions, as well as displaying
the issue number when someone attempts to use an unimplemented function.

Also changed the wording of the error message to be a little more
friendly.

Release justification: low risk, high benefit changes

Release note: None

53600: build: fix TESTS env variable in roachtest nightly script r=jlinder a=adityamaru

AWS TC nightly fails with `unbound variable TESTS`. This
change initializes it outside the if condition.

Release justification: non-production code changes

53604: telemetry: fix the integer quantization r=irfansharif a=knz

The code was previously quantizing 100 to 10, and only
101 and higher to 100. This patch fixes it.

Additionally, this code ensures that negative values also get
quantized. This change does not impact any caller for now (there is no
caller that expects negative value) but will be used in an upcoming
setting telemetry change.

Release justification: low risk, high benefit changes to existing functionality

Release note: None

53643: builtins: implement ST_ForceCollection r=otan a=erikgrinaker

Some test cases deviate from PostGIS due to a bug in the WKT encoder (see #53632). The behavior of `ST_ForceCollection` is correct, but the WKT output does not accurately represent the resulting geometry. Not sure if we should wait for the upstream fix or merge this as-is. 

Release justification: low risk, high benefit changes to existing functionality

Release note (sql change): Implement the geometry builtin
`ST_ForceCollection`.

Closes #48934.

53656: roachtest: ignore version upgrade no inbound stream connection error r=irfansharif a=asubiotto

Release justification: non-production code change

This error is fixed in 20.2, but may occur on earlier versions because version
upgrade is a mixed-version test.

Release note: None (testing change)

Closes #53393 

Co-authored-by: Oliver Tan <[email protected]>
Co-authored-by: Aditya Maru <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Erik Grinaker <[email protected]>
Co-authored-by: Alfonso Subiotto Marques <[email protected]>

build/teamcity-nightly-roachtest.sh

@@ -75,6 +75,7 @@ ARTIFACTS="${artifacts}"
 PARALLELISM=16
 CPUQUOTA=1024
 ZONES=""
+TESTS=""
 case "${CLOUD}" in
   gce)
     # We specify --zones below so that nodes are created in us-central1-b by
@@ -86,8 +87,7 @@ case "${CLOUD}" in
     PARALLELISM=3
     CPUQUOTA=384
     if [ -z "${TESTS}" ]; then
-      TESTS="kv(0|95)|ycsb|tpcc/(headroom/n4cpu16)|tpccbench/(nodes=3/cpu=16)|scbench/randomload/
-      (nodes=3/ops=2000/conc=1)|backup/(KMS/n3cpu4)"
+      TESTS="kv(0|95)|ycsb|tpcc/(headroom/n4cpu16)|tpccbench/(nodes=3/cpu=16)|scbench/randomload/(nodes=3/ops=2000/conc=1)|backup/(KMS/n3cpu4)"
     fi
     ;;
   *)

docs/generated/sql/functions.md

@@ -1235,6 +1235,8 @@ Bottom Left.</p>
 </span></td></tr>
 <tr><td><a name="st_force2d"></a><code>st_force2d(geometry: geometry) &rarr; geometry</code></td><td><span class="funcdesc"><p>Returns a Geometry which only contains X and Y coordinates.</p>
 </span></td></tr>
+<tr><td><a name="st_forcecollection"></a><code>st_forcecollection(geometry: geometry) &rarr; geometry</code></td><td><span class="funcdesc"><p>Converts the geometry into a GeometryCollection.</p>
+</span></td></tr>
 <tr><td><a name="st_forcepolygonccw"></a><code>st_forcepolygonccw(geometry: geometry) &rarr; geometry</code></td><td><span class="funcdesc"><p>Returns a Geometry where all Polygon objects have exterior rings in the counter-clockwise orientation and interior rings in the clockwise orientation. Non-Polygon objects are unchanged.</p>
 </span></td></tr>
 <tr><td><a name="st_forcepolygoncw"></a><code>st_forcepolygoncw(geometry: geometry) &rarr; geometry</code></td><td><span class="funcdesc"><p>Returns a Geometry where all Polygon objects have exterior rings in the clockwise orientation and interior rings in the counter-clockwise orientation. Non-Polygon objects are unchanged.</p>

pkg/cmd/roachtest/versionupgrade.go

@@ -20,6 +20,7 @@ import (
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/util/binfetcher"
 	"github.com/cockroachdb/cockroach/pkg/util/retry"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
@@ -28,7 +29,10 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-var v201 = roachpb.Version{Major: 20, Minor: 1}
+var (
+	v201 = roachpb.Version{Major: 20, Minor: 1}
+	v202 = roachpb.Version{Major: 20, Minor: 2}
+)
 
 // Feature tests that are invoked between each step of the version upgrade test.
 // Tests can use u.clusterVersion to determine which version is active at the
@@ -430,6 +434,11 @@ func stmtFeatureTest(
 			}
 			db := u.conn(ctx, t, i)
 			if _, err := db.ExecContext(ctx, stmt, args...); err != nil {
+				if testutils.IsError(err, "no inbound stream connection") && u.clusterVersion(ctx, t, i).Less(v202) {
+					// This error has been fixed in 20.2+ but may still occur on earlier
+					// versions.
+					return true // skipped
+				}
 				t.Fatal(err)
 			}
 			return false

pkg/geo/geomfn/collections.go

@@ -339,6 +339,53 @@ func collectionHomogenizeGeomT(t geom.T) (geom.T, error) {
 	}
 }
 
+// ForceCollection converts the input into a GeometryCollection.
+func ForceCollection(g geo.Geometry) (geo.Geometry, error) {
+	t, err := g.AsGeomT()
+	if err != nil {
+		return geo.Geometry{}, err
+	}
+	t, err = forceCollectionFromGeomT(t)
+	if err != nil {
+		return geo.Geometry{}, err
+	}
+	return geo.MakeGeometryFromGeomT(t)
+}
+
+// forceCollectionFromGeomT converts a geom.T into a geom.GeometryCollection.
+func forceCollectionFromGeomT(t geom.T) (geom.T, error) {
+	gc := geom.NewGeometryCollection().SetSRID(t.SRID())
+	switch t := t.(type) {
+	case *geom.Point, *geom.LineString, *geom.Polygon:
+		if err := gc.Push(t); err != nil {
+			return nil, err
+		}
+	case *geom.MultiPoint:
+		for i := 0; i < t.NumPoints(); i++ {
+			if err := gc.Push(t.Point(i)); err != nil {
+				return nil, err
+			}
+		}
+	case *geom.MultiLineString:
+		for i := 0; i < t.NumLineStrings(); i++ {
+			if err := gc.Push(t.LineString(i)); err != nil {
+				return nil, err
+			}
+		}
+	case *geom.MultiPolygon:
+		for i := 0; i < t.NumPolygons(); i++ {
+			if err := gc.Push(t.Polygon(i)); err != nil {
+				return nil, err
+			}
+		}
+	case *geom.GeometryCollection:
+		gc = t
+	default:
+		return nil, errors.AssertionFailedf("unknown geometry type: %T", t)
+	}
+	return gc, nil
+}
+
 // Multi converts the given geometry into a new multi-geometry.
 func Multi(g geo.Geometry) (geo.Geometry, error) {
 	t, err := g.AsGeomT() // implicitly clones the input

pkg/geo/geomfn/collections_test.go

@@ -254,6 +254,57 @@ func TestCollectionHomogenize(t *testing.T) {
 	}
 }
 
+func TestForceCollection(t *testing.T) {
+	testCases := []struct {
+		wkt      string
+		expected string
+	}{
+		{"POINT EMPTY", "GEOMETRYCOLLECTION (POINT EMPTY)"},
+		{"POINT (1 2)", "GEOMETRYCOLLECTION (POINT (1 2))"},
+		{"MULTIPOINT EMPTY", "GEOMETRYCOLLECTION EMPTY"},
+		{"MULTIPOINT (1 2)", "GEOMETRYCOLLECTION (POINT (1 2))"},
+		{"MULTIPOINT (1 2, 3 4)", "GEOMETRYCOLLECTION (POINT (1 2), POINT (3 4))"},
+		{"LINESTRING EMPTY", "GEOMETRYCOLLECTION (LINESTRING EMPTY)"},
+		{"LINESTRING (1 2, 3 4)", "GEOMETRYCOLLECTION (LINESTRING (1 2, 3 4))"},
+		{"MULTILINESTRING EMPTY", "GEOMETRYCOLLECTION EMPTY"},
+		{"MULTILINESTRING ((1 2, 3 4))", "GEOMETRYCOLLECTION (LINESTRING (1 2, 3 4))"},
+		{
+			"MULTILINESTRING ((1 2, 3 4), (5 6, 7 8))",
+			"GEOMETRYCOLLECTION (LINESTRING (1 2, 3 4), LINESTRING (5 6, 7 8))",
+		},
+		{"POLYGON EMPTY", "GEOMETRYCOLLECTION (POLYGON EMPTY)"},
+		{"POLYGON ((1 2, 3 4, 5 6, 1 2))", "GEOMETRYCOLLECTION (POLYGON ((1 2, 3 4, 5 6, 1 2)))"},
+		{"MULTIPOLYGON EMPTY", "GEOMETRYCOLLECTION EMPTY"},
+		{"MULTIPOLYGON (((1 2, 3 4, 5 6, 1 2)))", "GEOMETRYCOLLECTION (POLYGON ((1 2, 3 4, 5 6, 1 2)))"},
+		{
+			"MULTIPOLYGON (((1 2, 3 4, 5 6, 1 2)), ((7 8, 9 0, 1 2, 7 8)))",
+			"GEOMETRYCOLLECTION (POLYGON ((1 2, 3 4, 5 6, 1 2)), POLYGON ((7 8, 9 0, 1 2, 7 8)))",
+		},
+		{"GEOMETRYCOLLECTION EMPTY", "GEOMETRYCOLLECTION EMPTY"},
+		{"GEOMETRYCOLLECTION (MULTIPOINT (1 1, 2 2))", "GEOMETRYCOLLECTION (MULTIPOINT (1 1, 2 2))"},
+		{"GEOMETRYCOLLECTION (GEOMETRYCOLLECTION EMPTY)", "GEOMETRYCOLLECTION (GEOMETRYCOLLECTION EMPTY)"},
+		{
+			"GEOMETRYCOLLECTION (GEOMETRYCOLLECTION(MULTIPOINT (1 1)))",
+			"GEOMETRYCOLLECTION (GEOMETRYCOLLECTION (MULTIPOINT (1 1)))",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.wkt, func(t *testing.T) {
+			srid := geopb.SRID(4000)
+			g, err := geo.ParseGeometryFromEWKT(geopb.EWKT(tc.wkt), srid, true)
+			require.NoError(t, err)
+
+			result, err := ForceCollection(g)
+			require.NoError(t, err)
+			wkt, err := geo.SpatialObjectToWKT(result.SpatialObject(), 0)
+			require.NoError(t, err)
+			require.EqualValues(t, tc.expected, wkt)
+			require.EqualValues(t, srid, result.SRID())
+		})
+	}
+}
+
 func TestMulti(t *testing.T) {
 	testCases := []struct {
 		wkt      string

pkg/server/telemetry/features.go

@@ -12,6 +12,7 @@ package telemetry
 
 import (
 	"fmt"
+	"math"
 	"sync/atomic"
 
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
@@ -27,16 +28,22 @@ import (
 // raw numbers.
 // The numbers 0-10 are reported unchanged.
 func Bucket10(num int64) int64 {
-	if num <= 0 {
-		return 0
+	if num == math.MinInt64 {
+		// This is needed to prevent overflow in the negation below.
+		return -1000000000000000000
+	}
+	sign := int64(1)
+	if num < 0 {
+		sign = -1
+		num = -num
 	}
 	if num < 10 {
-		return num
+		return num * sign
 	}
 	res := int64(10)
-	for ; res < 1000000000000000000 && res*10 < num; res *= 10 {
+	for ; res < 1000000000000000000 && res*10 <= num; res *= 10 {
 	}
-	return res
+	return res * sign
 }
 
 // CountBucketed counts the feature identified by prefix and the value, using

pkg/server/telemetry/features_test.go

@@ -11,6 +11,7 @@
 package telemetry_test
 
 import (
+	"math"
 	"sync"
 	"testing"
 
@@ -40,3 +41,53 @@ func TestGetCounterDoesNotRace(t *testing.T) {
 	}
 	require.Len(t, counterSet, 1)
 }
+
+// TestBucket checks integer quantization.
+func TestBucket(t *testing.T) {
+	testData := []struct {
+		input    int64
+		expected int64
+	}{
+		{0, 0},
+		{1, 1},
+		{2, 2},
+		{3, 3},
+		{4, 4},
+		{5, 5},
+		{6, 6},
+		{7, 7},
+		{8, 8},
+		{9, 9},
+		{10, 10},
+		{11, 10},
+		{20, 10},
+		{99, 10},
+		{100, 100},
+		{101, 100},
+		{200, 100},
+		{999, 100},
+		{1000, 1000},
+		{math.MaxInt64, 1000000000000000000},
+		{-1, -1},
+		{-2, -2},
+		{-3, -3},
+		{-4, -4},
+		{-5, -5},
+		{-6, -6},
+		{-7, -7},
+		{-8, -8},
+		{-9, -9},
+		{-10, -10},
+		{-11, -10},
+		{-20, -10},
+		{-100, -100},
+		{-200, -100},
+		{math.MinInt64, -1000000000000000000},
+	}
+
+	for _, tc := range testData {
+		if actual, expected := telemetry.Bucket10(tc.input), tc.expected; actual != expected {
+			t.Errorf("%d: expected %d, got %d", tc.input, expected, actual)
+		}
+	}
+}

pkg/sql/logictest/testdata/logic_test/geospatial

@@ -1257,26 +1257,27 @@ Square (right)                            POINT (0.5 0.5)
 Square overlapping left and right square  POINT (0.45 0.5)
 
 # Collections
-query TTT
+query TTTT
 SELECT
   dsc,
-  ST_AsEWKT(ST_Multi(geom)),
-  ST_AsEWKT(ST_CollectionHomogenize(geom))
+  ST_AsText(ST_Multi(geom)),
+  ST_AsText(ST_CollectionHomogenize(geom)),
+  ST_AsText(ST_ForceCollection(geom))
 FROM geom_operators_test
 ORDER BY dsc ASC
 ----
-Empty GeometryCollection                  GEOMETRYCOLLECTION EMPTY                               GEOMETRYCOLLECTION EMPTY
-Empty LineString                          MULTILINESTRING EMPTY                                  LINESTRING EMPTY
-Empty Point                               MULTIPOINT EMPTY                                       POINT EMPTY
-Faraway point                             MULTIPOINT (5 5)                                       POINT (5 5)
-Line going through left and right square  MULTILINESTRING ((-0.5 0.5, 0.5 0.5))                  LINESTRING (-0.5 0.5, 0.5 0.5)
-NULL                                      NULL                                                   NULL
-Nested Geometry Collection                GEOMETRYCOLLECTION (GEOMETRYCOLLECTION (POINT (0 0)))  POINT (0 0)
-Point middle of Left Square               MULTIPOINT (-0.5 0.5)                                  POINT (-0.5 0.5)
-Point middle of Right Square              MULTIPOINT (0.5 0.5)                                   POINT (0.5 0.5)
-Square (left)                             MULTIPOLYGON (((-1 0, 0 0, 0 1, -1 1, -1 0)))          POLYGON ((-1 0, 0 0, 0 1, -1 1, -1 0))
-Square (right)                            MULTIPOLYGON (((0 0, 1 0, 1 1, 0 1, 0 0)))             POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))
-Square overlapping left and right square  MULTIPOLYGON (((-0.1 0, 1 0, 1 1, -0.1 1, -0.1 0)))    POLYGON ((-0.1 0, 1 0, 1 1, -0.1 1, -0.1 0))
+Empty GeometryCollection                  GEOMETRYCOLLECTION EMPTY                               GEOMETRYCOLLECTION EMPTY                      GEOMETRYCOLLECTION EMPTY
+Empty LineString                          MULTILINESTRING EMPTY                                  LINESTRING EMPTY                              GEOMETRYCOLLECTION (LINESTRING EMPTY)
+Empty Point                               MULTIPOINT EMPTY                                       POINT EMPTY                                   GEOMETRYCOLLECTION (POINT EMPTY)
+Faraway point                             MULTIPOINT (5 5)                                       POINT (5 5)                                   GEOMETRYCOLLECTION (POINT (5 5))
+Line going through left and right square  MULTILINESTRING ((-0.5 0.5, 0.5 0.5))                  LINESTRING (-0.5 0.5, 0.5 0.5)                GEOMETRYCOLLECTION (LINESTRING (-0.5 0.5, 0.5 0.5))
+NULL                                      NULL                                                   NULL                                          NULL
+Nested Geometry Collection                GEOMETRYCOLLECTION (GEOMETRYCOLLECTION (POINT (0 0)))  POINT (0 0)                                   GEOMETRYCOLLECTION (GEOMETRYCOLLECTION (POINT (0 0)))
+Point middle of Left Square               MULTIPOINT (-0.5 0.5)                                  POINT (-0.5 0.5)                              GEOMETRYCOLLECTION (POINT (-0.5 0.5))
+Point middle of Right Square              MULTIPOINT (0.5 0.5)                                   POINT (0.5 0.5)                               GEOMETRYCOLLECTION (POINT (0.5 0.5))
+Square (left)                             MULTIPOLYGON (((-1 0, 0 0, 0 1, -1 1, -1 0)))          POLYGON ((-1 0, 0 0, 0 1, -1 1, -1 0))        GEOMETRYCOLLECTION (POLYGON ((-1 0, 0 0, 0 1, -1 1, -1 0)))
+Square (right)                            MULTIPOLYGON (((0 0, 1 0, 1 1, 0 1, 0 0)))             POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))           GEOMETRYCOLLECTION (POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0)))
+Square overlapping left and right square  MULTIPOLYGON (((-0.1 0, 1 0, 1 1, -0.1 1, -0.1 0)))    POLYGON ((-0.1 0, 1 0, 1 1, -0.1 1, -0.1 0))  GEOMETRYCOLLECTION (POLYGON ((-0.1 0, 1 0, 1 1, -0.1 1, -0.1 0)))
 
 query TTTT
 SELECT