Merge #55607 #55618 #55638 #55651 #55696

55607: sql: fix reg* cast escaping r=rafiss a=otan

Resolves #53686

----

tree: add more casting fixes for text to reg*

Release note (sql change): Added ability to cast a string containing all
integers to be cast into a given regtype, e.g. '1234'::regproc is now
valid.

Release note (bug fix): Fixed a bug where casts from string to regproc,
regtype or regprocedure would not work if they contained " characters at
the beginning or at the end.

sql: fix unescaped casts to regclass types

Release note (bug fix): Fix a bug where casts to
regclass were not escaped (i.e. when the
type or table name had " characters).



55618: server: improve error message on drain timeout r=andreimatei a=andreimatei

When draining times out, we print out a warning. Commonly that warning
tells you that the leases could not be drained in time. In this case,
the patch improves the message:
- don't print a useless stack trace any more
- refer to the cluster setting controlling the timeout
- refer to leases, not "Raft leadership". Draining deals with both, but
  leases are the concept that's somewhat exposed to users.

Release note: None

55638: tpcc: remove unused and unpartitionable stock_item_fk_idx r=nvanbenschoten a=nvanbenschoten

This was discussed in #50911. We ended up not making the change because
it didn't appear to improve performance. Given the fact that the index
is only used in a single place and can easily be replaced by the primary
key of the stock table, it doesn't seem possible that this would
actually hurt performance. However, it does seem possible for the index
to hurt performance in multi-region clusters, where the fk validation
was not guaranteed to be local because it was not using the partitioned
primary key of the stock table.

This allows us to remove a scary comment around partitioning code,
because the index was not partitionable by warehouse id.

55651: *: include bazel in .gitignore r=irfansharif a=otan

Release note: None

55696: clusterversion: remove unused timestamp-related cluster versions r=lucy-zhang a=lucy-zhang

Remove `VersionTimeTZType` and `VersionTimePrecision`, which are unused.

Part of #47447.

Release note: None

Co-authored-by: Oliver Tan <[email protected]>
Co-authored-by: Andrei Matei <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Lucy Zhang <[email protected]>

.gitignore

@@ -32,3 +32,6 @@ customenv.mk
 # Generated files containing include paths.
 zcgo_flags*.go
 build/Railroad.jar
+
+# Bazel generated symlinks
+/bazel-*

pkg/ccl/importccl/read_import_avro_test.go

@@ -570,9 +570,8 @@ func benchmarkAvroImport(b *testing.B, avroOpts roachpb.AvroOptions, testData st
     s_order_cnt  integer,
     s_remote_cnt integer,
     s_data       varchar(50),
-		primary key (s_w_id, s_i_id),
-    index stock_item_fk_idx (s_i_id))
-  `)
+    primary key (s_w_id, s_i_id)
+)`)
 
 	require.NoError(b, err)
 

pkg/clusterversion/cockroach_versions.go

@@ -52,8 +52,6 @@ const (
 	VersionStatementDiagnosticsSystemTables
 	VersionSchemaChangeJob
 	VersionSavepoints
-	VersionTimeTZType
-	VersionTimePrecision
 	Version20_1
 	VersionStart20_2
 	VersionGeospatialType
@@ -293,16 +291,6 @@ var versionsSingleton = keyedVersions([]keyedVersion{
 		Key:     VersionSavepoints,
 		Version: roachpb.Version{Major: 19, Minor: 2, Unstable: 16},
 	},
-	{
-		// VersionTimeTZType enables the use of the TimeTZ data type.
-		Key:     VersionTimeTZType,
-		Version: roachpb.Version{Major: 19, Minor: 2, Unstable: 17},
-	},
-	{
-		// VersionTimePrecision enables the use of precision with time/intervals.
-		Key:     VersionTimePrecision,
-		Version: roachpb.Version{Major: 19, Minor: 2, Unstable: 18},
-	},
 	{
 		// Version20_1 is CockroachDB v20.1. It's used for all v20.1.x patch releases.
 		Key:     Version20_1,

pkg/kv/kvserver/store.go

@@ -149,17 +149,17 @@ var queueAdditionOnSystemConfigUpdateBurst = settings.RegisterNonNegativeIntSett
 	"the burst rate at which the store will add all replicas to the split and merge queue due to system config gossip",
 	32)
 
-// raftLeadershipTransferTimeout limits the amount of time a drain command
-// waits for lease transfers.
-var raftLeadershipTransferWait = func() *settings.DurationSetting {
+// leaseTransferWait limits the amount of time a drain command waits for lease
+// and Raft leadership transfers.
+var leaseTransferWait = func() *settings.DurationSetting {
 	s := settings.RegisterValidatedDurationSetting(
-		raftLeadershipTransferWaitKey,
+		leaseTransferWaitSettingName,
 		"the amount of time a server waits to transfer range leases before proceeding with the rest of the shutdown process",
 		5*time.Second,
 		func(v time.Duration) error {
 			if v < 0 {
 				return errors.Errorf("cannot set %s to a negative duration: %s",
-					raftLeadershipTransferWaitKey, v)
+					leaseTransferWaitSettingName, v)
 			}
 			return nil
 		},
@@ -168,7 +168,7 @@ var raftLeadershipTransferWait = func() *settings.DurationSetting {
 	return s
 }()
 
-const raftLeadershipTransferWaitKey = "server.shutdown.lease_transfer_wait"
+const leaseTransferWaitSettingName = "server.shutdown.lease_transfer_wait"
 
 // ExportRequestsLimit is the number of Export requests that can run at once.
 // Each extracts data from RocksDB to a temp file and then uploads it to cloud
@@ -1218,9 +1218,10 @@ func (s *Store) SetDraining(drain bool, reporter func(int, redact.SafeString)) {
 
 	// We've seen all the replicas once. Now we're going to iterate
 	// until they're all gone, up to the configured timeout.
-	transferTimeout := raftLeadershipTransferWait.Get(&s.cfg.Settings.SV)
+	transferTimeout := leaseTransferWait.Get(&s.cfg.Settings.SV)
 
-	if err := contextutil.RunWithTimeout(ctx, "wait for raft leadership transfer", transferTimeout,
+	drainLeasesOp := "transfer range leases"
+	if err := contextutil.RunWithTimeout(ctx, drainLeasesOp, transferTimeout,
 		func(ctx context.Context) error {
 			opts := retry.Options{
 				InitialBackoff: 10 * time.Millisecond,
@@ -1251,9 +1252,16 @@ func (s *Store) SetDraining(drain bool, reporter func(int, redact.SafeString)) {
 			// err, take it into account here.
 			return errors.CombineErrors(err, ctx.Err())
 		}); err != nil {
-		// You expect this message when shutting down a server in an unhealthy
-		// cluster. If we see it on healthy ones, there's likely something to fix.
-		log.Warningf(ctx, "unable to drain cleanly within %s, service might briefly deteriorate: %+v", transferTimeout, err)
+		if tErr := (*contextutil.TimeoutError)(nil); errors.As(err, &tErr) && tErr.Operation() == drainLeasesOp {
+			// You expect this message when shutting down a server in an unhealthy
+			// cluster, or when draining all nodes with replicas for some range at the
+			// same time. If we see it on healthy ones, there's likely something to fix.
+			log.Warningf(ctx, "unable to drain cleanly within %s (cluster setting %s), "+
+				"service might briefly deteriorate if the node is terminated: %s",
+				transferTimeout, leaseTransferWaitSettingName, tErr.Cause())
+		} else {
+			log.Warningf(ctx, "drain error: %+v", err)
+		}
 	}
 }
 

pkg/sql/logictest/testdata/logic_test/pgoidtype

@@ -100,7 +100,7 @@ SELECT 'blah ()'::REGPROC
 query error unknown function: blah\(\)
 SELECT 'blah( )'::REGPROC
 
-query error unknown function: blah\(, \)\(\)
+query error invalid name: expected separator \.: blah\(, \)
 SELECT 'blah(, )'::REGPROC
 
 query error more than one function named 'sqrt'
@@ -347,3 +347,42 @@ FROM
       (d.adrelid = a.attrelid AND d.adnum = a.attnum)
   JOIN (SELECT 1 AS oid, 1 AS attnum) AS vals ON
       (c.oid = vals.oid AND a.attnum = vals.attnum);
+
+statement error relation ".*"regression_53686.*"" does not exist
+SELECT '\"regression_53686\"'::regclass
+
+statement ok
+CREATE TABLE "regression_53686""" (a int)
+
+query T
+SELECT 'regression_53686"'::regclass
+----
+"regression_53686"""
+
+query T
+SELECT 'public.regression_53686"'::regclass
+----
+"regression_53686"""
+
+query T
+SELECT 'pg_catalog."radians"'::regproc
+----
+radians
+
+query T
+SELECT 'pg_catalog."radians"("float4")'::regproc
+----
+radians
+
+statement error unknown function: pg_catalog.radians"\(\)
+SELECT 'pg_catalog."radians"""'::regproc
+
+query TTTTT
+SELECT
+  '12345'::regclass::string,
+  '12345'::regtype::string,
+  '12345'::oid::string,
+  '12345'::regproc::string,
+  '12345'::regprocedure::string
+----
+12345  12345  12345  12345  12345

pkg/sql/opt/bench/bench_test.go

@@ -146,8 +146,7 @@ var schemas = [...]string{
 		s_order_cnt  integer,
 		s_remote_cnt integer,
 		s_data       varchar(50),
-		primary key (s_w_id, s_i_id),
-		index stock_item_fk_idx (s_i_id)
+		primary key (s_w_id, s_i_id)
 	)
 	`,
 	`

pkg/sql/opt/testutils/opttester/testfixtures/tpcc_schema

@@ -141,7 +141,6 @@ CREATE TABLE stock
     s_remote_cnt integer,
     s_data       varchar(50),
     primary key (s_w_id, s_i_id),
-    index stock_item_fk_idx (s_i_id),
     foreign key (s_w_id) references warehouse (w_id),
     foreign key (s_i_id) references item (i_id)
 ) interleave in parent warehouse (s_w_id)

pkg/sql/opt/xform/testdata/external/tpcc

@@ -404,9 +404,9 @@ insert order_line
       │         │    └── cardinality: [6 - 6]
       │         └── filters (true)
       └── f-k-checks-item: order_line(ol_supply_w_id,ol_i_id) -> stock(s_w_id,s_i_id)
-           └── anti-join (lookup stock@stock_item_fk_idx)
+           └── anti-join (lookup stock)
                 ├── columns: column6:35!null column5:36!null
-                ├── key columns: [36 35] = [37 38]
+                ├── key columns: [35 36] = [38 37]
                 ├── lookup columns are key
                 ├── cardinality: [0 - 6]
                 ├── with-scan &1

pkg/sql/opt/xform/testdata/external/tpcc-later-stats

@@ -407,9 +407,9 @@ insert order_line
       │         │    └── cardinality: [6 - 6]
       │         └── filters (true)
       └── f-k-checks-item: order_line(ol_supply_w_id,ol_i_id) -> stock(s_w_id,s_i_id)
-           └── anti-join (lookup stock@stock_item_fk_idx)
+           └── anti-join (lookup stock)
                 ├── columns: column6:35!null column5:36!null
-                ├── key columns: [36 35] = [37 38]
+                ├── key columns: [35 36] = [38 37]
                 ├── lookup columns are key
                 ├── cardinality: [0 - 6]
                 ├── with-scan &1

pkg/sql/opt/xform/testdata/external/tpcc-no-stats

@@ -401,9 +401,9 @@ insert order_line
       │         │    └── cardinality: [6 - 6]
       │         └── filters (true)
       └── f-k-checks-item: order_line(ol_supply_w_id,ol_i_id) -> stock(s_w_id,s_i_id)
-           └── anti-join (lookup stock@stock_item_fk_idx)
+           └── anti-join (lookup stock)
                 ├── columns: column6:35!null column5:36!null
-                ├── key columns: [36 35] = [37 38]
+                ├── key columns: [35 36] = [38 37]
                 ├── lookup columns are key
                 ├── cardinality: [0 - 6]
                 ├── with-scan &1