Merge #106122 #106203

106122: server: use HTTP query parameter `cluster` for manual selection r=yuzefovich a=knz

Informs #106068.
Epic: CRDB-29380

Prior to this patch, there was a debug-only way to manually force a HTTP request to be routed to a particular virtual cluster through the server controller. This was achieved via the query parameter `tenant_name`.

This patch renames the paramater to `cluster`, for a better UX coherence with the option of the same name in `cockroach sql`.

Release note: None

106203: sql: fix CREATE AS sourcing vtable panics r=chengxiong-ruan a=ecwall

Fixes #106166
Fixes #106167
Fixes #106168
Informs #105895

Fixes panics affects CREATE AS sourcing from vtables. These statements now work properly:
```
CREATE TABLE t AS SELECT * FROM pg_catalog.pg_prepared_statements;
CREATE MATERIALIZED VIEW v AS SELECT * FROM pg_catalog.pg_prepared_statements;
CREATE TABLE t AS SELECT * FROM pg_catalog.pg_cursors;
CREATE MATERIALIZED VIEW v AS SELECT * FROM pg_catalog.pg_cursors;
CREATE TABLE t AS SELECT * FROM crdb_internal.create_statements;
CREATE MATERIALIZED VIEW v AS SELECT * FROM crdb_internal.create_statements;
```

Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
Co-authored-by: Evan Wall <wall@cockroachlabs.com>

pkg/cli/democluster/demo_cluster.go

@@ -1923,8 +1923,8 @@ func (c *transientCluster) ListDemoNodes(w, ew io.Writer, justOne, verbose bool)
 		if !c.demoCtx.Multitenant || verbose {
 			// Connection parameters for the system tenant follow.
 			uiURL := s.Cfg.AdminURL()
-			if q := uiURL.Query(); c.demoCtx.Multitenant && !c.demoCtx.DisableServerController && !q.Has(server.TenantNameParamInQueryURL) {
-				q.Add(server.TenantNameParamInQueryURL, catconstants.SystemTenantName)
+			if q := uiURL.Query(); c.demoCtx.Multitenant && !c.demoCtx.DisableServerController && !q.Has(server.ClusterNameParamInQueryURL) {
+				q.Add(server.ClusterNameParamInQueryURL, catconstants.SystemTenantName)
 				uiURL.RawQuery = q.Encode()
 			}
 
@@ -2006,7 +2006,7 @@ func (c *transientCluster) addDemoLoginToURL(uiURL *url.URL, includeTenantName b
 	}
 
 	if !includeTenantName {
-		q.Del(server.TenantNameParamInQueryURL)
+		q.Del(server.ClusterNameParamInQueryURL)
 	}
 
 	uiURL.RawQuery = q.Encode()

pkg/cli/interactive_tests/test_demo_cli_integration.tcl

@@ -167,7 +167,7 @@ eexpect ":/# "
 # Check that the cookies work.
 set pyfile [file join [file dirname $argv0] test_auth_cookie.py]
 
-send "$python $pyfile cookie_system.txt 'http://localhost:8080/_admin/v1/users?tenant_name=system'\r"
+send "$python $pyfile cookie_system.txt 'http://localhost:8080/_admin/v1/users?cluster=system'\r"
 eexpect "username"
 eexpect "demo"
 # No tenant name specified -> use default tenant.
@@ -190,7 +190,7 @@ eexpect "defaultdb>"
 
 set spawn_id $shell_spawn_id
 
-send "$python $pyfile cookie_system.txt 'http://localhost:8080/_admin/v1/users?tenant_name=system'\r"
+send "$python $pyfile cookie_system.txt 'http://localhost:8080/_admin/v1/users?cluster=system'\r"
 eexpect "username"
 eexpect "demo"
 # No tenant name specified -> use default tenant.

pkg/server/server_controller_http.go

@@ -28,8 +28,9 @@ const (
 	// TenantSelectHeader is the HTTP header used to select a particular tenant.
 	TenantSelectHeader = `X-Cockroach-Tenant`
 
-	// TenantNameParamInQueryURL is the HTTP query URL parameter used to select a particular tenant.
-	TenantNameParamInQueryURL = "tenant_name"
+	// ClusterNameParamInQueryURL is the HTTP query URL parameter used
+	// to select a particular virtual cluster.
+	ClusterNameParamInQueryURL = "cluster"
 
 	// TenantSelectCookieName is the name of the HTTP cookie used to select a particular tenant,
 	// if the custom header is not specified.
@@ -109,7 +110,7 @@ func (c *serverController) httpMux(w http.ResponseWriter, r *http.Request) {
 
 func getTenantNameFromHTTPRequest(st *cluster.Settings, r *http.Request) roachpb.TenantName {
 	// Highest priority is manual override on the URL query parameters.
-	if tenantName := r.URL.Query().Get(TenantNameParamInQueryURL); tenantName != "" {
+	if tenantName := r.URL.Query().Get(ClusterNameParamInQueryURL); tenantName != "" {
 		return roachpb.TenantName(tenantName)
 	}
 

pkg/server/testserver_http.go

@@ -68,7 +68,7 @@ func (ts *httpTestServer) AdminURL() *serverutils.TestURL {
 	u := ts.t.sqlServer.execCfg.RPCContext.Config.AdminURL()
 	if ts.t.tenantName != "" {
 		q := u.Query()
-		q.Add(TenantNameParamInQueryURL, string(ts.t.tenantName))
+		q.Add(ClusterNameParamInQueryURL, string(ts.t.tenantName))
 		u.RawQuery = q.Encode()
 	}
 	return &serverutils.TestURL{URL: u}

pkg/sql/create_as_test.go

@@ -31,20 +31,6 @@ import (
 func TestCreateAsVTable(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 
-	// These are the vtables that need to be fixed.
-	// The map should be empty if all vtables are supported.
-	brokenTables := map[string]struct{}{
-		// TODO(sql-foundations): Fix nil pointer dereference.
-		//  See https://github.com/cockroachdb/cockroach/issues/106166.
-		`pg_catalog.pg_prepared_statements`: {},
-		// TODO(sql-foundations): Fix nil pointer dereference.
-		//  See https://github.com/cockroachdb/cockroach/issues/106167.
-		`pg_catalog.pg_cursors`: {},
-		// TODO(sql-foundations): Fix nil pointer dereference.
-		//  See https://github.com/cockroachdb/cockroach/issues/106168.
-		`"".crdb_internal.create_statements`: {},
-	}
-
 	ctx := context.Background()
 	testCluster := serverutils.StartNewTestCluster(t, 1, base.TestClusterArgs{})
 	defer testCluster.Stopper().Stop(ctx)
@@ -88,10 +74,6 @@ func TestCreateAsVTable(t *testing.T) {
 			}
 
 			fqName := name.FQString()
-			if _, ok := brokenTables[fqName]; ok {
-				continue
-			}
-
 			// Filter by trace_id to prevent error when selecting from
 			// crdb_internal.cluster_inflight_traces:
 			// "pq: a trace_id value needs to be specified".

pkg/sql/planner.go

@@ -405,6 +405,7 @@ func newInternalPlanner(
 	p.semaCtx.SearchPath = &sd.SearchPath
 	p.semaCtx.TypeResolver = p
 	p.semaCtx.FunctionResolver = p
+	p.semaCtx.NameResolver = p
 	p.semaCtx.DateStyle = sd.GetDateStyle()
 	p.semaCtx.IntervalStyle = sd.GetIntervalStyle()
 
@@ -457,6 +458,8 @@ func newInternalPlanner(
 
 	p.queryCacheSession.Init()
 	p.optPlanningCtx.init(p)
+	p.sqlCursors = emptySqlCursors{}
+	p.preparedStatements = emptyPreparedStatements{}
 	p.createdSequences = emptyCreatedSequences{}
 
 	p.schemaResolver.descCollection = p.Descriptors()

pkg/sql/prepared_stmt.go

@@ -126,6 +126,27 @@ type preparedStatementsAccessor interface {
 	DeleteAll(ctx context.Context)
 }
 
+// emptyPreparedStatements is the default impl used by the planner when the
+// connExecutor is not available.
+type emptyPreparedStatements struct{}
+
+var _ preparedStatementsAccessor = emptyPreparedStatements{}
+
+func (e emptyPreparedStatements) List() map[string]*PreparedStatement {
+	return nil
+}
+
+func (e emptyPreparedStatements) Get(string, bool) (*PreparedStatement, bool) {
+	return nil, false
+}
+
+func (e emptyPreparedStatements) Delete(context.Context, string) bool {
+	return false
+}
+
+func (e emptyPreparedStatements) DeleteAll(context.Context) {
+}
+
 // PortalPausablity mark if the portal is pausable and the reason. This is
 // needed to give the correct error for usage of multiple active portals.
 type PortalPausablity int64

pkg/sql/sql_cursor.go

@@ -301,6 +301,32 @@ type sqlCursors interface {
 	list() map[tree.Name]*sqlCursor
 }
 
+// emptySqlCursors is the default impl used by the planner when the
+// connExecutor is not available.
+type emptySqlCursors struct{}
+
+var _ sqlCursors = emptySqlCursors{}
+
+func (e emptySqlCursors) closeAll(bool) error {
+	return errors.AssertionFailedf("closeAll not supported in emptySqlCursors")
+}
+
+func (e emptySqlCursors) closeCursor(tree.Name) error {
+	return errors.AssertionFailedf("closeCursor not supported in emptySqlCursors")
+}
+
+func (e emptySqlCursors) getCursor(tree.Name) *sqlCursor {
+	return nil
+}
+
+func (e emptySqlCursors) addCursor(tree.Name, *sqlCursor) error {
+	return errors.AssertionFailedf("addCursor not supported in emptySqlCursors")
+}
+
+func (e emptySqlCursors) list() map[tree.Name]*sqlCursor {
+	return nil
+}
+
 // cursorMap is a sqlCursors that's backed by an actual map.
 type cursorMap struct {
 	cursors map[tree.Name]*sqlCursor