Merge #155774

155774: sql/inspect: add DETACHED option to run INSPECT without waiting r=spilchen a=spilchen

Adds support for running `INSPECT ... WITH OPTIONS (DETACHED)`, which submits the inspection as a background job and returns immediately with the job ID in a notice. This allows INSPECT to run inside a transaction.

**Note to reviewers:**
This was added to support running INSPECT within the random schema workload, which executes statements inside transactions. Previously, INSPECT could not run inside a transaction. It also gives us async behaviour. While expanding INSPECT test coverage, I have wanted to kick off an async run of INSPECT. Until now, I worked around it by using `statement_timeout` to force the job to run in the background so we could later wait on it. The detached option provides a cleaner approach. It is similar to how commands like BACKUP support an async mode.
 
Informs: #155483
Epic: CRDB-55075

Release note (sql change): INSPECT supports a DETACHED option to run the operation without waiting for it.

Co-authored-by: Matt Spilchen <[email protected]>

Author: craig[bot]

docs/generated/sql/bnf/stmt_block.bnf

@@ -3643,6 +3643,9 @@ only_signed_fconst ::=
 inspect_option ::=
 	'INDEX' 'ALL'
 	| 'INDEX' '(' table_index_name_list ')'
+	| 'DETACHED'
+	| 'DETACHED' '=' 'TRUE'
+	| 'DETACHED' '=' 'FALSE'
 
 virtual_cluster_name ::=
 	'VIRTUAL_CLUSTER_NAME'

pkg/sql/inspect/BUILD.bazel

@@ -41,6 +41,7 @@ go_library(
         "//pkg/sql/lexbase",
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",
+        "//pkg/sql/pgwire/pgnotice",
         "//pkg/sql/physicalplan",
         "//pkg/sql/privilege",
         "//pkg/sql/rowexec",

pkg/sql/inspect/inspect_plan.go

@@ -11,11 +11,13 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/jobs"
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
+	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice"
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
@@ -25,12 +27,21 @@ import (
 func inspectTypeCheck(
 	ctx context.Context, stmt tree.Statement, p sql.PlanHookState,
 ) (matched bool, header colinfo.ResultColumns, _ error) {
-	_, ok := stmt.(*tree.Inspect)
+	inspectStmt, ok := stmt.(*tree.Inspect)
 	if !ok {
 		return false, nil, nil
 	}
 
-	return true, jobs.InspectJobExecutionResultHeader, nil
+	// Determine header based on DETACHED option to ensure consistency in
+	// local-prepared mode where plans are cached.
+	detached := inspectStmt.Options.IsDetached()
+	if detached {
+		header = jobs.DetachedJobExecutionResultHeader
+	} else {
+		header = jobs.InspectJobExecutionResultHeader
+	}
+
+	return true, header, nil
 }
 
 // inspectRun represents the runtime state of an execution of INSPECT.
@@ -72,8 +83,8 @@ func newInspectRun(
 		return inspectRun{}, errors.AssertionFailedf("unexpected INSPECT type received, got: %v", stmt.Typ)
 	}
 
-	if len(stmt.Options) == 0 || stmt.Options.HasIndexAll() {
-		// No options or INDEX ALL specified - inspect all indexes.
+	if !stmt.Options.HasIndexOption() || stmt.Options.HasIndexAll() {
+		// No INDEX options or INDEX ALL specified - inspect all indexes.
 		switch stmt.Typ {
 		case tree.InspectTable:
 			checks, err := sql.InspectChecksForTable(ctx, p, run.table)
@@ -166,7 +177,9 @@ func inspectPlanHook(
 		return nil, nil, false, err
 	}
 
-	if !p.ExtendedEvalContext().TxnIsSingleStmt {
+	detached := inspectStmt.Options.IsDetached()
+
+	if !detached && !p.ExtendedEvalContext().TxnIsSingleStmt {
 		return nil, nil, false, pgerror.Newf(pgcode.InvalidTransactionState,
 			"cannot run within a multi-statement transaction")
 	}
@@ -176,6 +189,30 @@ func inspectPlanHook(
 		return nil, nil, false, err
 	}
 
+	if detached {
+		fn := func(ctx context.Context, resultsCh chan<- tree.Datums) error {
+			jobID := p.ExecCfg().JobRegistry.MakeJobID()
+			jr := makeInspectJobRecord(tree.AsString(stmt), jobID, run.checks, run.asOfTimestamp)
+			if _, err := p.ExecCfg().JobRegistry.CreateAdoptableJobWithTxn(
+				ctx, jr, jobID, p.InternalSQLTxn(),
+			); err != nil {
+				return err
+			}
+			var notice pgnotice.Notice
+			if p.ExtendedEvalContext().TxnImplicit {
+				notice = pgnotice.Newf("INSPECT job %d running in the background", jobID)
+			} else {
+				notice = pgnotice.Newf("INSPECT job %d queued; will start after the current transaction commits", jobID)
+			}
+			if err := p.SendClientNotice(ctx, notice, true /* immediateFlush */); err != nil {
+				return err
+			}
+			resultsCh <- tree.Datums{tree.NewDInt(tree.DInt(jobID))}
+			return nil
+		}
+		return fn, jobs.DetachedJobExecutionResultHeader, false, nil
+	}
+
 	fn := func(ctx context.Context, resultsCh chan<- tree.Datums) error {
 		// We create the job record in the planner's transaction to ensure that
 		// the job record creation happens transactionally.
@@ -186,6 +223,14 @@ func inspectPlanHook(
 			return err
 		}
 
+		if err = p.SendClientNotice(
+			ctx,
+			pgnotice.Newf("waiting for INSPECT job to complete: %s\nIf the statement is canceled, the job will continue in the background.", sj.ID()),
+			true, /* immediateFlush */
+		); err != nil {
+			return err
+		}
+
 		if err := sj.AwaitCompletion(ctx); err != nil {
 			return err
 		}
@@ -195,6 +240,27 @@ func inspectPlanHook(
 	return fn, jobs.InspectJobExecutionResultHeader, false, nil
 }
 
+func makeInspectJobRecord(
+	description string, jobID jobspb.JobID, checks []*jobspb.InspectDetails_Check, asOf hlc.Timestamp,
+) jobs.Record {
+	descIDs := catalog.DescriptorIDSet{}
+	for _, check := range checks {
+		descIDs.Add(check.TableID)
+	}
+	return jobs.Record{
+		JobID:       jobID,
+		Description: description,
+		Details: jobspb.InspectDetails{
+			Checks: checks,
+			AsOf:   asOf,
+		},
+		Progress:      jobspb.InspectProgress{},
+		CreatedBy:     nil,
+		Username:      username.NodeUserName(),
+		DescriptorIDs: descIDs.Ordered(),
+	}
+}
+
 func init() {
 	sql.AddPlanHook("inspect", inspectPlanHook, inspectTypeCheck)
 }

pkg/sql/logictest/testdata/logic_test/inspect

@@ -15,6 +15,10 @@ WHERE job_type = 'INSPECT'
 ORDER BY created DESC
 LIMIT 1
 
+# Make job adoption faster. This is needed to speed up DETACHED jobs.
+statement ok
+SET CLUSTER SETTING jobs.registry.interval.adopt = '500ms';
+
 statement ok
 CREATE TABLE foo (c1 INT, c2 INT, INDEX idx_c1 (c1), INDEX idx_c2 (c2));
 
@@ -27,6 +31,9 @@ INSPECT DATABASE test;
 statement ok
 SET enable_inspect_command = true;
 
+subtest transaction_behavior
+
+# Test that regular INSPECT cannot run within a transaction.
 statement ok
 BEGIN;
 
@@ -36,7 +43,77 @@ INSPECT TABLE foo;
 statement ok
 ROLLBACK;
 
+statement notice NOTICE: INSPECT job [0-9]+ running in the background
+INSPECT TABLE foo WITH OPTIONS DETACHED
+
+# Wait for the detached job to eventually succeed.
+query T retry
+SELECT status FROM last_inspect_job
+----
+succeeded
+
+# Test that DETACHED INSPECT can run inside a transaction.
+statement ok
+BEGIN;
+
+statement notice NOTICE: INSPECT job [0-9]+ queued; will start after the current transaction commits
+INSPECT TABLE foo WITH OPTIONS DETACHED
+
+# Verify the job isn't committed yet. It will stay in the running stage because we haven't committed it yet.
+query T
+SELECT status FROM last_inspect_job
+----
+running
+
 statement ok
+COMMIT;
+
+# Wait for the new job to eventually succeed.
+query T retry
+SELECT status FROM last_inspect_job
+----
+succeeded
+
+subtest end
+
+subtest inspect_options
+
+# Test DETACHED option validation.
+
+# Failure if both DETACHED and DETACHED = FALSE are specified.
+statement error pq: conflicting INSPECT options: DETACHED specified with different values
+INSPECT TABLE foo WITH OPTIONS DETACHED, DETACHED = FALSE
+
+# DETACHED = TRUE is the same as DETACHED.
+# Both should check all indexes (2 total: idx_c1 and idx_c2).
+statement notice NOTICE: INSPECT job [0-9]+ running in the background
+INSPECT TABLE foo WITH OPTIONS DETACHED = TRUE
+
+query TI retry
+SELECT * FROM last_inspect_job
+----
+succeeded  2
+
+statement notice NOTICE: INSPECT job [0-9]+ running in the background
+INSPECT TABLE foo WITH OPTIONS DETACHED
+
+query TI retry
+SELECT * FROM last_inspect_job
+----
+succeeded  2
+
+# DETACHED = TRUE with a specific index (verify the number of checks).
+statement notice NOTICE: INSPECT job [0-9]+ running in the background
+INSPECT TABLE foo WITH OPTIONS DETACHED = TRUE, INDEX (idx_c1)
+
+query TI retry
+SELECT * FROM last_inspect_job
+----
+succeeded  1
+
+# Test regular (non-detached) inspect operations.
+
+statement notice NOTICE: waiting for INSPECT job to complete: [0-9]+\nIf the statement is canceled, the job will continue in the background.
 INSPECT TABLE foo;
 
 query TI
@@ -123,6 +200,8 @@ SELECT * FROM last_inspect_job;
 ----
 succeeded  3
 
+subtest end
+
 subtest permissions
 
 statement ok

pkg/sql/parser/sql.y

@@ -7939,6 +7939,18 @@ inspect_option:
   {
     $$.val = &tree.InspectOptionIndex{IndexNames: $3.newTableIndexNames()}
   }
+| DETACHED
+  {
+    $$.val = &tree.InspectOptionDetached{Detached: *tree.MakeDBool(true)}
+  }
+| DETACHED '=' TRUE
+  {
+    $$.val = &tree.InspectOptionDetached{Detached: *tree.MakeDBool(true)}
+  }
+| DETACHED '=' FALSE
+  {
+    $$.val = &tree.InspectOptionDetached{Detached: *tree.MakeDBool(false)}
+  }
 
 // %Help: SHOW INSPECT ERRORS - list errors recorded by one INSPECT run
 // %Category: Misc

pkg/sql/parser/testdata/inspect

@@ -31,6 +31,14 @@ INSPECT DATABASE x AS OF SYSTEM TIME (1) WITH OPTIONS INDEX (index_name_1, index
 INSPECT DATABASE x AS OF SYSTEM TIME _ WITH OPTIONS INDEX (index_name_1, index_name_2) -- literals removed
 INSPECT DATABASE _ AS OF SYSTEM TIME 1 WITH OPTIONS INDEX (_, _) -- identifiers removed
 
+parse
+INSPECT DATABASE x WITH OPTIONS DETACHED, INDEX (index_name)
+----
+INSPECT DATABASE x WITH OPTIONS DETACHED, INDEX (index_name)
+INSPECT DATABASE x WITH OPTIONS DETACHED, INDEX (index_name) -- fully parenthesized
+INSPECT DATABASE x WITH OPTIONS DETACHED, INDEX (index_name) -- literals removed
+INSPECT DATABASE _ WITH OPTIONS DETACHED, INDEX (_) -- identifiers removed
+
 parse
 INSPECT TABLE x WITH OPTIONS INDEX (index_name)
 ----
@@ -79,6 +87,14 @@ INSPECT TABLE x WITH OPTIONS INDEX (index_name_1, index_name_2) -- fully parenth
 INSPECT TABLE x WITH OPTIONS INDEX (index_name_1, index_name_2) -- literals removed
 INSPECT TABLE _ WITH OPTIONS INDEX (_, _) -- identifiers removed
 
+parse
+INSPECT TABLE x WITH OPTIONS DETACHED
+----
+INSPECT TABLE x WITH OPTIONS DETACHED
+INSPECT TABLE x WITH OPTIONS DETACHED -- fully parenthesized
+INSPECT TABLE x WITH OPTIONS DETACHED -- literals removed
+INSPECT TABLE _ WITH OPTIONS DETACHED -- identifiers removed
+
 parse
 INSPECT TABLE db.schema.t1 WITH OPTIONS INDEX (index_name)
 ----

pkg/sql/sem/tree/inspect.go

@@ -134,6 +134,19 @@ func indexMatchesTable(index *TableIndexName, table TableName) bool {
 	return true
 }
 
+// IsDetached returns the value of the DETACHED option, if set.
+// Returns false if DETACHED is not specified.
+// Conflicts are handled by validate(), so this simply returns the first
+// DETACHED value found.
+func (n *InspectOptions) IsDetached() bool {
+	for _, option := range *n {
+		if opt, ok := option.(*InspectOptionDetached); ok {
+			return bool(opt.Detached)
+		}
+	}
+	return false
+}
+
 // HasIndexAll checks if the options include an INDEX ALL option.
 func (n *InspectOptions) HasIndexAll() bool {
 	for _, option := range *n {
@@ -144,6 +157,18 @@ func (n *InspectOptions) HasIndexAll() bool {
 	return false
 }
 
+// HasIndexOption checks if the options include an INDEX option with
+// specific index names (i.e., INDEX (name1, name2, ...)).
+// This does NOT include INDEX ALL - use HasIndexAll() for that.
+func (n *InspectOptions) HasIndexOption() bool {
+	for _, option := range *n {
+		if _, ok := option.(*InspectOptionIndex); ok {
+			return true
+		}
+	}
+	return false
+}
+
 // Validate checks for internal consistency of the INSPECT command.
 func (n *Inspect) Validate() error {
 	if err := n.Options.validate(); err != nil {
@@ -156,13 +181,23 @@ func (n *Inspect) Validate() error {
 // validate checks for internal consistency of options on the INSPECT command.
 func (n *InspectOptions) validate() error {
 	var hasOptionIndex, hasOptionIndexAll bool
+	var detachedSeen bool
+	var detachedVal bool
 
 	for _, option := range *n {
-		switch option.(type) {
+		switch opt := option.(type) {
 		case *InspectOptionIndex:
 			hasOptionIndex = true
 		case *InspectOptionIndexAll:
 			hasOptionIndexAll = true
+		case *InspectOptionDetached:
+			optVal := bool(opt.Detached)
+			if detachedSeen && optVal != detachedVal {
+				return pgerror.Newf(pgcode.Syntax,
+					"conflicting INSPECT options: DETACHED specified with different values")
+			}
+			detachedSeen = true
+			detachedVal = optVal
 		default:
 			return fmt.Errorf("unknown inspect option: %T", option)
 		}
@@ -224,3 +259,23 @@ type InspectOptionIndexAll struct{}
 func (n *InspectOptionIndexAll) Format(ctx *FmtCtx) {
 	ctx.WriteString("INDEX ALL")
 }
+
+// InspectOptionDetached keeps track of state for the DETACHED option.
+type InspectOptionDetached struct {
+	Detached DBool
+}
+
+// inspectOptionType implements InspectOption.
+func (*InspectOptionDetached) inspectOptionType() {}
+
+// Format implements the NodeFormatter interface.
+func (n *InspectOptionDetached) Format(ctx *FmtCtx) {
+	if bool(n.Detached) {
+		ctx.WriteString("DETACHED")
+		return
+	}
+	ctx.WriteString("DETACHED = false")
+}
+
+// String implements fmt.Stringer.
+func (n *InspectOptionDetached) String() string { return AsString(n) }