Merge #84399 #85744 #85770 #85858

84399: workload/schemachange: add select support for workload r=fqazi a=fqazi

1. Refactor the op interface to allow us to easily support different types of queries
2. Add a new select operation and make interface changes to help support some level of validation
3. Introduce a new connection watch dog, which ensures that connections make progress
4. Limit the CTAS operations size, since with join we can produce extremely large inserts
5. Skip constraint validation when generated columns cannot be evalauted

85744: sql: version gating CREATE FUNCTION r=chengxiong-ruan a=chengxiong-ruan

This commit adds a version gate for `CREATE FUNCTION` statement.

Release note: None.

85770: externalconn: add DROP privilege for External Connection r=adityamaru a=adityamaru

This diff introduces the DROP privilege that can be granted
to a role/user on a particular External Connection object. External
Connection privileges are non-descriptor backed synthetic privileges
that are uniquely identified by the object's name.

This change requires a user to have the DROP privilege on External
Connection object that they are attempting to DROP.

Release note (sql change): Users can now
GRANT DROP ON EXTERNAL CONNECTION and
REVOKE DROP ON EXTERNAL CONNECTION to grant and revoke the
DROP privilege. This privilege is required by the user to DROP
a particular External Connection.

85858: tools/bazel: enable parallel bazel by default r=dt a=dt

This changes the tools/bazel script to use the first available output base
from those that exist, instead of a hard-coded 1 to 3, allowing users to choose
to add or remove bases to control their desired bazel parallelism. Additionally
it now defaults to initializing two custom bases automatically, so a user who
take no explicit action should see up to three concurrent bazel runs before the
next waits (the first two are giving the custom bases and the third is then just
invoked to use the defaults, as is the fourth which then waits on the third).

To increase bazel parallelism, 'mkdir /private/var/tmp/_bazel_${USER}/bases/$X'
where X is a new base number that does not yet exist. To decrease it, delete of the
bases that exist in that path.

Release note: none.

Co-authored-by: Faizan Qazi <[email protected]>
Co-authored-by: Chengxiong Ruan <[email protected]>
Co-authored-by: Aditya Maru <[email protected]>
Co-authored-by: David Taylor <[email protected]>

docs/generated/settings/settings-for-tenants.txt

@@ -286,4 +286,4 @@ trace.jaeger.agent	string		the address of a Jaeger agent to receive traces using
 trace.opentelemetry.collector	string		address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as <host>:<port>. If no port is specified, 4317 will be used.
 trace.span_registry.enabled	boolean	true	if set, ongoing traces can be seen at https://<ui>/#/debug/tracez
 trace.zipkin.collector	string		the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.
-version	version	22.1-42	set the active cluster version in the format '<major>.<minor>'
+version	version	22.1-44	set the active cluster version in the format '<major>.<minor>'

docs/generated/settings/settings.html

@@ -217,6 +217,6 @@
 <tr><td><code>trace.opentelemetry.collector</code></td><td>string</td><td><code></code></td><td>address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as <host>:<port>. If no port is specified, 4317 will be used.</td></tr>
 <tr><td><code>trace.span_registry.enabled</code></td><td>boolean</td><td><code>true</code></td><td>if set, ongoing traces can be seen at https://<ui>/#/debug/tracez</td></tr>
 <tr><td><code>trace.zipkin.collector</code></td><td>string</td><td><code></code></td><td>the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.</td></tr>
-<tr><td><code>version</code></td><td>version</td><td><code>22.1-42</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
+<tr><td><code>version</code></td><td>version</td><td><code>22.1-44</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
 </tbody>
 </table>

pkg/ccl/cloudccl/externalconn/testdata/create_drop_external_connection

@@ -95,6 +95,51 @@ pq: only users with the EXTERNALCONNECTION system privilege are allowed to CREAT
 
 subtest end
 
+subtest drop-external-storage-privilege
+
+exec-sql
+CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
+----
+
+# Create another External Connection.
+exec-sql
+CREATE EXTERNAL CONNECTION 'privileged-dup' AS 'nodelocal://1/foo'
+----
+
+exec-sql user=testuser
+DROP EXTERNAL CONNECTION privileged
+----
+pq: user testuser does not have DROP privilege on external_connection privileged
+
+inspect-system-table
+----
+privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
+privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
+
+exec-sql
+GRANT DROP ON EXTERNAL CONNECTION privileged TO testuser;
+----
+
+exec-sql user=testuser
+DROP EXTERNAL CONNECTION privileged
+----
+
+# Try to drop the second external connection, testuser should be disallowed.
+exec-sql user=testuser
+DROP EXTERNAL CONNECTION 'privileged-dup'
+----
+pq: user testuser does not have DROP privilege on external_connection privileged-dup
+
+inspect-system-table
+----
+privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
+
+exec-sql
+DROP EXTERNAL CONNECTION 'privileged-dup'
+----
+
+subtest end
+
 subtest basic-gs-kms
 
 exec-sql

pkg/ccl/cloudccl/externalconn/testdata/multi-tenant/create_drop_external_connection

@@ -98,6 +98,51 @@ pq: only users with the EXTERNALCONNECTION system privilege are allowed to CREAT
 
 subtest end
 
+subtest drop-external-storage-privilege
+
+exec-sql
+CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
+----
+
+# Create another External Connection.
+exec-sql
+CREATE EXTERNAL CONNECTION 'privileged-dup' AS 'nodelocal://1/foo'
+----
+
+exec-sql user=testuser
+DROP EXTERNAL CONNECTION privileged
+----
+pq: user testuser does not have DROP privilege on external_connection privileged
+
+inspect-system-table
+----
+privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
+privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
+
+exec-sql
+GRANT DROP ON EXTERNAL CONNECTION privileged TO testuser;
+----
+
+exec-sql user=testuser
+DROP EXTERNAL CONNECTION privileged
+----
+
+# Try to drop the second external connection, testuser should be disallowed.
+exec-sql user=testuser
+DROP EXTERNAL CONNECTION 'privileged-dup'
+----
+pq: user testuser does not have DROP privilege on external_connection privileged-dup
+
+inspect-system-table
+----
+privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
+
+exec-sql
+DROP EXTERNAL CONNECTION 'privileged-dup'
+----
+
+subtest end
+
 subtest basic-gs-kms
 
 exec-sql

pkg/clusterversion/cockroach_versions.go

@@ -340,6 +340,9 @@ const (
 	// SQLSchemaTelemetryScheduledJobs adds an automatic schedule for SQL schema
 	// telemetry logging jobs.
 	SQLSchemaTelemetryScheduledJobs
+	// SchemaChangeSupportsCreateFunction adds support of CREATE FUNCTION
+	// statement.
+	SchemaChangeSupportsCreateFunction
 
 	// *************************************************
 	// Step (1): Add new versions here.
@@ -597,6 +600,10 @@ var versionsSingleton = keyedVersions{
 		Key:     SQLSchemaTelemetryScheduledJobs,
 		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 42},
 	},
+	{
+		Key:     SchemaChangeSupportsCreateFunction,
+		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 44},
+	},
 	// *************************************************
 	// Step (2): Add new versions here.
 	// Do not add new versions to a patch release.

pkg/sql/create_function.go

@@ -15,6 +15,7 @@ import (
 	"fmt"
 	"sort"
 
+	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
@@ -43,6 +44,17 @@ type createFunctionNode struct {
 func (n *createFunctionNode) ReadingOwnWrites() {}
 
 func (n *createFunctionNode) startExec(params runParams) error {
+	if !params.EvalContext().Settings.Version.IsActive(
+		params.ctx,
+		clusterversion.SchemaChangeSupportsCreateFunction,
+	) {
+		// TODO(chengxiong): remove this version gate in 23.1.
+		return pgerror.Newf(
+			pgcode.FeatureNotSupported,
+			"cannot run CREATE FUNCTION before system is fully upgraded to v22.2",
+		)
+	}
+
 	if n.cf.RoutineBody != nil {
 		return unimplemented.NewWithIssue(85144, "CREATE FUNCTION...sql_body unimplemented")
 	}

pkg/sql/create_function_test.go

@@ -16,12 +16,15 @@ import (
 	"testing"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
+	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/kv"
+	"github.com/cockroachdb/cockroach/pkg/server"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descs"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/catid"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
+	"github.com/cockroachdb/cockroach/pkg/sql/tests"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
@@ -182,3 +185,38 @@ $$`,
 	})
 	require.NoError(t, err)
 }
+
+func TestCreateFunctionGating(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+
+	t.Run("new_schema_changer_version_enabled", func(t *testing.T) {
+		params, _ := tests.CreateTestServerParams()
+		// Override binary version to be older.
+		params.Knobs.Server = &server.TestingKnobs{
+			DisableAutomaticVersionUpgrade: make(chan struct{}),
+			BinaryVersionOverride:          clusterversion.ByKey(clusterversion.SchemaChangeSupportsCreateFunction),
+		}
+
+		s, sqlDB, _ := serverutils.StartServer(t, params)
+		defer s.Stopper().Stop(context.Background())
+
+		_, err := sqlDB.Exec(`CREATE FUNCTION f() RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$`)
+		require.NoError(t, err)
+	})
+
+	t.Run("new_schema_changer_version_disabled", func(t *testing.T) {
+		params, _ := tests.CreateTestServerParams()
+		// Override binary version to be older.
+		params.Knobs.Server = &server.TestingKnobs{
+			DisableAutomaticVersionUpgrade: make(chan struct{}),
+			BinaryVersionOverride:          clusterversion.ByKey(clusterversion.SchemaChangeSupportsCreateFunction - 1),
+		}
+
+		s, sqlDB, _ := serverutils.StartServer(t, params)
+		defer s.Stopper().Stop(context.Background())
+
+		_, err := sqlDB.Exec(`CREATE FUNCTION f() RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$`)
+		require.Error(t, err)
+		require.Equal(t, "pq: cannot run CREATE FUNCTION before system is fully upgraded to v22.2", err.Error())
+	})
+}

pkg/sql/drop_external_connection.go

@@ -13,10 +13,14 @@ package sql
 import (
 	"context"
 
+	"github.com/cockroachdb/cockroach/pkg/clusterversion"
+	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
+	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
+	"github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege"
 	"github.com/cockroachdb/errors"
 )
 
@@ -54,16 +58,10 @@ func (p *planner) makeDropExternalConnectionEval(
 }
 
 func (p *planner) dropExternalConnection(params runParams, n *tree.DropExternalConnection) error {
-	// TODO(adityamaru): Check that the user has `DROP` privileges on the External
-	// Connection once we add support for it. Remove admin only check.
-	hasAdmin, err := params.p.HasAdminRole(params.ctx)
-	if err != nil {
-		return err
-	}
-	if !hasAdmin {
-		return pgerror.New(
-			pgcode.InsufficientPrivilege,
-			"only users with the admin role are allowed to DROP EXTERNAL CONNECTION")
+	if !p.ExecCfg().Settings.Version.IsActive(params.ctx, clusterversion.SystemExternalConnectionsTable) {
+		return pgerror.Newf(pgcode.FeatureNotSupported,
+			"External Connections are not supported until upgrade to version %v is finalized",
+			clusterversion.ByKey(clusterversion.SystemExternalConnectionsTable))
 	}
 
 	// TODO(adityamaru): Add some metrics to track DROP EXTERNAL CONNECTION
@@ -79,11 +77,22 @@ func (p *planner) dropExternalConnection(params runParams, n *tree.DropExternalC
 		return errors.Wrap(err, "failed to resolve External Connection name")
 	}
 
+	// Check that the user has DROP privileges on the External Connection object.
+	ecPrivilege := &syntheticprivilege.ExternalConnectionPrivilege{
+		ConnectionName: name,
+	}
+	if err := p.CheckPrivilege(params.ctx, ecPrivilege, privilege.DROP); err != nil {
+		return err
+	}
+
+	// DROP EXTERNAL CONNECTION is only allowed for users with the `DROP`
+	// privilege on this object. We run the query as `node` since the user might
+	// not have `SELECT` on the system table.
 	if _ /* rows */, err = params.extendedEvalCtx.ExecCfg.InternalExecutor.ExecEx(
 		params.ctx,
 		dropExternalConnectionOp,
 		params.p.Txn(),
-		sessiondata.InternalExecutorOverride{User: params.p.User()},
+		sessiondata.InternalExecutorOverride{User: username.NodeUserName()},
 		`DELETE FROM system.external_connections WHERE connection_name = $1`, name,
 	); err != nil {
 		return errors.Wrapf(err, "failed to delete external connection")

pkg/sql/grant_revoke_system.go

@@ -216,6 +216,11 @@ func (n *changeNonDescriptorBackedPrivilegesNode) makeSystemPrivilegeObject(
 		}
 		return ret, nil
 	case privilege.ExternalConnection:
+		if !p.ExecCfg().Settings.Version.IsActive(ctx, clusterversion.SystemExternalConnectionsTable) {
+			return nil, errors.Newf("External Connections are not supported until upgrade to version %s is finalized",
+				clusterversion.SystemExternalConnectionsTable.String())
+		}
+
 		var ret []syntheticprivilege.Object
 		for _, externalConnectionName := range n.targets.ExternalConnections {
 			// Ensure that an External Connection of this name actually exists.

pkg/sql/logictest/testdata/logic_test/external_connection_privileges

@@ -12,54 +12,57 @@ SELECT * FROM system.privileges
 statement error pq: failed to resolve External Connection: external connection with name foo does not exist
 GRANT USAGE ON EXTERNAL CONNECTION foo TO testuser
 
+statement error pq: failed to resolve External Connection: external connection with name foo does not exist
+GRANT DROP ON EXTERNAL CONNECTION foo TO testuser
+
 statement ok
 CREATE EXTERNAL CONNECTION foo AS 'nodelocal://1/foo'
 
 statement ok
-GRANT USAGE ON EXTERNAL CONNECTION foo TO testuser
+GRANT USAGE,DROP ON EXTERNAL CONNECTION foo TO testuser
 
 query TTTT
 SELECT * FROM system.privileges
 ----
-testuser  /externalconn/foo  {USAGE}  {}
+testuser  /externalconn/foo  {DROP,USAGE}  {}
 
 statement ok
-REVOKE USAGE ON EXTERNAL CONNECTION foo FROM testuser
+REVOKE USAGE,DROP ON EXTERNAL CONNECTION foo FROM testuser
 
 query TTTT
 SELECT * FROM system.privileges
 ----
 
 statement ok
-GRANT USAGE ON EXTERNAL CONNECTION foo TO testuser
+GRANT USAGE,DROP ON EXTERNAL CONNECTION foo TO testuser
 
 statement ok
 CREATE USER bar
 
 # Attempt to grant usage as testuser, this should fail since we did not specify WITH GRANT OPTION
 user testuser
 
-statement error pq: user testuser missing WITH GRANT OPTION privilege on USAGE
-GRANT USAGE ON EXTERNAL CONNECTION foo TO bar
+statement error pq: user testuser missing WITH GRANT OPTION privilege on one or more of USAGE, DROP
+GRANT USAGE,DROP ON EXTERNAL CONNECTION foo TO bar
 
 user root
 
 statement ok
-GRANT USAGE ON EXTERNAL CONNECTION foo TO testuser WITH GRANT OPTION
+GRANT USAGE,DROP ON EXTERNAL CONNECTION foo TO testuser WITH GRANT OPTION
 
 # Attempt to grant usage as testuser, this should succeed since we did specify WITH GRANT OPTION
 user testuser
 
 statement ok
-GRANT USAGE ON EXTERNAL CONNECTION foo TO bar
+GRANT USAGE,DROP ON EXTERNAL CONNECTION foo TO bar
 
 user root
 
 query TTTT
 SELECT * FROM system.privileges ORDER BY username
 ----
-bar       /externalconn/foo  {USAGE}  {}
-testuser  /externalconn/foo  {USAGE}  {USAGE}
+bar       /externalconn/foo  {DROP,USAGE}  {}
+testuser  /externalconn/foo  {DROP,USAGE}  {DROP,USAGE}
 
 # Invalid grants on external connections.
 

pkg/sql/privilege/privilege.go

@@ -124,7 +124,7 @@ var (
 	SequencePrivileges           = List{ALL, USAGE, SELECT, UPDATE, CREATE, DROP, INSERT, DELETE, ZONECONFIG}
 	SystemPrivileges             = List{ALL, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, VIEWCLUSTERMETADATA, VIEWDEBUG}
 	VirtualTablePrivileges       = List{ALL, SELECT}
-	ExternalConnectionPrivileges = List{ALL, USAGE}
+	ExternalConnectionPrivileges = List{ALL, USAGE, DROP}
 )
 
 // Mask returns the bitmask for a given privilege.

pkg/workload/schemachange/BUILD.bazel

@@ -11,6 +11,7 @@ go_library(
         "operation_generator.go",
         "schemachange.go",
         "type_resolver.go",
+        "watch_dog.go",
         ":gen-optype-stringer",  # keep
     ],
     importpath = "github.com/cockroachdb/cockroach/pkg/workload/schemachange",
@@ -36,6 +37,7 @@ go_library(
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_jackc_pgconn//:pgconn",
         "@com_github_jackc_pgx_v4//:pgx",
+        "@com_github_jackc_pgx_v4//pgxpool",
         "@com_github_lib_pq//oid",
         "@com_github_spf13_pflag//:pflag",
     ],