Merge #77864 #78586 #79048 #79055 #79068

77864: roachtest: add pebble ycsb/A race build roachtest r=bananabrick a=bananabrick

Release note: None

78586: sql: regression test for MVCC timestamp in materialized views r=dt a=rafiss

refs #62932

Before abeaf01 this test would have
failed, so this test will help make sure we don't regress.

Release note: None

79048: sql: Node user always passes CheckAnyPrivilege r=rafiss a=RichardJCai

No release note since this only affects what is shown
in InternalExecutor queries.

Release note: None

79055: sql: use qualifiable schema name for comment on schema r=chengxiong-ruan a=chengxiong-ruan

Previously we only use current db to resolve a schema when comment
on a schema. This is painful at least for our testing sometimes
because we need to switch db before commenting on a schema.

Release justification: low impact but can be useful for users.
Release note (sql change): `COMMENT ON SCHEMA` now can use qualified
schema name. So can do both `COMMENT ON SCHEMA sc_name ...` and
`COMMENT ON SCHEMA db_name.sc_name ...`.

79068: dev: copy built binaries rather than symlinking them r=mari-crl a=rickystewart

This uses more disk space but is resilient to the confusing case
where your Bazel `output_base` gets deleted (either due to
`bazel clean --expunge` or if your `output_base` gets wiped due to a
macOS upgrade).

Release note: None

Co-authored-by: Arjun Nair <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: richardjcai <[email protected]>
Co-authored-by: Chengxiong Ruan <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>

build/teamcity/cockroach/nightlies/pebble_nightly_race_common.sh

@@ -0,0 +1,53 @@
+#!/usr/bin/env bash
+#
+# This script contains common configuration used by the Pebble Nightly race runs.
+
+set -euo pipefail
+
+if [[ "$GOOGLE_EPHEMERAL_CREDENTIALS" ]]; then
+  echo "$GOOGLE_EPHEMERAL_CREDENTIALS" > creds.json
+  gcloud auth activate-service-account --key-file=creds.json
+  export ROACHPROD_USER=teamcity
+else
+  echo 'warning: GOOGLE_EPHEMERAL_CREDENTIALS not set' >&2
+  echo "Assuming that you've run \`gcloud auth login\` from inside the builder." >&2
+fi
+
+set -ux
+
+if [[ ! -f ~/.ssh/id_rsa.pub ]]; then
+  ssh-keygen -q -N "" -f ~/.ssh/id_rsa
+fi
+
+# The artifacts dir should match up with that supplied by TC.
+artifacts=$PWD/artifacts
+mkdir -p "${artifacts}"
+chmod o+rwx "${artifacts}"
+mkdir -p "$PWD/bin"
+chmod o+rwx "$PWD/bin"
+
+build_tag=$(git describe --abbrev=0 --tags --match=v[0-9]*)
+export build_tag
+
+# Build the roachtest binary.
+bazel build //pkg/cmd/roachtest --config ci -c opt
+BAZEL_BIN=$(bazel info bazel-bin --config ci -c opt)
+cp $BAZEL_BIN/pkg/cmd/roachtest/roachtest_/roachtest bin
+chmod a+w bin/roachtest
+
+# Pull in the latest version of Pebble from upstream. The benchmarks run
+# against the tip of the 'master' branch. We do this by `go get`ting the
+# latest version of the module, and then running `mirror` to update `DEPS.bzl`
+# accordingly.
+bazel run @go_sdk//:bin/go get github.com/cockroachdb/pebble@latest
+NEW_DEPS_BZL_CONTENT=$(bazel run //pkg/cmd/mirror)
+echo "$NEW_DEPS_BZL_CONTENT" > DEPS.bzl
+bazel build @com_github_cockroachdb_pebble//cmd/pebble --config race --config ci -c opt
+BAZEL_BIN=$(bazel info bazel-bin --config race --config ci -c opt)
+cp $BAZEL_BIN/external/com_github_cockroachdb_pebble/cmd/pebble/pebble_/pebble ./pebble.linux
+chmod a+w ./pebble.linux
+
+# Set the location of the pebble binary. This is referenced by the roachtests,
+# which will push this binary out to all workers in order to run the
+# benchmarks.
+export PEBBLE_BIN=pebble.linux

build/teamcity/cockroach/nightlies/pebble_nightly_ycsb_race.sh

@@ -0,0 +1,18 @@
+#!/usr/bin/env bash
+#
+# This script runs the Pebble Nightly YCSB A benchmark with the race flag.
+# It is used to detect data races which may have been introduced to the latest
+# Pebble master branch.
+#
+# It is run by the Pebble Nightly YCSB A race TeamCity build
+# configuration.
+
+set -euo pipefail
+
+dir="$(dirname $(dirname $(dirname $(dirname "${0}"))))"
+
+source "$dir/teamcity-support.sh"  # For $root
+source "$dir/teamcity-bazel-support.sh"  # For run_bazel
+
+BAZEL_SUPPORT_EXTRA_DOCKER_ARGS="-e LITERAL_ARTIFACTS_DIR=$root/artifacts -e AWS_ACCESS_KEY_ID -e AWS_SECRET_ACCESS_KEY -e GOOGLE_EPHEMERAL_CREDENTIALS -e TC_BUILD_BRANCH -e TC_BUILD_ID -e TC_SERVER_URL" \
+                               run_bazel build/teamcity/cockroach/nightlies/pebble_nightly_ycsb_race_impl.sh

build/teamcity/cockroach/nightlies/pebble_nightly_ycsb_race_impl.sh

@@ -0,0 +1,33 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+_dir="$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
+
+# Execute the common commands for the benchmark runs.
+. "$_dir/pebble_nightly_race_common.sh"
+
+# Run the YCSB benchmark.
+#
+# NB: We specify "true" for the --cockroach and --workload binaries to
+# prevent roachtest from complaining (and failing) when it can't find
+# them. The pebble roachtests don't actually use either cockroach or
+# workload.
+exit_status=0
+if ! timeout -s INT $((1000*60)) bin/roachtest run \
+  --build-tag "${build_tag}" \
+  --slack-token "${SLACK_TOKEN-}" \
+  --cluster-id "${TC_BUILD_ID-$(date +"%Y%m%d%H%M%S")}" \
+  --cloud "aws" \
+  --cockroach "true" \
+  --workload "true" \
+  --artifacts "$artifacts" \
+  --artifacts-literal="${LITERAL_ARTIFACTS_DIR:-}" \
+  --parallelism 3 \
+  --teamcity \
+  --cpu-quota=384 \
+  pebble tag:pebble_nightly_ycsb_race; then
+  exit_status=$?
+fi
+
+exit "$exit_status"

docs/generated/sql/bnf/comment.bnf

@@ -1,6 +1,6 @@
 comment_stmt ::=
 	'COMMENT' 'ON' 'DATABASE' database_name 'IS' comment_text
-	| 'COMMENT' 'ON' 'SCHEMA' schema_name 'IS' comment_text
+	| 'COMMENT' 'ON' 'SCHEMA' qualifiable_schema_name 'IS' comment_text
 	| 'COMMENT' 'ON' 'TABLE' table_name 'IS' comment_text
 	| 'COMMENT' 'ON' 'COLUMN' column_name 'IS' comment_text
 	| 'COMMENT' 'ON' 'INDEX' table_index_name 'IS' comment_text

docs/generated/sql/bnf/stmt_block.bnf

@@ -58,7 +58,7 @@ copy_from_stmt ::=
 
 comment_stmt ::=
 	'COMMENT' 'ON' 'DATABASE' database_name 'IS' comment_text
-	| 'COMMENT' 'ON' 'SCHEMA' schema_name 'IS' comment_text
+	| 'COMMENT' 'ON' 'SCHEMA' qualifiable_schema_name 'IS' comment_text
 	| 'COMMENT' 'ON' 'TABLE' table_name 'IS' comment_text
 	| 'COMMENT' 'ON' 'COLUMN' column_path 'IS' comment_text
 	| 'COMMENT' 'ON' 'INDEX' table_index_name 'IS' comment_text
@@ -298,8 +298,9 @@ comment_text ::=
 	'SCONST'
 	| 'NULL'
 
-schema_name ::=
+qualifiable_schema_name ::=
 	name
+	| name '.' name
 
 column_path ::=
 	name
@@ -1376,10 +1377,6 @@ privilege ::=
 type_name_list ::=
 	( type_name ) ( ( ',' type_name ) )*
 
-qualifiable_schema_name ::=
-	name
-	| name '.' name
-
 type_list ::=
 	( typename ) ( ( ',' typename ) )*
 
@@ -2024,6 +2021,9 @@ alter_zone_partition_stmt ::=
 	| 'ALTER' 'PARTITION' partition_name 'OF' 'INDEX' table_index_name set_zone_config
 	| 'ALTER' 'PARTITION' partition_name 'OF' 'INDEX' table_name '@' '*' set_zone_config
 
+schema_name ::=
+	name
+
 opt_add_val_placement ::=
 	'BEFORE' 'SCONST'
 	| 'AFTER' 'SCONST'

pkg/cmd/dev/build.go

@@ -225,13 +225,13 @@ func (d *dev) stageArtifacts(ctx context.Context, targets []buildTarget) error {
 		}
 		binaryPath := filepath.Join(bazelBin, bazelutil.OutputOfBinaryRule(target.fullName, runtime.GOOS == "windows"))
 		base := targetToBinBasename(target.fullName)
-		var symlinkPaths []string
+		var copyPaths []string
 		// Binaries beginning with the string "cockroach" go right at
 		// the top of the workspace; others go in the `bin` directory.
 		if strings.HasPrefix(base, "cockroach") {
-			symlinkPaths = append(symlinkPaths, filepath.Join(workspace, base))
+			copyPaths = append(copyPaths, filepath.Join(workspace, base))
 			if strings.HasPrefix(base, "cockroach-short") {
-				symlinkPaths = append(symlinkPaths, filepath.Join(workspace, "cockroach"))
+				copyPaths = append(copyPaths, filepath.Join(workspace, "cockroach"))
 			}
 		} else if base == "dev" {
 			buf, err := d.os.ReadFile(filepath.Join(workspace, "dev"))
@@ -248,23 +248,23 @@ func (d *dev) stageArtifacts(ctx context.Context, targets []buildTarget) error {
 				return errors.New("could not find DEV_VERSION in top-level `dev` script")
 			}
 
-			symlinkPaths = append(symlinkPaths,
+			copyPaths = append(copyPaths,
 				filepath.Join(workspace, "bin", "dev-versions", fmt.Sprintf("dev.%s", devVersion)))
 		} else {
-			symlinkPaths = append(symlinkPaths, filepath.Join(workspace, "bin", base))
+			copyPaths = append(copyPaths, filepath.Join(workspace, "bin", base))
 		}
 
-		// Symlink from binaryPath -> symlinkPath, clear out detritus, if any.
-		for _, symlinkPath := range symlinkPaths {
-			if err := d.os.Remove(symlinkPath); err != nil && !os.IsNotExist(err) {
+		// Copy from binaryPath -> copyPath, clear out detritus, if any.
+		for _, copyPath := range copyPaths {
+			if err := d.os.Remove(copyPath); err != nil && !os.IsNotExist(err) {
 				return err
 			}
-			if err := d.os.Symlink(binaryPath, symlinkPath); err != nil {
+			if err := d.os.CopyFile(binaryPath, copyPath); err != nil {
 				return err
 			}
-			rel, err := filepath.Rel(workspace, symlinkPath)
+			rel, err := filepath.Rel(workspace, copyPath)
 			if err != nil {
-				rel = symlinkPath
+				rel = copyPath
 			}
 			log.Printf("Successfully built binary for target %s at %s", target.fullName, rel)
 		}

pkg/cmd/dev/io/os/os.go

@@ -155,20 +155,6 @@ func (o *OS) RemoveAll(path string) error {
 	return err
 }
 
-// Symlink wraps around os.Symlink, creating a symbolic link to and from the
-// named paths.
-func (o *OS) Symlink(to, from string) error {
-	command := fmt.Sprintf("ln -s %s %s", to, from)
-	if !o.knobs.silent {
-		o.logger.Print(command)
-	}
-
-	_, err := o.Next(command, func() (output string, err error) {
-		return "", os.Symlink(to, from)
-	})
-	return err
-}
-
 // Getenv wraps around os.Getenv, retrieving the value of the environment
 // variable named by the key.
 func (o OS) Getenv(key string) string {
@@ -280,6 +266,8 @@ func (o *OS) WriteFile(filename, contents string) error {
 // implementation would wipe `dst` (and `src` accordingly).
 // Unlike a simple io.Copy, this function checks for that case and is a
 // no-op if `src` is already a symlink to `dst`.
+// The destination file will be readable and writable by everyone and will be
+// executable if the source file is as well.
 func (o *OS) CopyFile(src, dst string) error {
 	command := fmt.Sprintf("cp %s %s", src, dst)
 	if !o.knobs.silent {
@@ -292,15 +280,15 @@ func (o *OS) CopyFile(src, dst string) error {
 			return "", err
 		}
 		defer func() { _ = srcFile.Close() }()
+		srcInfo, err := srcFile.Stat()
+		if err != nil {
+			return "", err
+		}
 		originalDstFile, err := os.Open(dst)
 		if err != nil && !os.IsNotExist(err) {
 			return "", err
 		} else if err == nil {
 			defer func() { _ = originalDstFile.Close() }()
-			srcInfo, err := srcFile.Stat()
-			if err != nil {
-				return "", err
-			}
 			dstInfo, err := originalDstFile.Stat()
 			if err != nil {
 				return "", err
@@ -311,7 +299,12 @@ func (o *OS) CopyFile(src, dst string) error {
 				return "", nil
 			}
 		}
-		dstFile, err := os.Create(dst)
+		isExecutable := srcInfo.Mode().Perm()&0111 != 0
+		dstPerm := fs.FileMode(0666)
+		if isExecutable {
+			dstPerm = fs.FileMode(0777)
+		}
+		dstFile, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, dstPerm)
 		if err != nil {
 			return "", err
 		}

pkg/cmd/dev/testdata/datadriven/dev-build

@@ -6,9 +6,9 @@ bazel info workspace --color=no
 mkdir crdb-checkout/bin
 bazel info bazel-bin --color=no
 rm crdb-checkout/cockroach-short
-ln -s sandbox/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short crdb-checkout/cockroach-short
+cp sandbox/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short crdb-checkout/cockroach-short
 rm crdb-checkout/cockroach
-ln -s sandbox/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short crdb-checkout/cockroach
+cp sandbox/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short crdb-checkout/cockroach
 
 exec
 dev build cockroach-short --cpus=12
@@ -18,9 +18,9 @@ bazel info workspace --color=no
 mkdir crdb-checkout/bin
 bazel info bazel-bin --color=no
 rm crdb-checkout/cockroach-short
-ln -s sandbox/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short crdb-checkout/cockroach-short
+cp sandbox/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short crdb-checkout/cockroach-short
 rm crdb-checkout/cockroach
-ln -s sandbox/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short crdb-checkout/cockroach
+cp sandbox/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short crdb-checkout/cockroach
 
 exec
 dev build --debug short
@@ -30,9 +30,9 @@ bazel info workspace --color=no
 mkdir crdb-checkout/bin
 bazel info bazel-bin --color=no
 rm crdb-checkout/cockroach-short
-ln -s sandbox/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short crdb-checkout/cockroach-short
+cp sandbox/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short crdb-checkout/cockroach-short
 rm crdb-checkout/cockroach
-ln -s sandbox/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short crdb-checkout/cockroach
+cp sandbox/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short crdb-checkout/cockroach
 
 exec
 dev build short -- -s
@@ -42,9 +42,9 @@ bazel info workspace --color=no
 mkdir crdb-checkout/bin
 bazel info bazel-bin --color=no
 rm crdb-checkout/cockroach-short
-ln -s sandbox/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short crdb-checkout/cockroach-short
+cp sandbox/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short crdb-checkout/cockroach-short
 rm crdb-checkout/cockroach
-ln -s sandbox/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short crdb-checkout/cockroach
+cp sandbox/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short crdb-checkout/cockroach
 
 exec
 dev build -- --verbose_failures --sandbox_debug
@@ -55,7 +55,7 @@ bazel info workspace --color=no
 mkdir crdb-checkout/bin
 bazel info bazel-bin --color=no
 rm crdb-checkout/cockroach
-ln -s sandbox/pkg/cmd/cockroach/cockroach_/cockroach crdb-checkout/cockroach
+cp sandbox/pkg/cmd/cockroach/cockroach_/cockroach crdb-checkout/cockroach
 
 exec
 dev build stress
@@ -65,4 +65,4 @@ bazel info workspace --color=no
 mkdir crdb-checkout/bin
 bazel info bazel-bin --color=no
 rm crdb-checkout/bin/stress
-ln -s sandbox/external/com_github_cockroachdb_stress/stress_/stress crdb-checkout/bin/stress
+cp sandbox/external/com_github_cockroachdb_stress/stress_/stress crdb-checkout/bin/stress

pkg/cmd/roachtest/tests/pebble_ycsb.go

@@ -57,18 +57,39 @@ func registerPebbleYCSB(r registry.Registry) {
 				Cluster: r.MakeClusterSpec(5, spec.CPU(16)),
 				Tags:    []string{tag},
 				Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
-					runPebbleYCSB(ctx, t, c, size, pebble, d)
+					runPebbleYCSB(ctx, t, c, size, pebble, d, nil)
 				},
 			})
 		}
 	}
+
+	// Add the race build.
+	r.Add(registry.TestSpec{
+		Name:    "pebble/ycsb/A/race/duration=30",
+		Owner:   registry.OwnerStorage,
+		Timeout: 12 * time.Hour,
+		Cluster: r.MakeClusterSpec(5, spec.CPU(16)),
+		Tags:    []string{"pebble_nightly_ycsb_race"},
+		Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
+			runPebbleYCSB(ctx, t, c, 64, pebble, 30, []string{"A"})
+		},
+	})
 }
 
 // runPebbleYCSB runs the Pebble YCSB benchmarks.
 func runPebbleYCSB(
-	ctx context.Context, t test.Test, c cluster.Cluster, size int, bin string, dur int64,
+	ctx context.Context,
+	t test.Test,
+	c cluster.Cluster,
+	size int,
+	bin string,
+	dur int64,
+	workloads []string,
 ) {
 	c.Put(ctx, bin, "./pebble")
+	if workloads == nil {
+		workloads = []string{"A", "B", "C", "D", "E", "F"}
+	}
 
 	const initialKeys = 10_000_000
 	const cache = 4 << 30 // 4 GB
@@ -93,7 +114,7 @@ func runPebbleYCSB(
 			"rm -f %s && tar cvPf %s %s) > init.log 2>&1",
 		benchDir, size, initialKeys, cache, dataTar, dataTar, benchDir))
 
-	for _, workload := range []string{"A", "B", "C", "D", "E", "F"} {
+	for _, workload := range workloads {
 		keys := "zipf"
 		switch workload {
 		case "D":

pkg/sql/BUILD.bazel

@@ -467,6 +467,7 @@ go_test(
         "alter_column_type_test.go",
         "ambiguous_commit_test.go",
         "as_of_test.go",
+        "authorization_test.go",
         "backfill_num_ranges_in_span_test.go",
         "backfill_test.go",
         "builtin_mem_usage_test.go",