Merge #63743 #63805 #63930 #64027 #64076

63743: sql: batch write event logs for grant/revoke r=ajwerner a=the-ericwang35

Helps with #41930.

Previously, if we ran grant/revoke on multiple tables,
we would create event logs for each table and write them
one by one, resulting in round trips proportional to
the number of tables.
This patch addresses this by batch writing the event logs,
so that 1 write to the event log table occurs regardless
of the number of tables updated.

Release note: None

63805: opt,sql: support streaming set ops r=rytaft a=rytaft

This commit adds support for planning streaming set operations in
the optimizer. Streaming set operations consist of a `UNION`, `INTERSECT`,
or `EXCEPT` (including `ALL` variants) in which both inputs are ordered
on the desired output ordering, and the set operation merges the
inputs to maintain the ordering. The execution engine already
supported this type of streaming set operation, so the purpose of
this commit was to teach the optimizer about this capability and hook
it up to the execution engine.

This is also a prerequisite for #56201.

Fixes #40797
Informs #56201

Release note (performance improvement): Set operations (`UNION`,
`UNION ALL`, `INTERSECT`, `INTERSECT ALL`, `EXCEPT` and `EXCEPT ALL`) can
now maintain ordering if both inputs are ordered on the desired
output ordering. This can eliminate unnecessary sort operations
and improve performance.

63930: scripts/gceworker.sh: always populate username r=ajwerner a=ajwerner

In the previous change (#63645) to this file we were to allow injecting a username.
The code before this commit, the code would populate the username as "" leading
to a leading `@` in the address. The `ssh` family of commands are happy enough
with this but the unison command is not. Before this change, we'd see:

```
Fatal error: ill-formed root specification ssh://@gceworker-ajwerner.us-east1-b.cockroach-workers/go/src/github.com/cockroachdb/cockroach
```

I'm content doing something different here like detecting whether we have
a username injected instead. I don't have a strong reason to do that but
maybe by explicitly making the user the output of `whoami` I'm preventing
some other customization.

Release note: None

64027: sql: disallow RBR transforms if transitioning indexes are incompatible r=arulajmani a=otan

Release note (bug fix): Previously, if a DROP INDEX failed during a
REGIONAL BY ROW transition, the index may be re-inserted back into the
REGIONAL BY ROW table but would be invalid if it was sharded or
partitioned. This is now rectified.

64076: sql: add statement_id to crdb_internal.node_statement_statistics r=arulajmani a=Azhng

Currently, crdb_internal.node_transaction_statistics uses the
statement_ids column to reference statements in
crdb_internal.node_statement_statistics. However, the statement
statistics view does not have column that shows statement id.

This commit introduce a new statement_id column in the statement
statistics view.

Release note (sql change): crdb_internal.node_statement_statistics
 now stores statement_id.

Closes: #64072

Co-authored-by: Eric Wang <[email protected]>
Co-authored-by: Rebecca Taft <[email protected]>
Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Oliver Tan <[email protected]>
Co-authored-by: Azhng <[email protected]>

pkg/bench/ddl_analysis/testdata/benchmark_expectations

@@ -48,8 +48,8 @@ exp,benchmark
 40,DropView/drop_2_views
 57,DropView/drop_3_views
 19,Grant/grant_all_on_1_table
-23,Grant/grant_all_on_2_tables
-27,Grant/grant_all_on_3_tables
+22,Grant/grant_all_on_2_tables
+25,Grant/grant_all_on_3_tables
 19,GrantRole/grant_1_role
 22,GrantRole/grant_2_roles
 2,ORMQueries/activerecord_type_introspection_query
@@ -61,8 +61,8 @@ exp,benchmark
 2,ORMQueries/pg_namespace
 2,ORMQueries/pg_type
 19,Revoke/revoke_all_on_1_table
-23,Revoke/revoke_all_on_2_tables
-27,Revoke/revoke_all_on_3_tables
+22,Revoke/revoke_all_on_2_tables
+25,Revoke/revoke_all_on_3_tables
 18,RevokeRole/revoke_1_role
 20,RevokeRole/revoke_2_roles
 1,SystemDatabaseQueries/select_system.users_with_empty_database_name

pkg/ccl/logictestccl/testdata/logic_test/alter_table_locality

@@ -2625,7 +2625,7 @@ CREATE TABLE hash_sharded_idx_table (
   pk INT PRIMARY KEY USING HASH WITH BUCKET_COUNT = 8
 )
 
-statement error cannot convert a table to REGIONAL BY ROW if table table contains hash sharded indexes
+statement error cannot convert hash_sharded_idx_table to REGIONAL BY ROW as the table contains hash sharded indexes
 ALTER TABLE hash_sharded_idx_table SET LOCALITY REGIONAL BY ROW
 
 statement ok

pkg/sql/alter_database.go

@@ -746,7 +746,7 @@ func checkCanConvertTableToMultiRegion(
 			tableDesc.GetName(),
 		)
 	}
-	for _, idx := range tableDesc.NonDropIndexes() {
+	for _, idx := range tableDesc.AllIndexes() {
 		if idx.GetPartitioning().NumColumns > 0 {
 			return errors.WithDetailf(
 				pgerror.Newf(

pkg/sql/alter_table_locality.go

@@ -240,9 +240,13 @@ func (n *alterTableSetLocalityNode) alterTableLocalityToRegionalByRow(
 		return interleaveOnRegionalByRowError()
 	}
 
-	for _, idx := range n.tableDesc.NonDropIndexes() {
+	for _, idx := range n.tableDesc.AllIndexes() {
 		if idx.IsSharded() {
-			return pgerror.New(pgcode.FeatureNotSupported, "cannot convert a table to REGIONAL BY ROW if table table contains hash sharded indexes")
+			return pgerror.Newf(
+				pgcode.FeatureNotSupported,
+				"cannot convert %s to REGIONAL BY ROW as the table contains hash sharded indexes",
+				tree.Name(n.tableDesc.GetName()),
+			)
 		}
 	}
 

pkg/sql/crdb_internal.go

@@ -840,6 +840,7 @@ CREATE TABLE crdb_internal.node_statement_statistics (
   node_id             INT NOT NULL,
   application_name    STRING NOT NULL,
   flags               STRING NOT NULL,
+  statement_id        STRING NOT NULL,
   key                 STRING NOT NULL,
   anonymized          STRING,
   count               INT NOT NULL,
@@ -941,14 +942,15 @@ CREATE TABLE crdb_internal.node_statement_statistics (
 					flags = "!" + flags
 				}
 				err := addRow(
-					tree.NewDInt(tree.DInt(nodeID)),                      // node_id
-					tree.NewDString(appName),                             // application_name
-					tree.NewDString(flags),                               // flags
-					tree.NewDString(stmtKey.anonymizedStmt),              // key
-					anonymized,                                           // anonymized
-					tree.NewDInt(tree.DInt(s.mu.data.Count)),             // count
-					tree.NewDInt(tree.DInt(s.mu.data.FirstAttemptCount)), // first_attempt_count
-					tree.NewDInt(tree.DInt(s.mu.data.MaxRetries)),        // max_retries
+					tree.NewDInt(tree.DInt(nodeID)),                         // node_id
+					tree.NewDString(appName),                                // application_name
+					tree.NewDString(flags),                                  // flags
+					tree.NewDString(strconv.FormatUint(uint64(stmtID), 10)), // statement_id
+					tree.NewDString(stmtKey.anonymizedStmt),                 // key
+					anonymized,                                              // anonymized
+					tree.NewDInt(tree.DInt(s.mu.data.Count)),                // count
+					tree.NewDInt(tree.DInt(s.mu.data.FirstAttemptCount)),    // first_attempt_count
+					tree.NewDInt(tree.DInt(s.mu.data.MaxRetries)),           // max_retries
 					errString, // last_error
 					tree.NewDFloat(tree.DFloat(s.mu.data.NumRows.Mean)),                             // rows_avg
 					tree.NewDFloat(tree.DFloat(s.mu.data.NumRows.GetVariance(s.mu.data.Count))),     // rows_var

pkg/sql/create_stats.go

@@ -576,15 +576,15 @@ func (r *createStatsResumer) Resume(ctx context.Context, execCtx interface{}) er
 	// See: https://github.com/cockroachdb/cockroach/issues/57739
 	return evalCtx.ExecCfg.DB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error {
 		return logEventInternalForSQLStatements(ctx, evalCtx.ExecCfg, txn,
-			details.Table.ID,
+			descpb.IDs{details.Table.ID},
 			evalCtx.SessionData.User(),
 			evalCtx.SessionData.ApplicationName,
 			details.Statement,
-			nil, /* no placeholders known at this point */
+			nil,  /* no placeholders known at this point */
+			true, /* writeToEventLog */
 			&eventpb.CreateStatistics{
 				TableName: details.FQTableName,
 			},
-			true, /* writeToEventLog */
 		)
 	})
 }

pkg/sql/distsql_physical_planner.go

@@ -3330,15 +3330,8 @@ func (dsp *DistSQLPlanner) createPlanForSetOp(
 		return nil, err
 	}
 
-	if len(leftPlan.MergeOrdering.Columns) != 0 || len(rightPlan.MergeOrdering.Columns) != 0 {
-		return nil, errors.AssertionFailedf("set op inputs should have no orderings")
-	}
-
-	// TODO(radu): for INTERSECT and EXCEPT, the mergeOrdering should be set when
-	// we can use merge joiners below. The optimizer needs to be modified to take
-	// advantage of this optimization and pass down merge orderings. Tracked by
-	// #40797.
-	var mergeOrdering execinfrapb.Ordering
+	// Set the merge ordering.
+	mergeOrdering := dsp.convertOrdering(n.reqOrdering, p.PlanToStreamColMap)
 
 	// Merge processors, streams, result routers, and stage counter.
 	leftRouters := leftPlan.ResultRouters
@@ -3475,11 +3468,6 @@ func (dsp *DistSQLPlanner) createPlanForSetOp(
 			)
 		}
 
-		// An EXCEPT ALL is like a left outer join, so there is no guaranteed ordering.
-		if n.unionType == tree.ExceptOp {
-			mergeOrdering = execinfrapb.Ordering{}
-		}
-
 		p.SetMergeOrdering(mergeOrdering)
 	}
 

pkg/sql/distsql_spec_exec_factory.go

@@ -592,7 +592,11 @@ func (e *distSQLSpecExecFactory) ConstructDistinct(
 }
 
 func (e *distSQLSpecExecFactory) ConstructSetOp(
-	typ tree.UnionType, all bool, left, right exec.Node, hardLimit uint64,
+	typ tree.UnionType,
+	all bool,
+	left, right exec.Node,
+	reqOrdering exec.OutputOrdering,
+	hardLimit uint64,
 ) (exec.Node, error) {
 	return nil, unimplemented.NewWithIssue(47473, "experimental opt-driven distsql planning: set op")
 }