Merge #74612 #75726 #75770 #75771 #75807 #75813 #75836

74612: rangefeedcache,settingswatcher,systemcfgwatcher: lose gossip deps r=ajwerner a=ajwerner

This is a pile of commits which supersedes #69269 and pretty much puts in place all of the pieces to move on #70560. 

75726: sql: refactor pg_has_role to remove privilege.GRANT usage r=RichardJCai a=ecwall

refs #73129

Also combines some layers of privilege checking code.

Release note: None

75770: vendor: bump cockroachdb/apd to v3.1.0, speed up decimal division r=nvanbenschoten a=nvanbenschoten

Picks up two PRs that improved the performance of `Quo`, `Sqrt`, `Cbrt`, `Exp`, `Ln`, `Log`, and `Pow`:
- cockroachdb/apd#114
- cockroachdb/apd#115

Almost all of the testing changes here are due to the rounding behavior in cockroachdb/apd#115. This brings us closer to PG's behavior, but also creates a lot of noise in this diff. To verify that this noise wasn't hiding any correctness regressions caused by the rewrite of `Context.Quo` in the first PR, I created #75757, which only includes the first PR. #75757 passes CI with minimal testing changes. The testing changes that PR did require all have to do with trailing zeros, and most of them are replaced in this PR.

Release note (performance improvement): The performance of many DECIMAL arithmetic operators has been improved by as much as 60%. These operators include division (`/`), `sqrt`, `cbrt`, `exp`, `ln`, `log`, and `pow`.

----

### Speedup on TPC-DS dataset

The TPC-DS dataset is full of decimal columns, so it's a good playground to test this change. Unfortunately, the variance in the runtime performance of the TPC-DS queries themselves is high (many queries varied by 30-40% per attempt), so it was hard to get signal out of them. Instead, I imported the TPC-DS dataset with a scale factor of 10 and ran some custom aggregation queries against the largest table (web_sales, row count = 7,197,566):

```sql
# q1
select sum(ws_wholesale_cost / ws_ext_list_price) from web_sales;

# q2
select sum(ws_wholesale_cost / ws_ext_list_price - sqrt(ws_net_paid_inc_tax)) from web_sales;
```

Here's the difference in runtime of these two queries before and after this change on an `n2-standard-8` instance:

```
name              old s/op   new s/op   delta
TPC-DS/custom/q1  22.4 ± 0%   8.6 ± 0%  -61.33%  (p=0.002 n=6+6)
TPC-DS/custom/q2  91.4 ± 0%  32.1 ± 0%  -64.85%  (p=0.008 n=5+5)
```

75771: colexec: close the ordered sync used by the external sorter r=yuzefovich a=yuzefovich

**colexec: close the ordered sync used by the external sorter**

Previously, we forgot to close the ordered synchronizer that is used by
the external sorter to merge already sorted partitions. This could
result in some tracing spans never being finished and is now fixed.

Release note: None

**colexec: return an error rather than logging it on Close in some cases**

This error eventually will be logged anyway, but we should try to
propagate it up the stack as much as possible.

Release note: None

75807: ui: Add CircleFilled component r=ericharmeling a=ericharmeling

Previously, there was no component for a filled circle icon in the `ui` package.
Recent product designs have requested this icon for the DB Console (see #67510, #73463).
This PR adds a `CircleFilled` component to the `ui` package.

Release note: None

75813: sql: fix flakey TestShowRangesMultipleStores r=ajwerner a=ajwerner

Fixes #75699

Release note: None

75836: dev: add generate protobuf r=ajwerner a=ajwerner

Convenient, fast.

Release note: None

Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Evan Wall <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Eric Harmeling <[email protected]>

DEPS.bzl

@@ -1114,10 +1114,10 @@ def go_deps():
         name = "com_github_cockroachdb_apd_v3",
         build_file_proto_mode = "disable_global",
         importpath = "github.com/cockroachdb/apd/v3",
-        sha256 = "29727b520b1239e501cbad8d1215ca2d1d5f79c60ca26e6c954153fc81f6ceb4",
-        strip_prefix = "github.com/cockroachdb/apd/v3@v3.0.1",
+        sha256 = "0571a4bf35a79df25f1e4a0b807ce3e30f7edbf69412d0272affaa925e25f625",
+        strip_prefix = "github.com/cockroachdb/apd/v3@v3.1.0",
         urls = [
-            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/apd/v3/com_github_cockroachdb_apd_v3-v3.0.1.zip",
+            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/apd/v3/com_github_cockroachdb_apd_v3-v3.1.0.zip",
         ],
     )
     go_repository(

dev

@@ -3,7 +3,7 @@
 set -euo pipefail
 
 # Bump this counter to force rebuilding `dev` on all machines.
-DEV_VERSION=8
+DEV_VERSION=9
 
 THIS_DIR=$(cd "$(dirname "$0")" && pwd)
 BINARY_DIR=$THIS_DIR/bin/dev-versions

go.mod

@@ -32,7 +32,7 @@ require (
 	github.com/bufbuild/buf v0.56.0
 	github.com/cenkalti/backoff v2.2.1+incompatible
 	github.com/client9/misspell v0.3.4
-	github.com/cockroachdb/apd/v3 v3.0.1
+	github.com/cockroachdb/apd/v3 v3.1.0
 	github.com/cockroachdb/circuitbreaker v2.2.2-0.20190114160014-a614b14ccf63+incompatible
 	github.com/cockroachdb/cmux v0.0.0-20170110192607-30d10be49292
 	github.com/cockroachdb/cockroach-go/v2 v2.2.6

go.sum

@@ -387,8 +387,8 @@ github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1 h1:zH8ljVhhq7yC0MIeUL/
 github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs=
 github.com/cockroachdb/apd v1.1.0 h1:3LFP3629v+1aKXU5Q37mxmRxX/pIu1nijXydLShEq5I=
 github.com/cockroachdb/apd v1.1.0/go.mod h1:8Sl8LxpKi29FqWXR16WEFZRNSz3SoPzUzeMeY4+DwBQ=
-github.com/cockroachdb/apd/v3 v3.0.1 h1:g1iaIyOhxq9f6iQ95fiPFJl9uCU69B3NkGXE2RLdZpI=
-github.com/cockroachdb/apd/v3 v3.0.1/go.mod h1:6qgPBMXjATAdD/VefbRP9NoSLKjbB4LCoA7gN4LpHs4=
+github.com/cockroachdb/apd/v3 v3.1.0 h1:MK3Ow7LH0W8zkd5GMKA1PvS9qG3bWFI95WaVNfyZJ/w=
+github.com/cockroachdb/apd/v3 v3.1.0/go.mod h1:6qgPBMXjATAdD/VefbRP9NoSLKjbB4LCoA7gN4LpHs4=
 github.com/cockroachdb/circuitbreaker v2.2.2-0.20190114160014-a614b14ccf63+incompatible h1:u3uQ4oAKM5g2eODBAsDdDSrTs7zRWXtvu+nvSDA9098=
 github.com/cockroachdb/circuitbreaker v2.2.2-0.20190114160014-a614b14ccf63+incompatible/go.mod h1:v3T8+rm/HmCL0D1BwDcGaHHAQDuFPW7EsnYs2nBRqUo=
 github.com/cockroachdb/cmux v0.0.0-20170110192607-30d10be49292 h1:dzj1/xcivGjNPwwifh/dWTczkwcuqsXXFHY1X/TZMtw=

pkg/BUILD.bazel

@@ -124,6 +124,7 @@ ALL_TESTS = [
     "//pkg/kv/kvclient/kvstreamer:kvstreamer_test",
     "//pkg/kv/kvclient/rangecache:rangecache_test",
     "//pkg/kv/kvclient/rangefeed/rangefeedbuffer:rangefeedbuffer_test",
+    "//pkg/kv/kvclient/rangefeed/rangefeedcache:rangefeedcache_test",
     "//pkg/kv/kvclient/rangefeed:rangefeed_test",
     "//pkg/kv/kvnemesis:kvnemesis_test",
     "//pkg/kv/kvprober:kvprober_test",
@@ -186,6 +187,7 @@ ALL_TESTS = [
     "//pkg/server/serverpb:serverpb_test",
     "//pkg/server/settingswatcher:settingswatcher_test",
     "//pkg/server/status:status_test",
+    "//pkg/server/systemconfigwatcher:systemconfigwatcher_test",
     "//pkg/server/telemetry:telemetry_test",
     "//pkg/server/tracedumper:tracedumper_test",
     "//pkg/server:server_test",

pkg/ccl/benchccl/rttanalysisccl/testdata/benchmark_expectations

@@ -2,7 +2,7 @@ exp,benchmark
 21,AlterPrimaryRegion/alter_empty_database_alter_primary_region
 25-26,AlterPrimaryRegion/alter_empty_database_set_initial_primary_region
 21,AlterPrimaryRegion/alter_populated_database_alter_primary_region
-27,AlterPrimaryRegion/alter_populated_database_set_initial_primary_region
+26,AlterPrimaryRegion/alter_populated_database_set_initial_primary_region
 20,AlterRegions/alter_empty_database_add_region
 21,AlterRegions/alter_empty_database_drop_region
 20,AlterRegions/alter_populated_database_add_region

pkg/ccl/importccl/import_table_creation.go

@@ -329,14 +329,14 @@ func (so *importSequenceOperators) IsTypeVisible(
 	return false, false, errors.WithStack(errSequenceOperators)
 }
 
-// HasPrivilege is part of the tree.EvalDatabase interface.
-func (so *importSequenceOperators) HasPrivilege(
+// HasAnyPrivilege is part of the tree.EvalDatabase interface.
+func (so *importSequenceOperators) HasAnyPrivilege(
 	ctx context.Context,
 	specifier tree.HasPrivilegeSpecifier,
 	user security.SQLUsername,
-	priv privilege.Privilege,
-) (bool, error) {
-	return false, errors.WithStack(errSequenceOperators)
+	privs []privilege.Privilege,
+) (tree.HasAnyPrivilegeResult, error) {
+	return tree.HasNoPrivilege, errors.WithStack(errSequenceOperators)
 }
 
 // IncrementSequenceByID implements the tree.SequenceOperators interface.

pkg/ccl/logictestccl/testdata/logic_test/multi_region_query_behavior

@@ -1,6 +1,12 @@
 # LogicTest: multiregion-9node-3region-3azs
 # TODO(#69265): enable multiregion-9node-3region-3azs-tenant.
 
+# Set the closed timestamp interval to be short to shorten the amount of time
+# we need to wait for the system config to propagate.
+statement ok
+SET CLUSTER SETTING kv.closed_timestamp.side_transport_interval = '10ms';
+SET CLUSTER SETTING kv.closed_timestamp.target_duration = '10ms';
+
 statement ok
 CREATE DATABASE multi_region_test_db PRIMARY REGION "ca-central-1" REGIONS "ap-southeast-2", "us-east-1";
 USE multi_region_test_db
@@ -187,7 +193,7 @@ ALTER TABLE history INJECT STATISTICS '[
 
 # Regression test for #63735. Ensure that we choose locality optimized anti
 # joins for the foreign key checks.
-query T
+query T retry
 EXPLAIN INSERT
 INTO
   history (h_c_id, h_c_d_id, h_c_w_id, h_d_id, h_w_id, h_amount, h_date, h_data)

pkg/ccl/logictestccl/testdata/logic_test/regional_by_row_query_behavior

@@ -2,6 +2,12 @@
 # TODO(#69265): enable multiregion-9node-3region-3azs-tenant and/or revert
 # the commit that split these changes out.
 
+# Set the closed timestamp interval to be short to shorten the amount of time
+# we need to wait for the system config to propagate.
+statement ok
+SET CLUSTER SETTING kv.closed_timestamp.side_transport_interval = '10ms';
+SET CLUSTER SETTING kv.closed_timestamp.target_duration = '10ms';
+
 statement ok
 CREATE DATABASE multi_region_test_db PRIMARY REGION "ca-central-1" REGIONS "ap-southeast-2", "us-east-1" SURVIVE REGION FAILURE;
 USE multi_region_test_db
@@ -20,7 +26,7 @@ CREATE TABLE regional_by_row_table (
 ) LOCALITY REGIONAL BY ROW
 
 # Do a REGEXP replace of the enums as these may not be static.
-query T
+query T retry
 SELECT regexp_replace(info, '@\d+', '@<enum_val>', 'g') FROM
 [EXPLAIN (OPT, CATALOG) SELECT * FROM regional_by_row_table]
 ----

pkg/ccl/serverccl/BUILD.bazel

@@ -29,6 +29,7 @@ go_test(
         "//pkg/security/securitytest",
         "//pkg/server",
         "//pkg/server/serverpb",
+        "//pkg/server/systemconfigwatcher/systemconfigwatchertest",
         "//pkg/sql",
         "//pkg/sql/distsql",
         "//pkg/sql/tests",

pkg/ccl/serverccl/server_sql_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/ccl/utilccl/licenseccl"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security"
+	"github.com/cockroachdb/cockroach/pkg/server/systemconfigwatcher/systemconfigwatchertest"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/distsql"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
@@ -250,3 +251,8 @@ func TestNoInflightTracesVirtualTableOnTenant(t *testing.T) {
 	require.Error(t, err, "cluster_inflight_traces should be unsupported")
 	require.Contains(t, err.Error(), "table crdb_internal.cluster_inflight_traces is not implemented on tenants")
 }
+
+func TestSystemConfigWatcherCache(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	systemconfigwatchertest.TestSystemConfigWatcher(t, false /* skipSecondary */)
+}

pkg/ccl/telemetryccl/testdata/telemetry/multiregion

@@ -384,6 +384,14 @@ IMPORT INTO t7 CSV DATA ('nodelocal://0/t7/export*.csv')
 sql.multiregion.import
 
 # Test for locality optimized search counter.
+
+# Lower the closed timestamp subsystem so system config info is transmitted
+# rapidly.
+exec
+SET CLUSTER SETTING kv.closed_timestamp.side_transport_interval = '10ms';
+SET CLUSTER SETTING kv.closed_timestamp.target_duration = '5ms';
+----
+
 feature-allowlist
 sql.plan.opt.locality-optimized-search
 ----
@@ -393,6 +401,12 @@ USE survive_region;
 CREATE TABLE t8 (a INT PRIMARY KEY) LOCALITY REGIONAL BY ROW
 ----
 
+# Sleep a large multiple of the closed timestamp target duration to ensure
+# that a fresh system config has made its way to the optimizer.
+exec
+SELECT pg_sleep(.05);
+----
+
 feature-usage
 SELECT * FROM t8 WHERE a = 1
 ----

pkg/cli/clisqlshell/sql_test.go

@@ -115,7 +115,7 @@ func Example_sql() {
 	// ERROR: -e: woops! COPY has confused this client! Suggestion: use 'psql' for COPY
 	// sql -e select 1/(@1-2) from generate_series(1,3)
 	// ?column?
-	// -1
+	// -1.0000000000000000000
 	// (error encountered after some results were delivered)
 	// ERROR: division by zero
 	// SQLSTATE: 22012
@@ -190,9 +190,9 @@ func Example_sql_watch() {
 	// INSERT 1
 	// sql --watch .1s -e update d set x=x-1 returning 1/x as dec
 	// dec
-	// 0.5
+	// 0.50000000000000000000
 	// dec
-	// 1
+	// 1.0000000000000000000
 	// ERROR: division by zero
 	// SQLSTATE: 22012
 }

pkg/cmd/dev/generate.go

@@ -50,9 +50,10 @@ func makeGenerateCmd(runE func(cmd *cobra.Command, args []string) error) *cobra.
 
 func (d *dev) generate(cmd *cobra.Command, targets []string) error {
 	var generatorTargetMapping = map[string]func(cmd *cobra.Command) error{
-		"bazel": d.generateBazel,
-		"docs":  d.generateDocs,
-		"go":    d.generateGo,
+		"bazel":    d.generateBazel,
+		"docs":     d.generateDocs,
+		"go":       d.generateGo,
+		"protobuf": d.generateProtobuf,
 	}
 
 	if len(targets) == 0 {
@@ -177,3 +178,13 @@ func (d *dev) generateGo(cmd *cobra.Command) error {
 	}
 	return d.hoistGeneratedCode(ctx, workspace, bazelBin)
 }
+
+func (d *dev) generateProtobuf(cmd *cobra.Command) error {
+	// The bazel target //pkg/gen:go_proto builds and hoists the protobuf
+	// go files.
+	return d.exec.CommandContextInheritingStdStreams(
+		cmd.Context(), "bazel", append(append([]string{"run"},
+			mustGetRemoteCacheArgs(remoteCacheAddr)...),
+			"//pkg/gen:go_proto")...,
+	)
+}

pkg/kv/kvclient/rangefeed/config.go

@@ -26,11 +26,17 @@ type Option interface {
 
 type config struct {
 	scanConfig
-	retryOptions         retry.Options
-	onInitialScanDone    OnInitialScanDone
-	withInitialScan      bool
+	retryOptions       retry.Options
+	onInitialScanDone  OnInitialScanDone
+	withInitialScan    bool
+	onInitialScanError OnInitialScanError
+	// useRowTimestampInInitialScan indicates that when rows are scanned in an
+	// initial scan, they should report their timestamp as it exists in KV as
+	// opposed to the timestamp at which the scan occurred. Both behaviors can
+	// be sane depending on your use case.
+	useRowTimestampInInitialScan bool
+
 	withDiff             bool
-	onInitialScanError   OnInitialScanError
 	onUnrecoverableError OnUnrecoverableError
 	onCheckpoint         OnCheckpoint
 	onFrontierAdvance    OnFrontierAdvance
@@ -101,6 +107,17 @@ func WithOnInitialScanError(f OnInitialScanError) Option {
 	})
 }
 
+// WithRowTimestampInInitialScan indicates whether the timestamp of rows
+// reported during an initial scan should correspond to the timestamp of that
+// row as it exists in KV or should correspond to the timestamp of the initial
+// scan. The default is false, indicating that the timestamp should correspond
+// to the timestamp of the initial scan.
+func WithRowTimestampInInitialScan(shouldUse bool) Option {
+	return optionFunc(func(c *config) {
+		c.useRowTimestampInInitialScan = shouldUse
+	})
+}
+
 // WithOnInternalError sets up a callback to report unrecoverable errors during
 // operation.
 func WithOnInternalError(f OnUnrecoverableError) Option {
@@ -109,11 +126,12 @@ func WithOnInternalError(f OnUnrecoverableError) Option {
 	})
 }
 
-// WithDiff makes an option to enable an initial scan which defaults to
-// false.
-func WithDiff() Option {
+// WithDiff makes an option to set whether rangefeed events carry the previous
+// value in addition to the new value. The option defaults to false. If set,
+// initial scan events will carry the same value for both Value and PrevValue.
+func WithDiff(withDiff bool) Option {
 	return optionFunc(func(c *config) {
-		c.withDiff = true
+		c.withDiff = withDiff
 	})
 }
 

pkg/kv/kvclient/rangefeed/rangefeed_external_test.go

@@ -83,7 +83,7 @@ func TestRangeFeedIntegration(t *testing.T) {
 			case <-ctx.Done():
 			}
 		},
-		rangefeed.WithDiff(),
+		rangefeed.WithDiff(true),
 		rangefeed.WithInitialScan(func(ctx context.Context) {
 			close(initialScanDone)
 		}),
@@ -387,7 +387,7 @@ func TestRangefeedValueTimestamps(t *testing.T) {
 			case <-ctx.Done():
 			}
 		},
-		rangefeed.WithDiff(),
+		rangefeed.WithDiff(true),
 	)
 	require.NoError(t, err)
 	defer r.Close()
@@ -669,7 +669,7 @@ func TestUnrecoverableErrors(t *testing.T) {
 
 	r, err := f.RangeFeed(ctx, "test", []roachpb.Span{sp}, preGCThresholdTS,
 		func(context.Context, *roachpb.RangeFeedValue) {},
-		rangefeed.WithDiff(),
+		rangefeed.WithDiff(true),
 		rangefeed.WithOnInternalError(func(ctx context.Context, err error) {
 			mu.Lock()
 			defer mu.Unlock()

pkg/kv/kvclient/rangefeed/rangefeed_mock_test.go

@@ -281,7 +281,7 @@ func TestRangeFeedMock(t *testing.T) {
 			ctx context.Context, value *roachpb.RangeFeedValue,
 		) {
 			rows <- value
-		}, rangefeed.WithDiff())
+		}, rangefeed.WithDiff(true))
 		require.NoError(t, err)
 		<-rows
 		r.Close()

pkg/kv/kvclient/rangefeed/rangefeedbuffer/BUILD.bazel

@@ -2,10 +2,14 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "rangefeedbuffer",
-    srcs = ["buffer.go"],
+    srcs = [
+        "buffer.go",
+        "kvs.go",
+    ],
     importpath = "github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangefeed/rangefeedbuffer",
     visibility = ["//visibility:public"],
     deps = [
+        "//pkg/roachpb:with-mocks",
         "//pkg/util/hlc",
         "//pkg/util/log",
         "//pkg/util/syncutil",
@@ -15,9 +19,15 @@ go_library(
 
 go_test(
     name = "rangefeedbuffer_test",
-    srcs = ["buffer_test.go"],
+    srcs = [
+        "buffer_test.go",
+        "kvs_test.go",
+    ],
+    embed = [":rangefeedbuffer"],
     deps = [
-        ":rangefeedbuffer",
+        "//pkg/keys",
+        "//pkg/roachpb:with-mocks",
+        "//pkg/util/encoding",
         "//pkg/util/hlc",
         "//pkg/util/leaktest",
         "@com_github_stretchr_testify//require",