Merge #105582 #106944 #107086

105582: opt: check that generator functions are not used in CASE or COALESCE r=DrewKimball a=DrewKimball

#### opt: check that generator functions are not used in CASE or COALESCE

This patch adds checks during type-checking to ensure that generator functions
are not used in the arguments of `CASE`, `IF`, `COALESCE`, or `IFNULL`
expressions. This mirrors postgres behavior. This patch also corrects the error
message that is returned in these cases to say "set-returning" instead of
"generator".

Fixes #97119
Fixes #94890

Release note (bug fix): CASE, IF, COALESCE, and IFNULL expressions now return
an error when passed a generator function as an argument. This mirrors postgres
behavior.

106944: bazel,dev: in `dev`, handle different `--compilation_mode`s correctly... r=rail a=rickystewart

... and switch our default compilation mode to `dbg`.

Under `fastbuild`, built binaries are stripped. This is not the case with `dbg`. Have `dev doctor` recommend using `dbg`, and either way make `dev` resilient to whatever kind of `--compilation_mode` you're using.

In principle or theory `dbg` is slower than `fastbuild`, but for the Go compiler it generates debuggable binaries by default and you have to opt-in to stripping, so it shoould make no real difference.

Now, we think of the `--compilation_mode` simply as follows: `dbg` is the default that everyone is opted into by default unless they explicitly set `-c opt` (we use this for release). For now I don't see a reason anyone would need `fastbuild`.

Epic: CRDB-17171
Release note: None
Closes: #106820

107086: server: minor improvement around TestTenantInterface r=yuzefovich a=yuzefovich

Addresses: #76378.
Fixes: #106903.


Co-authored-by: Drew Kimball <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>

.bazelrc

@@ -36,6 +36,7 @@ build --define gotags=bazel,gss
 build --experimental_proto_descriptor_sets_include_source_info
 build --incompatible_strict_action_env --incompatible_enable_cc_toolchain_resolution
 build --symlink_prefix=_bazel/
+build -c dbg
 common --experimental_allow_tags_propagation
 test --config=test --experimental_ui_max_stdouterr_bytes=10485760
 build --ui_event_filters=-DEBUG

build/teamcity/cockroach/ci/builds/build_impl.sh

@@ -38,7 +38,7 @@ fi
 
 bazel build //pkg/cmd/bazci --config=ci
 BAZEL_BIN=$(bazel info bazel-bin --config=ci)
-"$BAZEL_BIN/pkg/cmd/bazci/bazci_/bazci" -- build -c opt \
+"$BAZEL_BIN/pkg/cmd/bazci/bazci_/bazci" -- build \
 		       --config "$CONFIG" --config ci $EXTRA_ARGS \
 		       //pkg/cmd/cockroach-short //pkg/cmd/cockroach \
 		       //pkg/cmd/cockroach-sql \

build/teamcity/cockroach/nightlies/pebble_nightly_common.sh

@@ -27,8 +27,8 @@ mkdir -p "$PWD/bin"
 chmod o+rwx "$PWD/bin"
 
 # Build the roachtest binary.
-bazel build //pkg/cmd/roachtest --config ci -c opt
-BAZEL_BIN=$(bazel info bazel-bin --config ci -c opt)
+bazel build //pkg/cmd/roachtest --config ci
+BAZEL_BIN=$(bazel info bazel-bin --config ci)
 cp $BAZEL_BIN/pkg/cmd/roachtest/roachtest_/roachtest bin
 chmod a+w bin/roachtest
 
@@ -39,8 +39,8 @@ chmod a+w bin/roachtest
 bazel run @go_sdk//:bin/go get github.com/cockroachdb/pebble@latest
 NEW_DEPS_BZL_CONTENT=$(bazel run //pkg/cmd/mirror/go:mirror)
 echo "$NEW_DEPS_BZL_CONTENT" > DEPS.bzl
-bazel build @com_github_cockroachdb_pebble//cmd/pebble --config ci -c opt
-BAZEL_BIN=$(bazel info bazel-bin --config ci -c opt)
+bazel build @com_github_cockroachdb_pebble//cmd/pebble --config ci
+BAZEL_BIN=$(bazel info bazel-bin --config ci)
 cp $BAZEL_BIN/external/com_github_cockroachdb_pebble/cmd/pebble/pebble_/pebble ./pebble.linux
 chmod a+w ./pebble.linux
 
@@ -67,8 +67,8 @@ function prepare_datadir() {
 # Build the mkbench tool from within the Pebble repo. This is used to parse
 # the benchmark data.
 function build_mkbench() {
-  bazel build @com_github_cockroachdb_pebble//internal/mkbench --config ci -c opt
-  BAZEL_BIN=$(bazel info bazel-bin --config ci -c opt)
+  bazel build @com_github_cockroachdb_pebble//internal/mkbench --config ci
+  BAZEL_BIN=$(bazel info bazel-bin --config ci)
   cp $BAZEL_BIN/external/com_github_cockroachdb_pebble/internal/mkbench/mkbench_/mkbench .
   chmod a+w mkbench
 }

build/teamcity/cockroach/nightlies/pebble_nightly_race_common.sh

@@ -27,8 +27,8 @@ mkdir -p "$PWD/bin"
 chmod o+rwx "$PWD/bin"
 
 # Build the roachtest binary.
-bazel build //pkg/cmd/roachtest --config ci -c opt
-BAZEL_BIN=$(bazel info bazel-bin --config ci -c opt)
+bazel build //pkg/cmd/roachtest --config ci
+BAZEL_BIN=$(bazel info bazel-bin --config ci)
 cp $BAZEL_BIN/pkg/cmd/roachtest/roachtest_/roachtest bin
 chmod a+w bin/roachtest
 
@@ -39,8 +39,8 @@ chmod a+w bin/roachtest
 bazel run @go_sdk//:bin/go get github.com/cockroachdb/pebble@latest
 NEW_DEPS_BZL_CONTENT=$(bazel run //pkg/cmd/mirror/go: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)
+bazel build @com_github_cockroachdb_pebble//cmd/pebble --config race --config ci
+BAZEL_BIN=$(bazel info bazel-bin --config race --config ci)
 cp $BAZEL_BIN/external/com_github_cockroachdb_pebble/cmd/pebble/pebble_/pebble ./pebble.linux
 chmod a+w ./pebble.linux
 

build/teamcity/cockroach/nightlies/roachtest_compile_bits.sh

@@ -35,21 +35,21 @@ for platform in "${cross_builds[@]}"; do
 
   echo "Building $platform, os=$os, arch=$arch..."
   # Build cockroach, workload and geos libs.
-  bazel build --config $platform --config ci -c opt --config force_build_cdeps \
+  bazel build --config $platform --config ci --config force_build_cdeps \
         //pkg/cmd/cockroach //pkg/cmd/workload \
         //c-deps:libgeos
-  BAZEL_BIN=$(bazel info bazel-bin --config $platform --config ci -c opt)
+  BAZEL_BIN=$(bazel info bazel-bin --config $platform --config ci)
 
   # N.B. roachtest is built once, for the host architecture.
   if [[ $os == "linux" && $arch == $host_arch ]]; then
-    bazel build --config $platform --config ci  -c opt //pkg/cmd/roachtest
+    bazel build --config $platform --config ci //pkg/cmd/roachtest
 
     cp $BAZEL_BIN/pkg/cmd/roachtest/roachtest_/roachtest bin/roachtest
     # Make it writable to simplify cleanup and copying (e.g., scp retry).
     chmod a+w bin/roachtest
   fi
   # Build cockroach-short with assertions enabled.
-  bazel build --config $platform --config ci -c opt //pkg/cmd/cockroach-short --crdb_test
+  bazel build --config $platform --config ci //pkg/cmd/cockroach-short --crdb_test
   # Copy the binaries.
   cp $BAZEL_BIN/pkg/cmd/cockroach/cockroach_/cockroach bin/cockroach.$os-$arch
   cp $BAZEL_BIN/pkg/cmd/workload/workload_/workload    bin/workload.$os-$arch

build/toolchains/BUILD.bazel

@@ -404,13 +404,6 @@ config_setting(
     },
 )
 
-config_setting(
-    name = "opt",
-    values = {
-        "compilation_mode": "opt",
-    },
-)
-
 bool_flag(
     name = "nogo_flag",
     build_setting_default = False,

dev

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

pkg/ccl/serverccl/role_authentication_test.go

@@ -33,12 +33,7 @@ func TestVerifyPassword(t *testing.T) {
 	defer log.Scope(t).Close(t)
 
 	ctx := context.Background()
-	s, db, _ := serverutils.StartServer(t,
-		base.TestServerArgs{
-			// One of the sub-tests fails.
-			DefaultTestTenant: base.TestDoesNotWorkWithSecondaryTenantsButWeDontKnowWhyYet(106903),
-		},
-	)
+	s, db, _ := serverutils.StartServer(t, base.TestServerArgs{})
 	defer s.Stopper().Stop(ctx)
 
 	ts := s.TenantOrServer()
@@ -145,7 +140,7 @@ func TestVerifyPassword(t *testing.T) {
 		{"user with VALID UNTIL NULL should succeed", "cthon98", "12345", false, false, true, true},
 	} {
 		t.Run(tc.testName, func(t *testing.T) {
-			execCfg := s.ExecutorConfig().(sql.ExecutorConfig)
+			execCfg := ts.ExecutorConfig().(sql.ExecutorConfig)
 			username := username.MakeSQLUsernameFromPreNormalizedString(tc.username)
 			exists, canLoginSQL, canLoginDBConsole, canUseReplicationMode, isSuperuser, _, pwRetrieveFn, err := sql.GetUserSessionInitInfo(
 				context.Background(), &execCfg, username, "", /* databaseName */

pkg/cmd/dev/build.go

@@ -170,6 +170,8 @@ func (d *dev) build(cmd *cobra.Command, commandLine []string) error {
 		return err
 	}
 	args = append(args, additionalBazelArgs...)
+	configArgs := getConfigArgs(args)
+	configArgs = append(configArgs, getConfigArgs(additionalBazelArgs)...)
 
 	if cross == "" {
 		// Do not log --build_event_binary_file=... because it is not relevant to the actual call
@@ -183,7 +185,7 @@ func (d *dev) build(cmd *cobra.Command, commandLine []string) error {
 		if err := d.exec.CommandContextInheritingStdStreams(ctx, "bazel", args...); err != nil {
 			return err
 		}
-		return d.stageArtifacts(ctx, buildTargets)
+		return d.stageArtifacts(ctx, buildTargets, configArgs)
 	}
 	volume := mustGetFlagString(cmd, volumeFlag)
 	cross = "cross" + cross
@@ -252,7 +254,9 @@ func (d *dev) crossBuild(
 	return err
 }
 
-func (d *dev) stageArtifacts(ctx context.Context, targets []buildTarget) error {
+func (d *dev) stageArtifacts(
+	ctx context.Context, targets []buildTarget, configArgs []string,
+) error {
 	workspace, err := d.getWorkspace(ctx)
 	if err != nil {
 		return err
@@ -261,7 +265,7 @@ func (d *dev) stageArtifacts(ctx context.Context, targets []buildTarget) error {
 	if err = d.os.MkdirAll(path.Join(workspace, "bin")); err != nil {
 		return err
 	}
-	bazelBin, err := d.getBazelBin(ctx)
+	bazelBin, err := d.getBazelBin(ctx, configArgs)
 	if err != nil {
 		return err
 	}
@@ -453,11 +457,19 @@ func (d *dev) getBasicBuildArgs(
 	return args, buildTargets, nil
 }
 
-// Given a list of Bazel arguments, find the ones starting with --config= and
-// return them.
+// Given a list of Bazel arguments, find the ones that represent a "config"
+// (either --config or -c) and return all of these. This is used to find
+// the appropriate bazel-bin for any invocation.
 func getConfigArgs(args []string) (ret []string) {
+	var addNext bool
 	for _, arg := range args {
-		if strings.HasPrefix(arg, "--config=") {
+		if addNext {
+			ret = append(ret, arg)
+			addNext = false
+		} else if arg == "--config" || arg == "--compilation_mode" || arg == "-c" {
+			ret = append(ret, arg)
+			addNext = true
+		} else if strings.HasPrefix(arg, "--config=") || strings.HasPrefix(arg, "--compilation_mode=") {
 			ret = append(ret, arg)
 		}
 	}

pkg/cmd/dev/doctor.go

@@ -503,7 +503,7 @@ func (d *dev) doctor(cmd *cobra.Command, _ []string) error {
 	if err != nil {
 		return err
 	}
-	bazelBin, err := d.getBazelBin(ctx)
+	bazelBin, err := d.getBazelBin(ctx, []string{})
 	if err != nil {
 		return err
 	}

pkg/cmd/dev/generate.go

@@ -315,7 +315,7 @@ func (d *dev) generateJs(cmd *cobra.Command) error {
 		return fmt.Errorf("building JS development prerequisites: %w", err)
 	}
 
-	bazelBin, err := d.getBazelBin(ctx)
+	bazelBin, err := d.getBazelBin(ctx, []string{})
 	if err != nil {
 		return err
 	}

pkg/cmd/dev/roachprod_stress.go

@@ -120,7 +120,7 @@ func (d *dev) roachprodStress(cmd *cobra.Command, commandLine []string) error {
 	if _, err := d.exec.CommandContextSilent(ctx, "bazel", args...); err != nil {
 		return err
 	}
-	if err := d.stageArtifacts(ctx, roachprodStressTarget); err != nil {
+	if err := d.stageArtifacts(ctx, roachprodStressTarget, []string{}); err != nil {
 		return err
 	}
 

pkg/cmd/dev/ui.go

@@ -463,7 +463,7 @@ func makeUICleanCmd(d *dev) *cobra.Command {
 //
 // See https://github.com/bazelbuild/rules_nodejs/issues/2028
 func arrangeFilesForWatchers(d *dev, ossOnly bool) error {
-	bazelBin, err := d.getBazelBin(d.cli.Context())
+	bazelBin, err := d.getBazelBin(d.cli.Context(), []string{})
 	if err != nil {
 		return err
 	}

pkg/cmd/dev/util.go

@@ -102,7 +102,7 @@ func mustGetFlagDuration(cmd *cobra.Command, name string) time.Duration {
 	return val
 }
 
-func (d *dev) getBazelInfo(ctx context.Context, key string) (string, error) {
+func (d *dev) getBazelInfo(ctx context.Context, key string, extraArgs []string) (string, error) {
 	args := []string{"info", key, "--color=no"}
 	out, err := d.exec.CommandContextSilent(ctx, "bazel", args...)
 	if err != nil {
@@ -117,15 +117,17 @@ func (d *dev) getWorkspace(ctx context.Context) (string, error) {
 		return os.Getwd()
 	}
 
-	return d.getBazelInfo(ctx, "workspace")
+	return d.getBazelInfo(ctx, "workspace", []string{})
 }
 
-func (d *dev) getBazelBin(ctx context.Context) (string, error) {
-	return d.getBazelInfo(ctx, "bazel-bin")
+// The second argument should be the relevant "config args", namely Bazel arguments
+// that are --config or --compilation_mode arguments (see getConfigArgs()).
+func (d *dev) getBazelBin(ctx context.Context, configArgs []string) (string, error) {
+	return d.getBazelInfo(ctx, "bazel-bin", configArgs)
 }
 
 func (d *dev) getExecutionRoot(ctx context.Context) (string, error) {
-	return d.getBazelInfo(ctx, "execution_root")
+	return d.getBazelInfo(ctx, "execution_root", []string{})
 }
 
 // getDevBin returns the path to the running dev executable.

pkg/cmd/roachtest/tests/follower_reads.go

@@ -530,9 +530,11 @@ func initFollowerReadsDB(
 			// parsing the replica_localities array using the same pattern as the
 			// one used by SHOW REGIONS.
 			const q2 = `
-			SELECT
-				count(distinct substring(unnest(replica_localities), 'region=([^,]*)'))
-			FROM [SHOW RANGES FROM TABLE test.test]`
+			SELECT count(DISTINCT substring(unnested, 'region=([^,]*)'))
+			FROM (
+				SELECT unnest(replica_localities) AS unnested
+				FROM [SHOW RANGES FROM TABLE test.test]
+			)`
 
 			var distinctRegions int
 			require.NoError(t, db.QueryRowContext(ctx, q2).Scan(&distinctRegions))