Merge cockroachdb#107224 cockroachdb#107301

107224: compose: Deflake testComposeCompare r=Xiang-Gu a=Xiang-Gu

We identified the fixed the following issues concerning test testComposeCompare:
1. Disallow several functions because they're not supported in Postgist
2. Disable random generation of `OID` type because it's natural to have the same OIDs assigned to different objects between CRDB and Postgist.
3. Disable using index hints because Postgist does not support this feature.
4. Disable locales because Postgist requires them to be double-quoted
and it's not feasible to change locale name formatting to be double-quoted.
5. Fixed a test bug where we'd mistakenly allow generation of stmts like `CREATE TABLE t (... inverted index (...))` in our test against Postgist.
6. Make this test a "enormous" test which has an 1-hour timeout because it executes two subtests sequentially for 10m each.

Inform cockroachdb#82867
Epic: None
Release note: None

107301: storage,clusterversion: Bump pebble FormatMajorVersion for VSSTs r=dt a=itsbilal

This change bumps Pebble's FormatMajorVersion used with CockroachDB to ExperimentalFormatVirtualSSTables. This doesn't do anything right off the bat, however it enables future work to land that will create virtual sstables (eg. disaggregated storage, online restore, ingest-time splits).

Epic: none

Release note: None

Co-authored-by: Xiang Gu <[email protected]>
Co-authored-by: Bilal Akhtar <[email protected]>

Author: 3 people

docs/generated/settings/settings-for-tenants.txt

@@ -303,4 +303,4 @@ trace.opentelemetry.collector	string		address of an OpenTelemetry trace collecto
 trace.snapshot.rate	duration	0s	if non-zero, interval at which background trace snapshots are captured	tenant-rw
 trace.span_registry.enabled	boolean	true	if set, ongoing traces can be seen at https://<ui>/#/debug/tracez	tenant-rw
 trace.zipkin.collector	string		the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.	tenant-rw
-version	version	1000023.1-14	set the active cluster version in the format '<major>.<minor>'	tenant-rw
+version	version	1000023.1-16	set the active cluster version in the format '<major>.<minor>'	tenant-rw

docs/generated/settings/settings.html

@@ -259,6 +259,6 @@
 <tr><td><div id="setting-trace-span-registry-enabled" class="anchored"><code>trace.span_registry.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>if set, ongoing traces can be seen at https://&lt;ui&gt;/#/debug/tracez</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-trace-zipkin-collector" class="anchored"><code>trace.zipkin.collector</code></div></td><td>string</td><td><code></code></td><td>the address of a Zipkin instance to receive traces, as &lt;host&gt;:&lt;port&gt;. If no port is specified, 9411 will be used.</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-ui-display-timezone" class="anchored"><code>ui.display_timezone</code></div></td><td>enumeration</td><td><code>etc/utc</code></td><td>the timezone used to format timestamps in the ui [etc/utc = 0, america/new_york = 1]</td><td>Dedicated/Self-Hosted</td></tr>
-<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000023.1-14</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
+<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000023.1-16</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 </tbody>
 </table>

pkg/clusterversion/cockroach_versions.go

@@ -553,6 +553,10 @@ const (
 	// DeleteSized operations.
 	V23_2_UseSizedPebblePointTombstones
 
+	// V23_2_PebbleFormatVirtualSSTables upgrades Pebble's format major version to
+	// FormatVirtualSSTables, allowing use of virtual sstables in Pebble.
+	V23_2_PebbleFormatVirtualSSTables
+
 	// *************************************************
 	// Step (1) Add new versions here.
 	// Do not add new versions to a patch release.
@@ -962,6 +966,10 @@ var rawVersionsSingleton = keyedVersions{
 		Key:     V23_2_UseSizedPebblePointTombstones,
 		Version: roachpb.Version{Major: 23, Minor: 1, Internal: 14},
 	},
+	{
+		Key:     V23_2_PebbleFormatVirtualSSTables,
+		Version: roachpb.Version{Major: 23, Minor: 1, Internal: 16},
+	},
 
 	// *************************************************
 	// Step (2): Add new versions here.

pkg/compose/BUILD.bazel

@@ -9,8 +9,12 @@ go_library(
 
 go_test(
     name = "compose_test",
+    size = "enormous",
     srcs = ["compose_test.go"],
-    args = ["-test.timeout=295s"],
+    args = select({
+        "//build/toolchains:use_ci_timeouts": ["-test.timeout=895s"],
+        "//conditions:default": ["-test.timeout=3595s"],
+    }),
     data = [
         "//c-deps:libgeos",
         "//pkg/compose:compare/docker-compose.yml",

pkg/compose/compose_test.go

@@ -32,6 +32,8 @@ import (
 )
 
 var (
+	// flagEach controls how long we are going to run each compose test. Ensure bazel BUILD file
+	// of compose tests has a longer timeout.
 	flagEach      = flag.Duration("each", 10*time.Minute, "individual test timeout")
 	flagTests     = flag.String("tests", ".", "tests within docker compose to run")
 	flagArtifacts = flag.String("artifacts", "", "artifact directory")

pkg/internal/sqlsmith/scalar.go

@@ -233,6 +233,9 @@ func getColRef(s *Smither, typ *types.T, refs colRefs) (tree.TypedExpr, *colRef,
 	if s.disableDecimals && col.typ.Family() == types.DecimalFamily {
 		return nil, nil, false
 	}
+	if s.disableOIDs && col.typ.Family() == types.OidFamily {
+		return nil, nil, false
+	}
 	return col.typedExpr(), col, true
 }
 

pkg/internal/sqlsmith/sqlsmith.go

@@ -105,6 +105,7 @@ type Smither struct {
 	disableInsertSelect        bool
 	disableDivision            bool
 	disableDecimals            bool
+	disableOIDs                bool
 	disableUDFs                bool
 
 	bulkSrv     *httptest.Server
@@ -508,6 +509,11 @@ var DisableDecimals = simpleOption("disable decimals", func(s *Smither) {
 	s.disableDecimals = true
 })
 
+// DisableOIDs disables use of OID types in the query.
+var DisableOIDs = simpleOption("disable OIDs", func(s *Smither) {
+	s.disableOIDs = true
+})
+
 // DisableUDFs causes the Smither to disable user-defined functions.
 var DisableUDFs = simpleOption("disable udfs", func(s *Smither) {
 	s.disableUDFs = true
@@ -536,6 +542,12 @@ var PostgresMode = multiOption(
 	simpleOption("postgres", func(s *Smither) {
 		s.postgres = true
 	})(),
+	// Postgres does not support index hinting.
+	DisableIndexHints(),
+	// CockroachDB supports OID type but the same OID value might be assigned to
+	// different objects from Postgres, and we thus disable using OID types in
+	// randomly generated queries.
+	DisableOIDs(),
 
 	// Some func impls differ from postgres, so skip them here.
 	// #41709
@@ -559,6 +571,9 @@ var PostgresMode = multiOption(
 	IgnoreFNs("^postgis_.*build_date"),
 	IgnoreFNs("^postgis_.*version"),
 	IgnoreFNs("^postgis_.*scripts"),
+	IgnoreFNs("hlc_to_timestamp"),
+	IgnoreFNs("st_s2covering"),
+	IgnoreFNs("sum_int"),
 )
 
 // MutatingMode causes the Smither to generate mutation statements in the same

pkg/internal/sqlsmith/type.go

@@ -47,6 +47,10 @@ func (s *Smither) pickAnyType(typ *types.T) *types.T {
 		if s.disableDecimals {
 			typ = s.randType()
 		}
+	case types.OidFamily:
+		if s.disableOIDs {
+			typ = s.randType()
+		}
 	}
 	return typ
 }
@@ -58,11 +62,14 @@ func (s *Smither) randScalarType() *types.T {
 	if s.types != nil {
 		scalarTypes = s.types.scalarTypes
 	}
-	typ := randgen.RandTypeFromSlice(s.rnd, scalarTypes)
-	if s.disableDecimals {
-		for typ.Family() == types.DecimalFamily {
-			typ = randgen.RandTypeFromSlice(s.rnd, scalarTypes)
+	var typ *types.T
+	for {
+		typ = randgen.RandTypeFromSlice(s.rnd, scalarTypes)
+		if (s.disableDecimals && typ.Family() == types.DecimalFamily) ||
+			(s.disableOIDs && typ.Family() == types.OidFamily) {
+			continue
 		}
+		break
 	}
 	return typ
 }
@@ -91,11 +98,14 @@ func (s *Smither) randType() *types.T {
 	if s.types != nil {
 		seedTypes = s.types.seedTypes
 	}
-	typ := randgen.RandTypeFromSlice(s.rnd, seedTypes)
-	if s.disableDecimals {
-		for typ.Family() == types.DecimalFamily {
-			typ = randgen.RandTypeFromSlice(s.rnd, seedTypes)
+	var typ *types.T
+	for {
+		typ = randgen.RandTypeFromSlice(s.rnd, seedTypes)
+		if s.disableDecimals && typ.Family() == types.DecimalFamily ||
+			(s.disableOIDs && typ.Family() == types.OidFamily) {
+			continue
 		}
+		break
 	}
 	return typ
 }

pkg/sql/randgen/mutator.go

@@ -742,10 +742,20 @@ func postgresCreateTableMutator(
 			for _, def := range stmt.Defs {
 				switch def := def.(type) {
 				case *tree.ColumnTableDef:
-					colTypes[string(def.Name)] = tree.MustBeStaticallyKnownType(def.Type)
+					colType := tree.MustBeStaticallyKnownType(def.Type)
+					// Disable locales because CRDB random generator generates
+					// locales without double-quotes, and they are unknown to
+					// Postgres (e.g. `i TEXT COLLATE de_DE` won't work with
+					// Postgres).
+					if colType.Family() == types.CollatedStringFamily {
+						colType = types.String
+					}
+					colTypes[string(def.Name)] = colType
 				}
 			}
 
+			// Exclude `INDEX` and `UNIQUE` table defs and hoist them into separate
+			// `CREATE INDEX` and `CREATE UNIQUE INDEX` statements.
 			var newdefs tree.TableDefs
 			for _, def := range stmt.Defs {
 				switch def := def.(type) {
@@ -774,6 +784,8 @@ func postgresCreateTableMutator(
 						break
 					}
 					def.Columns = newCols
+					// Hoist this IndexTableDef into a separate CreateIndex.
+					changed = true
 					// TODO(rafi): Postgres supports inverted indexes with a different
 					// syntax than Cockroach. Maybe we could add it later.
 					// The syntax is `CREATE INDEX name ON table USING gin(column)`.
@@ -786,7 +798,6 @@ func postgresCreateTableMutator(
 							Storing:  def.Storing,
 							// Postgres doesn't support NotVisible Index, so NotVisible is not populated here.
 						})
-						changed = true
 					}
 				case *tree.UniqueConstraintTableDef:
 					var newCols tree.IndexElemList

pkg/storage/pebble.go

@@ -2168,6 +2168,9 @@ func (p *Pebble) SetMinVersion(version roachpb.Version) error {
 	var formatVers pebble.FormatMajorVersion
 	// Cases are ordered from newer to older versions.
 	switch {
+	case !version.Less(clusterversion.ByKey(clusterversion.V23_2_PebbleFormatVirtualSSTables)):
+		formatVers = pebble.ExperimentalFormatVirtualSSTables
+
 	case !version.Less(clusterversion.ByKey(clusterversion.V23_2_PebbleFormatDeleteSizedAndObsolete)):
 		formatVers = pebble.ExperimentalFormatDeleteSizedAndObsolete