Merge #153727

153727: sql: implement show inspect errors r=bghal a=bghal

Adds the implementation for `SHOW INSPECT ERRORS` which reads from the
`system.inspect_errors` table. This command is used to review the
results of the data consistency validation jobs spun up by `INSPECT`.

Part of: #148287 
Epic: CRDB-30356

Release note: None

Co-authored-by: Brendan Gerrity <[email protected]>

Author: craig[bot]

pkg/sql/catalog/bootstrap/testdata/testdata

@@ -27,7 +27,6 @@ var (
 		catconstants.StatementActivityTableName,
 		catconstants.TransactionActivityTableName,
 		catconstants.PreparedTransactionsTableName,
-		catconstants.InspectErrorsTableName,
 	}
 
 	readWriteSystemTables = []catconstants.SystemTableName{
@@ -83,6 +82,7 @@ var (
 		catconstants.TransactionDiagnosticsRequestsTableName,
 		catconstants.TransactionDiagnosticsTableName,
 		catconstants.StatementHintsTableName,
+		catconstants.InspectErrorsTableName,
 	}
 
 	readWriteSystemSequences = []catconstants.SystemTableName{

pkg/sql/catalog/catprivilege/system.go

@@ -62,6 +62,7 @@ go_library(
         "//pkg/sql/privilege",
         "//pkg/sql/roleoption",
         "//pkg/sql/sem/catconstants",
+        "//pkg/sql/sem/catid",
         "//pkg/sql/sem/eval",
         "//pkg/sql/sem/tree",
         "//pkg/sql/sessiondatapb",

pkg/sql/catalog/systemschema_test/testdata/bootstrap_system

@@ -6,9 +6,15 @@
 package delegate
 
 import (
+	"fmt"
 	"strings"
 
+	"github.com/cockroachdb/cockroach/pkg/clusterversion"
+	"github.com/cockroachdb/cockroach/pkg/jobs"
+	"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/catid"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege"
@@ -17,21 +23,78 @@ import (
 func (d *delegator) delegateShowInspectErrors(n *tree.ShowInspectErrors) (tree.Statement, error) {
 	sqltelemetry.IncrementShowCounter(sqltelemetry.InspectErrors)
 
+	if !d.evalCtx.Settings.Version.IsActive(d.ctx, clusterversion.V25_4) {
+		return nil, pgerror.Newf(pgcode.FeatureNotSupported, "SHOW INSPECT ERRORS requires the cluster to be upgraded to v25.4")
+	}
+
 	if err := d.catalog.CheckPrivilege(d.ctx, syntheticprivilege.GlobalPrivilegeObject,
 		d.catalog.GetCurrentUser(), privilege.INSPECT); err != nil {
 		return nil, err
 	}
 
+	var tableID = catid.InvalidDescID
 	if n.TableName != nil {
-		_, _, err := d.catalog.ResolveDataSource(d.ctx, resolveFlags, n.TableName)
+		resolvedTableID, err := d.evalCtx.Planner.ResolveTableName(d.ctx, n.TableName)
 		if err != nil {
 			return nil, err
 		}
 
+		tableID = catid.DescID(resolvedTableID)
 	}
 
 	var query strings.Builder
-	query.WriteString("SELECT * FROM system.inspect_errors")
+
+	query.WriteString(`WITH
+	inspect_jobs AS (
+		SELECT id
+		FROM crdb_internal.system_jobs
+		WHERE job_type = 'INSPECT'
+	`)
+	if n.JobID == nil {
+		query.WriteString(fmt.Sprintf(` AND status IN ('%s', '%s', '%s', '%s')`,
+			jobs.StateFailed, jobs.StateSucceeded, jobs.StateCanceled, jobs.StateRevertFailed)) // in terminal state
+	} else {
+		query.WriteString(fmt.Sprintf(" AND id = %d", *n.JobID))
+	}
+
+	// TODO(148287): query the inspect job payload to figure out if a job touches a particular table or database ID
+	// If a table was specified, only consider jobs that reported errors on it.
+	// If a job ID was specified, only consider that job. The records from the
+	// most recent completed job that satisfies those criteria is used.
+	query.WriteString(`),
+	job_id AS (
+		SELECT max(inspect_jobs.id) as id
+		FROM inspect_jobs
+		JOIN crdb_internal.cluster_inspect_errors ie ON inspect_jobs.id = ie.job_id
+		WHERE 1=1
+	`)
+	if tableID != catid.InvalidDescID {
+		query.WriteString(fmt.Sprintf(" AND ie.id = %d", tableID))
+	}
+	if n.JobID != nil {
+		query.WriteString(fmt.Sprintf(" AND ie.job_id = %d", *n.JobID))
+	}
+
+	query.WriteString(`)
+	SELECT 
+		ie.error_type,
+		COALESCE(t.database_name, '<unknown>') AS database_name,
+		COALESCE(t.schema_name, '<unknown>') AS schema_name,
+		COALESCE(t.object_name, '<unknown>') AS table_name,
+		primary_key,
+		ie.job_id,
+		to_char(aost, 'YYYY-MM-DD HH24:MI:SS.US') as aost`)
+	if n.WithDetails {
+		query.WriteString(", jsonb_pretty(ie.details) as details")
+	}
+	query.WriteString(`
+		FROM crdb_internal.cluster_inspect_errors ie
+		LEFT JOIN crdb_internal.fully_qualified_names t ON ie.id = t.object_id
+		WHERE ie.job_id IN (SELECT id FROM job_id)`)
+	if tableID != catid.InvalidDescID {
+		query.WriteString(fmt.Sprintf(" AND ie.id = %d", tableID))
+	}
+	query.WriteString(` ORDER BY ie.error_type, ie.error_id`)
 
 	return d.parse(query.String())
 }

pkg/sql/catalog/systemschema_test/testdata/bootstrap_tenant

@@ -8,6 +8,9 @@ package sql
 import (
 	"context"
 
+	"github.com/cockroachdb/cockroach/pkg/clusterversion"
+	"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/util/errorutil/unimplemented"
@@ -23,6 +26,14 @@ type inspectNode struct {
 // Inspect checks the database.
 // Privileges: INSPECT.
 func (p *planner) Inspect(ctx context.Context, n *tree.Inspect) (planNode, error) {
+	if !p.extendedEvalCtx.Settings.Version.IsActive(ctx, clusterversion.V25_4) {
+		return nil, pgerror.Newf(pgcode.FeatureNotSupported, "SHOW INSPECT ERRORS requires the cluster to be upgraded to v25.4")
+	}
+
+	if err := p.CheckGlobalPrivilegeOrRoleOption(ctx, privilege.INSPECT); err != nil {
+		return nil, err
+	}
+
 	switch n.Typ {
 	case tree.InspectTable:
 		tableName := n.Table.ToTableName()
@@ -39,10 +50,6 @@ func (p *planner) Inspect(ctx context.Context, n *tree.Inspect) (planNode, error
 		return nil, errors.AssertionFailedf("unexpected INSPECT type received, got: %v", n.Typ)
 	}
 
-	if err := p.CheckGlobalPrivilegeOrRoleOption(ctx, privilege.INSPECT); err != nil {
-		return nil, err
-	}
-
 	return &inspectNode{n: n}, nil
 }
 

pkg/sql/delegate/BUILD.bazel

@@ -57,24 +57,33 @@ subtest end
 # inspect and scrub are converging so for now tests on INSPECT live here
 subtest inspect
 
+onlyif config local-mixed-25.2 local-mixed-25.3
+statement error pq: SHOW INSPECT ERRORS requires the cluster to be upgraded to v25.4
+SHOW INSPECT ERRORS
+
 statement ok
 CREATE TABLE foo (c1 INT);
 
+skipif config local-mixed-25.2 local-mixed-25.3
 statement error INSPECT TABLE requires the enable_inspect_command setting to be enabled
 INSPECT TABLE foo;
 
+skipif config local-mixed-25.2 local-mixed-25.3
 statement error INSPECT DATABASE requires the enable_inspect_command setting to be enabled
 INSPECT DATABASE test;
 
 statement ok
 SET enable_inspect_command = true;
 
+skipif config local-mixed-25.2 local-mixed-25.3
 statement ok
 INSPECT TABLE foo;
 
+skipif config local-mixed-25.2 local-mixed-25.3
 statement ok
 INSPECT TABLE foo WITH OPTIONS INDEX (bar.public.biz);
 
+skipif config local-mixed-25.2 local-mixed-25.3
 statement ok
 INSPECT TABLE foo AS OF SYSTEM TIME 1 WITH OPTIONS INDEX (bar,biz);
 
@@ -84,9 +93,11 @@ CREATE USER testuser2;
 
 user testuser2
 
+skipif config local-mixed-25.2 local-mixed-25.3
 statement error pq: user testuser2 does not have INSPECT privilege
 INSPECT TABLE foo;
 
+skipif config local-mixed-25.2 local-mixed-25.3
 statement error pq: user testuser2 does not have INSPECT privilege
 INSPECT DATABASE test;
 
@@ -100,9 +111,11 @@ user testuser2
 statement ok
 SET enable_inspect_command = true;
 
+skipif config local-mixed-25.2 local-mixed-25.3
 statement ok
 INSPECT TABLE foo;
 
+skipif config local-mixed-25.2 local-mixed-25.3
 statement ok
 INSPECT DATABASE test;
 

pkg/sql/delegate/show_inspect_errors.go

@@ -1,41 +1,142 @@
+onlyif config local-mixed-25.2 local-mixed-25.3
+statement error pq: SHOW INSPECT ERRORS requires the cluster to be upgraded to v25.4
+SHOW INSPECT ERRORS
+
 user testuser
 
-skipif config local-mixed-25.2
-skipif config local-mixed-25.3
+skipif config local-mixed-25.2 local-mixed-25.3
 statement error pq: user testuser does not have INSPECT system privilege
 SHOW INSPECT ERRORS
 
 user root
 
-skipif config local-mixed-25.2
-skipif config local-mixed-25.3
+statement ok
+GRANT SYSTEM INSPECT TO testuser;
+
+skipif config local-mixed-25.2 local-mixed-25.3
 statement error relation "bad_table" does not exist
 SHOW INSPECT ERRORS FOR TABLE bad_table
 
 statement ok
-CREATE TABLE foo (a INT)
+CREATE TABLE foo (a INT);
+CREATE TABLE bar (b INT);
+
+let $foo_table_id
+SELECT 'foo'::regclass::oid
+
+let $bar_table_id
+SELECT 'bar'::regclass::oid
+
+let $database_id
+SELECT id FROM system.namespace WHERE name = current_database() AND "parentID" = 0
+
+let $schema_id
+SELECT current_schema()::regnamespace::oid
+
+let $aost
+SELECT '2025-09-23-11:02:14-04:00'::timestamp
+
+# the job_info insert addresses the `crdb_internal.system_jobs` inner join
+statement ok
+INSERT INTO system.jobs (id, owner, status, job_type)
+VALUES (555, 'testuser', 'failed', 'INSPECT');
+INSERT INTO system.job_info (job_id, info_key)
+VALUES (555, 'legacy_payload');
 
-skipif config local-mixed-25.2
-skipif config local-mixed-25.3
+skipif config local-mixed-25.2 local-mixed-25.3
 statement ok
-SHOW INSPECT ERRORS
+INSERT INTO system.inspect_errors (job_id, error_type, aost, database_id, schema_id, id, details)
+VALUES 
+  (555, '555_foo', '$aost', $database_id, $schema_id, $foo_table_id, '{"detail":"555\"foo"}'),
+  (555, '555_bar', '$aost', $database_id, $schema_id, $bar_table_id, '{"detail":"555\"bar"}');
 
-skipif config local-mixed-25.2
-skipif config local-mixed-25.3
 statement ok
-SHOW INSPECT ERRORS WITH DETAILS
+INSERT INTO system.jobs (id, owner, status, job_type)
+VALUES (666, 'testuser', 'failed', 'INSPECT');
+INSERT INTO system.job_info (job_id, info_key)
+VALUES (666, 'legacy_payload');
 
-skipif config local-mixed-25.2
-skipif config local-mixed-25.3
+skipif config local-mixed-25.2 local-mixed-25.3
 statement ok
-SHOW INSPECT ERRORS FOR JOB 666
+INSERT INTO system.inspect_errors (job_id, error_type, aost, database_id, schema_id, id, details)
+VALUES (666, '666_foo', '$aost', $database_id, $schema_id, $foo_table_id, '{"detail1":"\u2603 666_foo_1","detail2":"\n666_foo_2"}');
 
-skipif config local-mixed-25.2
-skipif config local-mixed-25.3
 statement ok
-SHOW INSPECT ERRORS FOR TABLE foo
+INSERT INTO system.jobs (id, owner, status, job_type)
+VALUES (777, 'testuser', 'running', 'INSPECT');
+INSERT INTO system.job_info (job_id, info_key)
+VALUES (777, 'legacy_payload');
 
-skipif config local-mixed-25.2
-skipif config local-mixed-25.3
+skipif config local-mixed-25.2 local-mixed-25.3
 statement ok
-SHOW INSPECT ERRORS FOR TABLE foo FOR JOB 666 WITH DETAILS
+INSERT INTO system.inspect_errors (job_id, error_type, aost, database_id, schema_id, id, details)
+VALUES (777, '777_foo', '$aost', $database_id, $schema_id, $foo_table_id, '{"detail":"777 foo"}');
+
+user testuser
+
+# no options should show most records from the recent completed job
+skipif config local-mixed-25.2 local-mixed-25.3
+query TTTTTIT colnames
+SHOW INSPECT ERRORS
+----
+error_type  database_name  schema_name  table_name  primary_key  job_id  aost
+666_foo     test           public       foo         NULL         666     2025-09-23 04:00:00.000000
+
+skipif config local-mixed-25.2 local-mixed-25.3
+query TTTTTITT colnames
+SHOW INSPECT ERRORS WITH DETAILS
+----
+error_type  database_name  schema_name  table_name  primary_key  job_id  aost                        details
+666_foo     test           public       foo         NULL         666     2025-09-23 04:00:00.000000  {
+                                                                                                         "detail1": "☃ 666_foo_1",
+                                                                                                         "detail2": "\n666_foo_2"
+                                                                                                     }
+
+# specifying a job shows results regardless of status
+skipif config local-mixed-25.2 local-mixed-25.3
+query TTTTTIT
+SHOW INSPECT ERRORS FOR JOB 777
+----
+777_foo  test  public  foo  NULL  777  2025-09-23 04:00:00.000000
+
+skipif config local-mixed-25.2 local-mixed-25.3
+query TTTTTIT nosort
+SHOW INSPECT ERRORS FOR JOB 555
+----
+555_bar  test  public  bar  NULL  555  2025-09-23 04:00:00.000000
+555_foo  test  public  foo  NULL  555  2025-09-23 04:00:00.000000
+
+# query by table shows records from the most recent completed job that reported errors on that table
+skipif config local-mixed-25.2 local-mixed-25.3
+query TTTTTIT
+SHOW INSPECT ERRORS FOR TABLE foo
+----
+666_foo  test  public  foo  NULL  666  2025-09-23 04:00:00.000000
+
+# and query by table only shows records for that given table
+skipif config local-mixed-25.2 local-mixed-25.3
+query TTTTTIT
+SHOW INSPECT ERRORS FOR TABLE bar
+----
+555_bar  test  public  bar  NULL  555  2025-09-23 04:00:00.000000
+
+skipif config local-mixed-25.2 local-mixed-25.3
+query TTTTTIT
+SHOW INSPECT ERRORS FOR TABLE public.bar
+----
+555_bar  test  public  bar  NULL  555  2025-09-23 04:00:00.000000
+
+skipif config local-mixed-25.2 local-mixed-25.3
+query TTTTTIT
+SHOW INSPECT ERRORS FOR TABLE test.public.bar
+----
+555_bar  test  public  bar  NULL  555  2025-09-23 04:00:00.000000
+
+skipif config local-mixed-25.2 local-mixed-25.3
+query TTTTTITT colnames
+SHOW INSPECT ERRORS FOR TABLE foo FOR JOB 555 WITH DETAILS
+----
+error_type  database_name  schema_name  table_name  primary_key  job_id  aost                        details
+555_foo     test           public       foo         NULL         555     2025-09-23 04:00:00.000000  {
+                                                                                                         "detail": "555\"foo"
+                                                                                                     }