Merge #110565 #110966

110565: sql, insights: record error message for failed stmts r=yuzefovich a=xinhaoz

### Commit 1 sql: modify HasViewActivityOrViewActivityRedactedRole signature

This commit adds an additional return value to the function
`HasViewActivityOrViewActivityRedactedRole` which returns true if
the user has either `VIEWACTIVITY` or `VIEWACTIVITYREDACTED` roles.

In addition to that, the function will now return a boolean indicating
whether the user is not admin and has the `VIEWACTIVITYREDACTED` role.
This simplifies  areas where we need to redact values based on the
presence of this role, allowing callers to skip an additional role check
for admin and VIEWACTIVITYREDACTED.

Epic: none

Release note: None
### Commit 2 sql, insights: record error message for failed stmts

Part of: #87785.

Previously, the insights subsystem did not keep track of the error messages for failed executions, only the error codes.

This commit  updates the `[cluster|node]_execution_insights` and `[cluster|node]_txn_execution_insights` virtual tables to include a `last_error` column which contains the most recent error message.

Release note (sql change): adds `last_error` column to the `[cluster|node]_execution_insights` and `[cluster|node]_txn_execution_insights` tables which keeps track of the error message for failed executions.

110966: sql: fix gist decoding in foreign DB r=yuzefovich a=cucaroach

When a gist is exported and decoded in a foreign DB it should still
decode w/o panic. If a table scan result columns are fed into a
group by column the group by column code expects the result columns to
be in tact and panics. Fix this by making fake "unknown" result columns
so we can print the plan.

Making this change led to an unintended change in gists_tpce, this is
because the catalog was shared between the test files and I added a
table.  Fix this for the future by creating a new catalog for each file.
But with this change the table ids encoded in the gists_tpce got
smaller.

Epic: None
Fixes: #110964
Release note (bug fix): Fixed panic when decoding gist in DB without the
table referred to by the gist.


Co-authored-by: Xin Hao Zhang <xzhang@cockroachlabs.com>
Co-authored-by: Xin Hao Zhang <xzhang@cockroahchlabs.com>
Co-authored-by: Tommy Reilly <treilly@cockroachlabs.com>

pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant

@@ -327,25 +327,25 @@ SELECT * FROM crdb_internal.node_inflight_trace_spans WHERE span_id < 0
 ----
 trace_id  parent_span_id  span_id  goroutine_id  finished  start_time  duration  operation
 
-query TTTBTTTTTIITITTTTTTTTTTTTIT colnames
+query TTTBTTTTTIITITTTTTTTTTTTTITT colnames
 SELECT * FROM crdb_internal.cluster_execution_insights WHERE query = ''
 ----
-session_id  txn_id  txn_fingerprint_id  stmt_id  stmt_fingerprint_id  problem  causes  query  status  start_time  end_time  full_scan  user_name  app_name  database_name  plan_gist  rows_read  rows_written  priority  retries  last_retry_reason  exec_node_ids  contention  index_recommendations  implicit_txn  cpu_sql_nanos  error_code
+session_id  txn_id  txn_fingerprint_id  stmt_id  stmt_fingerprint_id  problem  causes  query  status  start_time  end_time  full_scan  user_name  app_name  database_name  plan_gist  rows_read  rows_written  priority  retries  last_retry_reason  exec_node_ids  contention  index_recommendations  implicit_txn  cpu_sql_nanos  error_code  last_error_redactable
 
-query TTTBTTTTTIITITTTTTTTTTTTTIT colnames
+query TTTBTTTTTIITITTTTTTTTTTTTITT colnames
 SELECT * FROM crdb_internal.node_execution_insights WHERE query = ''
 ----
-session_id  txn_id  txn_fingerprint_id  stmt_id  stmt_fingerprint_id  problem  causes  query  status  start_time  end_time  full_scan  user_name  app_name  database_name  plan_gist  rows_read  rows_written  priority  retries  last_retry_reason  exec_node_ids  contention  index_recommendations  implicit_txn  cpu_sql_nanos  error_code
+session_id  txn_id  txn_fingerprint_id  stmt_id  stmt_fingerprint_id  problem  causes  query  status  start_time  end_time  full_scan  user_name  app_name  database_name  plan_gist  rows_read  rows_written  priority  retries  last_retry_reason  exec_node_ids  contention  index_recommendations  implicit_txn  cpu_sql_nanos  error_code  last_error_redactable
 
-query TTTBTTTTTIITITTTTTITT colnames
+query TTTBTTTTTIITITTTTTITTT colnames
 SELECT * FROM crdb_internal.cluster_txn_execution_insights WHERE query = ''
 ----
-txn_id  txn_fingerprint_id  query  implicit_txn  session_id  start_time  end_time  user_name  app_name  rows_read  rows_written  priority  retries  last_retry_reason  contention  problems  causes  stmt_execution_ids  cpu_sql_nanos  last_error_code status
+txn_id  txn_fingerprint_id  query  implicit_txn  session_id  start_time  end_time  user_name  app_name  rows_read  rows_written  priority  retries  last_retry_reason  contention  problems  causes  stmt_execution_ids  cpu_sql_nanos  last_error_code  last_error_redactable status
 
-query TTTBTTTTTIITITTTTTITT colnames
+query TTTBTTTTTIITITTTTTITTT colnames
 SELECT * FROM crdb_internal.node_txn_execution_insights WHERE query = ''
 ----
-txn_id  txn_fingerprint_id  query  implicit_txn  session_id  start_time  end_time  user_name  app_name  rows_read  rows_written  priority  retries  last_retry_reason  contention  problems  causes  stmt_execution_ids  cpu_sql_nanos  last_error_code status
+txn_id  txn_fingerprint_id  query  implicit_txn  session_id  start_time  end_time  user_name  app_name  rows_read  rows_written  priority  retries  last_retry_reason  contention  problems  causes  stmt_execution_ids  cpu_sql_nanos  last_error_code  last_error_redactable status
 
 query ITTI
 SELECT range_id, start_pretty, end_pretty, lease_holder FROM crdb_internal.ranges

pkg/cli/zip_table_registry.go

@@ -162,6 +162,8 @@ var zipInternalTablesPerCluster = DebugZipTableRegistry{
 			"index_recommendations",
 			"retries",
 			"last_retry_reason",
+			"error_code",
+			"crdb_internal.redact(last_error_redactable) as last_error_redactable",
 		},
 	},
 	"crdb_internal.cluster_locks": {
@@ -283,6 +285,8 @@ var zipInternalTablesPerCluster = DebugZipTableRegistry{
 			"problems",
 			"causes",
 			"stmt_execution_ids",
+			"last_error_code",
+			"crdb_internal.redact(last_error_redactable) as last_error_redactable",
 		},
 	},
 	`"".crdb_internal.create_function_statements`: {
@@ -698,6 +702,8 @@ var zipInternalTablesPerNode = DebugZipTableRegistry{
 			"priority",
 			"retries",
 			"exec_node_ids",
+			"error_code",
+			"crdb_internal.redact(last_error_redactable) as last_error_redactable",
 		},
 	},
 	"crdb_internal.node_inflight_trace_spans": {
@@ -947,6 +953,8 @@ var zipInternalTablesPerNode = DebugZipTableRegistry{
 			"problems",
 			"causes",
 			"stmt_execution_ids",
+			"last_error_code",
+			"crdb_internal.redact(last_error_redactable) as last_error_redactable",
 		},
 	},
 	"crdb_internal.node_txn_stats": {

pkg/sql/authorization.go

@@ -1001,25 +1001,35 @@ func (p *planner) HasOwnershipOnSchema(
 	return hasOwnership, nil
 }
 
-// HasViewActivityOrViewActivityRedactedRole implements the AuthorizationAccessor interface.
+// HasViewActivityOrViewActivityRedactedRole is part of the eval.SessionAccessor interface.
+// It returns 2 boolean values - the first indicating if we have either privilege requested,
+// and the second indicating  whether or not it was VIEWACTIVITYREDACTED.
 // Requires a valid transaction to be open.
-func (p *planner) HasViewActivityOrViewActivityRedactedRole(ctx context.Context) (bool, error) {
+func (p *planner) HasViewActivityOrViewActivityRedactedRole(
+	ctx context.Context,
+) (hasPrivs bool, shouldRedact bool, err error) {
 	if hasAdmin, err := p.HasAdminRole(ctx); err != nil {
-		return hasAdmin, err
+		return hasAdmin, false, err
 	} else if hasAdmin {
-		return true, nil
-	}
-	if hasView, err := p.HasViewActivity(ctx); err != nil {
-		return false, err
-	} else if hasView {
-		return true, nil
+		return true, false, nil
 	}
+
+	// We check for VIEWACTIVITYREDACTED first as users can have both
+	// VIEWACTIVITY and VIEWACTIVITYREDACTED, where VIEWACTIVITYREDACTED
+	// takes precedence (i.e. we must redact senstitive values).
 	if hasViewRedacted, err := p.HasViewActivityRedacted(ctx); err != nil {
-		return false, err
+		return false, false, err
 	} else if hasViewRedacted {
-		return true, nil
+		return true, true, nil
 	}
-	return false, nil
+
+	if hasView, err := p.HasViewActivity(ctx); err != nil {
+		return false, false, err
+	} else if hasView {
+		return true, false, nil
+	}
+
+	return false, false, nil
 }
 
 func (p *planner) HasViewActivityRedacted(ctx context.Context) (bool, error) {