Merge #121731 #122058 #122060

121731: ui: index recommendations properly handle quoted table names r=dhartunian a=abarganier

Fixes: #119579

Epic: none

The UI code used to modify the index recommendations provided by the
server was not properly accounting for table and column names containing
quotation marks. This was causing invalid CREATE INDEX statements to
be generated, which would fail on request.

This patch fixes this by updating the query modification code to strip
quotation marks from the table name prior to using it to generate an
index name.

Release note (bug fix): Index recommendations in the DB Console will now
function properly for indexes on tables/columns whose names contain
quotation marks and/or whitespace.
For example: `CREATE INDEX ON "my table" ("my col");`

122058: roachtest: improve acceptance test running time, use suite r=renatolabs,srosenberg a=rickystewart

Closes #118513.

Epic: None
Release note: None

122060: go.mod: bump Pebble to 6cdb88d44473 r=RaduBerinde a=RaduBerinde

Changes:
 * [`6cdb88d4`](cockroachdb/pebble@6cdb88d4) sstable: clean up UpdateKeySuffixes

We update the Cockroach code to implement the new method.

Release note: none.
Epic: none.

Co-authored-by: Alex Barganier <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Radu Berinde <[email protected]>

DEPS.bzl

@@ -1683,10 +1683,10 @@ def go_deps():
         patches = [
             "@com_github_cockroachdb_cockroach//build/patches:com_github_cockroachdb_pebble.patch",
         ],
-        sha256 = "582dcec17e3f1ef969e7f00a239a877857c90ed818e4c37d68a10698a444eb22",
-        strip_prefix = "github.com/cockroachdb/[email protected]20240408191202-278b6a67a6f0",
+        sha256 = "f6d30631ba0ac55abbc0d06b4d28ae1d80df380cc88d0060df8cf188bdecd391",
+        strip_prefix = "github.com/cockroachdb/[email protected]20240409231136-6cdb88d44473",
         urls = [
-            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20240408191202-278b6a67a6f0.zip",
+            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20240409231136-6cdb88d44473.zip",
         ],
     )
     go_repository(

build/bazelutil/distdir_files.bzl

@@ -329,7 +329,7 @@ DISTDIR_FILES = {
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/gostdlib/com_github_cockroachdb_gostdlib-v1.19.0.zip": "c4d516bcfe8c07b6fc09b8a9a07a95065b36c2855627cb3514e40c98f872b69e",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/logtags/com_github_cockroachdb_logtags-v0.0.0-20230118201751-21c54148d20b.zip": "ca7776f47e5fecb4c495490a679036bfc29d95bd7625290cfdb9abb0baf97476",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/metamorphic/com_github_cockroachdb_metamorphic-v0.0.0-20231108215700-4ba948b56895.zip": "28c8cf42192951b69378cf537be5a9a43f2aeb35542908cc4fe5f689505853ea",
-    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20240408191202-278b6a67a6f0.zip": "582dcec17e3f1ef969e7f00a239a877857c90ed818e4c37d68a10698a444eb22",
+    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20240409231136-6cdb88d44473.zip": "f6d30631ba0ac55abbc0d06b4d28ae1d80df380cc88d0060df8cf188bdecd391",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/redact/com_github_cockroachdb_redact-v1.1.5.zip": "11b30528eb0dafc8bc1a5ba39d81277c257cbe6946a7564402f588357c164560",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/returncheck/com_github_cockroachdb_returncheck-v0.0.0-20200612231554-92cdbca611dd.zip": "ce92ba4352deec995b1f2eecf16eba7f5d51f5aa245a1c362dfe24c83d31f82b",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/stress/com_github_cockroachdb_stress-v0.0.0-20220803192808-1806698b1b7b.zip": "3fda531795c600daf25532a4f98be2a1335cd1e5e182c72789bca79f5f69fcc1",

build/github/local-roachtest.sh

@@ -35,11 +35,12 @@ cp $EXECROOT/external/archived_cdep_libgeos_linux/lib/libgeos.so lib/libgeos.so
 cp $EXECROOT/external/archived_cdep_libgeos_linux/lib/libgeos_c.so lib/libgeos_c.so
 chmod a+w lib/libgeos.so lib/libgeos_c.so
 
-$BAZEL_BIN/pkg/cmd/roachtest/roachtest_/roachtest run acceptance \
+$BAZEL_BIN/pkg/cmd/roachtest/roachtest_/roachtest run \
   --local \
   --parallelism=1 \
   --cockroach "$BAZEL_BIN/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short" \
   --workload "$BAZEL_BIN/pkg/cmd/workload/workload_/workload" \
   --artifacts $PWD/artifacts \
-  --github
+  --github \
+  --suite acceptance
 

go.mod

@@ -124,7 +124,7 @@ require (
 	github.com/cockroachdb/go-test-teamcity v0.0.0-20191211140407-cff980ad0a55
 	github.com/cockroachdb/gostdlib v1.19.0
 	github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b
-	github.com/cockroachdb/pebble v0.0.0-20240408191202-278b6a67a6f0
+	github.com/cockroachdb/pebble v0.0.0-20240409231136-6cdb88d44473
 	github.com/cockroachdb/redact v1.1.5
 	github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd
 	github.com/cockroachdb/stress v0.0.0-20220803192808-1806698b1b7b

go.sum

@@ -505,8 +505,8 @@ github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b h1:r6VH0faHjZe
 github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs=
 github.com/cockroachdb/metamorphic v0.0.0-20231108215700-4ba948b56895 h1:XANOgPYtvELQ/h4IrmPAohXqe2pWA8Bwhejr3VQoZsA=
 github.com/cockroachdb/metamorphic v0.0.0-20231108215700-4ba948b56895/go.mod h1:aPd7gM9ov9M8v32Yy5NJrDyOcD8z642dqs+F0CeNXfA=
-github.com/cockroachdb/pebble v0.0.0-20240408191202-278b6a67a6f0 h1:GCbJXi7f6EPZl9Ge877FGLVRN0ukY9SQhJJmvUufPBs=
-github.com/cockroachdb/pebble v0.0.0-20240408191202-278b6a67a6f0/go.mod h1:gm/vT3lwZUKyB3iTDgWIZfC0hu0gLr+VcXr/tZeTdEU=
+github.com/cockroachdb/pebble v0.0.0-20240409231136-6cdb88d44473 h1:/w0fCyZQgS0TowF2NBPGPF3SX1qmprPoDiYHrZ6VAnU=
+github.com/cockroachdb/pebble v0.0.0-20240409231136-6cdb88d44473/go.mod h1:gm/vT3lwZUKyB3iTDgWIZfC0hu0gLr+VcXr/tZeTdEU=
 github.com/cockroachdb/redact v1.1.3/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
 github.com/cockroachdb/redact v1.1.5 h1:u1PMllDkdFfPWaNGMyLD1+so+aq3uUItthCFqzwPJ30=
 github.com/cockroachdb/redact v1.1.5/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=

pkg/cmd/roachtest/registry/test_spec.go

@@ -279,11 +279,12 @@ const (
 	PebbleNightlyYCSB     = "pebble_nightly_ycsb"
 	PebbleNightlyYCSBRace = "pebble_nightly_ycsb_race"
 	Roachtest             = "roachtest"
+	Acceptance            = "acceptance"
 )
 
 var allSuites = []string{
 	Nightly, Weekly, ReleaseQualification, ORM, Driver, Tool, Smoketest, Quick, Fixtures,
-	Pebble, PebbleNightlyWrite, PebbleNightlyYCSB, PebbleNightlyYCSBRace, Roachtest,
+	Pebble, PebbleNightlyWrite, PebbleNightlyYCSB, PebbleNightlyYCSBRace, Roachtest, Acceptance,
 }
 
 // SuiteSet represents a set of suites.

pkg/cmd/roachtest/registry/testdata/filter/errors

@@ -10,7 +10,7 @@ error: invalid owner "badowner"
 
 filter suite=badsuite
 ----
-error: invalid suite "badsuite"; valid suites are nightly,weekly,release_qualification,orm,driver,tool,smoketest,quick,fixtures,pebble,pebble_nightly_write,pebble_nightly_ycsb,pebble_nightly_ycsb_race,roachtest
+error: invalid suite "badsuite"; valid suites are nightly,weekly,release_qualification,orm,driver,tool,smoketest,quick,fixtures,pebble,pebble_nightly_write,pebble_nightly_ycsb,pebble_nightly_ycsb_race,roachtest,acceptance
 
 filter cloud=badcloud owner=badowner suite=badsuite
 ----

pkg/cmd/roachtest/tests/acceptance.go

@@ -133,7 +133,7 @@ func registerAcceptance(r registry.Registry) {
 				EncryptionSupport: tc.encryptionSupport,
 				Timeout:           10 * time.Minute,
 				CompatibleClouds:  registry.AllExceptAWS,
-				Suites:            registry.Suites(registry.Nightly, registry.Quick),
+				Suites:            registry.Suites(registry.Nightly, registry.Quick, registry.Acceptance),
 				RequiresLicense:   tc.requiresLicense,
 			}
 

pkg/cmd/roachtest/tests/versionupgrade.go

@@ -101,17 +101,19 @@ DROP TABLE splitmerge.t;
 
 func runVersionUpgrade(ctx context.Context, t test.Test, c cluster.Cluster) {
 	testCtx := ctx
+	opts := []mixedversion.CustomOption{
+		mixedversion.AlwaysUseFixtures,
+		mixedversion.AlwaysUseLatestPredecessors,
+	}
 	if c.IsLocal() {
 		localTimeout := 30 * time.Minute
 		var cancel context.CancelFunc
 		testCtx, cancel = context.WithTimeout(ctx, localTimeout)
 		defer cancel()
+		opts = append(opts, mixedversion.NumUpgrades(1))
 	}
 
-	mvt := mixedversion.NewTest(
-		testCtx, t, t.L(), c, c.All(),
-		mixedversion.AlwaysUseFixtures, mixedversion.AlwaysUseLatestPredecessors,
-	)
+	mvt := mixedversion.NewTest(testCtx, t, t.L(), c, c.All(), opts...)
 	mvt.OnStartup(
 		"setup schema changer workload",
 		func(ctx context.Context, l *logger.Logger, rng *rand.Rand, h *mixedversion.Helper) error {

pkg/storage/pebble.go

@@ -568,10 +568,7 @@ type pebbleDataBlockMVCCTimeIntervalPointCollector struct {
 	pebbleDataBlockMVCCTimeIntervalCollector
 }
 
-var (
-	_ sstable.DataBlockIntervalCollector      = (*pebbleDataBlockMVCCTimeIntervalPointCollector)(nil)
-	_ sstable.SuffixReplaceableBlockCollector = (*pebbleDataBlockMVCCTimeIntervalPointCollector)(nil)
-)
+var _ sstable.DataBlockIntervalCollector = (*pebbleDataBlockMVCCTimeIntervalPointCollector)(nil)
 
 func (tc *pebbleDataBlockMVCCTimeIntervalPointCollector) Add(
 	key pebble.InternalKey, _ []byte,
@@ -585,10 +582,7 @@ type pebbleDataBlockMVCCTimeIntervalRangeCollector struct {
 	pebbleDataBlockMVCCTimeIntervalCollector
 }
 
-var (
-	_ sstable.DataBlockIntervalCollector      = (*pebbleDataBlockMVCCTimeIntervalRangeCollector)(nil)
-	_ sstable.SuffixReplaceableBlockCollector = (*pebbleDataBlockMVCCTimeIntervalRangeCollector)(nil)
-)
+var _ sstable.DataBlockIntervalCollector = (*pebbleDataBlockMVCCTimeIntervalRangeCollector)(nil)
 
 func (tc *pebbleDataBlockMVCCTimeIntervalRangeCollector) Add(
 	key pebble.InternalKey, value []byte,
@@ -668,6 +662,7 @@ func decodeWallTime(ts []byte) uint64 {
 	return binary.BigEndian.Uint64(ts[0:engineKeyVersionWallTimeLen])
 }
 
+// FinishDataBlock is part of the sstable.DataBlockIntervalCollector interface.
 func (tc *pebbleDataBlockMVCCTimeIntervalCollector) FinishDataBlock() (
 	lower uint64,
 	upper uint64,
@@ -699,12 +694,20 @@ func (tc *pebbleDataBlockMVCCTimeIntervalCollector) FinishDataBlock() (
 	return lower, upper, nil
 }
 
-func (tc *pebbleDataBlockMVCCTimeIntervalCollector) UpdateKeySuffixes(
-	_ []byte, _, newSuffix []byte,
+// AddCollectedWithSuffixReplacement is part of the
+// sstable.DataBlockIntervalCollector interface.
+func (tc *pebbleDataBlockMVCCTimeIntervalCollector) AddCollectedWithSuffixReplacement(
+	oldLower, oldUpper uint64, oldSuffix, newSuffix []byte,
 ) error {
 	return tc.add(newSuffix)
 }
 
+// SupportsSuffixReplacement is part of the sstable.DataBlockIntervalCollector
+// interface.
+func (tc *pebbleDataBlockMVCCTimeIntervalCollector) SupportsSuffixReplacement() bool {
+	return true
+}
+
 const mvccWallTimeIntervalCollector = "MVCCTimeInterval"
 
 var _ pebble.BlockPropertyFilterMask = (*mvccWallTimeIntervalRangeKeyMask)(nil)

pkg/storage/pebble_test.go

@@ -615,19 +615,17 @@ func TestPebbleMVCCTimeIntervalCollector(t *testing.T) {
 	// Using the same suffix for all keys in a block results in an interval of
 	// width one (inclusive lower bound to exclusive upper bound).
 	suffix := EncodeMVCCTimestampSuffix(hlc.Timestamp{WallTime: 42, Logical: 1})
-	require.NoError(t, collector.UpdateKeySuffixes(
-		nil /* old prop */, nil /* old suffix */, suffix,
-	))
+	require.NoError(t, collector.AddCollectedWithSuffixReplacement(0, 0, nil, suffix))
 	finishAndCheck(42, 43)
 	// An invalid key results in an error.
 	// Case 1: malformed sentinel.
 	key := EncodeMVCCKey(MVCCKey{aKey, hlc.Timestamp{WallTime: 2, Logical: 1}})
 	sentinelPos := len(key) - 1 - int(key[len(key)-1])
 	key[sentinelPos] = '\xff'
-	require.Error(t, collector.UpdateKeySuffixes(nil, nil, key))
+	require.Error(t, collector.AddCollectedWithSuffixReplacement(0, 0, nil, key))
 	// Case 2: malformed bare suffix (too short).
 	suffix = EncodeMVCCTimestampSuffix(hlc.Timestamp{WallTime: 42, Logical: 1})[1:]
-	require.Error(t, collector.UpdateKeySuffixes(nil, nil, suffix))
+	require.Error(t, collector.AddCollectedWithSuffixReplacement(0, 0, nil, suffix))
 }
 
 // TestPebbleMVCCTimeIntervalCollectorAndFilter tests that point and range key

pkg/ui/workspaces/cluster-ui/src/insights/indexActionBtn.spec.ts

@@ -69,6 +69,25 @@ describe("Create index name", () => {
       expected:
         "CREATE INDEX IF NOT EXISTS t_expr_storing_rec_idx ON t ((i + l)), (j + k), a) STORING (k)",
     },
+    {
+      name: "handles table names containing quotes, doesn't include quotes in idx name",
+      query:
+        'CREATE INDEX ON defaultdb.public."offers"."startdate" (n) STORING (b);',
+      expected:
+        'CREATE INDEX IF NOT EXISTS startdate_n_storing_rec_idx ON defaultdb.public."offers"."startdate" (n) STORING (b);',
+    },
+    {
+      name: "handles table and column names containing quotes & whitespace, doesn't include quotes in idx name",
+      query: 'CREATE INDEX ON "my table" ("my col");',
+      expected:
+        'CREATE INDEX IF NOT EXISTS my_table_my_col_rec_idx ON "my table" ("my col");',
+    },
+    {
+      name: "handles quotes within quotes, doesn't include quotes in idx name",
+      query: 'CREATE INDEX ON "my""table" ("with""quote");',
+      expected:
+        'CREATE INDEX IF NOT EXISTS mytable_withquote_rec_idx ON "my""table" ("with""quote");',
+    },
   ];
 
   for (let i = 0; i < testCases.length; i++) {

pkg/ui/workspaces/cluster-ui/src/insights/indexActionBtn.tsx

@@ -264,7 +264,9 @@ export function createIdxName(statement: string): string {
   // The table name is fully qualified at this point, but we don't need the full name,
   // so just use the last value (also an index name can't have ".")
   const idxNameArr = idxName.split(".");
-  idxName = idxNameArr[idxNameArr.length - 1].replace(/\s/g, "_") + suffix;
+  idxName =
+    idxNameArr[idxNameArr.length - 1].replace(/"/g, "").replace(/\s/g, "_") +
+    suffix;
 
   return statement.replace(
     "CREATE INDEX ON ",