Skip to content

Commit b31a0fe

Browse files
committed
sql/inspect: support AS OF SYSTEM TIME for historical descriptor access
Prior to this change, INSPECT could fail late during execution when running AS OF SYSTEM TIME queries that referenced objects which did not exist at the specified timestamp. For example, querying a database that had not yet been created would trigger an internal error instead of a clear user facing message. This change updates INSPECT’s planning logic to detect such cases earlier and return a more informative error (e.g. “database movr does not exist”). It does this by using a fixed timestamp and avoiding leased descriptors for ASOF queries. Informs: #155483 Epic: CRDB-55075 Release note: none
1 parent 54a9a86 commit b31a0fe

File tree

4 files changed

+77
-15
lines changed

4 files changed

+77
-15
lines changed

pkg/sql/exec_util.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2525,6 +2525,11 @@ func (p *planner) isAsOf(ctx context.Context, stmt tree.Statement) (*eval.AsOfSy
25252525
}
25262526
asOf = s.AsOf
25272527
forBackfill = true
2528+
case *tree.Inspect:
2529+
if s.AsOf.Expr == nil {
2530+
return nil, nil
2531+
}
2532+
asOf = s.AsOf
25282533
default:
25292534
return nil, nil
25302535
}

pkg/sql/inspect/inspect_plan.go

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,20 @@ func newInspectRun(
6060
) (inspectRun, error) {
6161
var run inspectRun
6262

63+
avoidLeased := false
64+
if aost := p.ExtendedEvalContext().AsOfSystemTime; aost != nil {
65+
avoidLeased = true
66+
}
67+
68+
if stmt.AsOf.Expr != nil {
69+
asOf, err := p.EvalAsOfTimestamp(ctx, stmt.AsOf)
70+
if err != nil {
71+
return inspectRun{}, err
72+
}
73+
run.asOfTimestamp = asOf.Timestamp
74+
avoidLeased = true
75+
}
76+
6377
switch stmt.Typ {
6478
case tree.InspectTable:
6579
if table, err := p.ResolveExistingObjectEx(ctx, stmt.Table, true /* required */, tree.ResolveRequireTableDesc); err != nil {
@@ -68,13 +82,21 @@ func newInspectRun(
6882
run.table = table
6983
}
7084

71-
if db, err := p.Descriptors().ByIDWithLeased(p.Txn()).Get().Database(ctx, run.table.GetParentID()); err != nil {
85+
dbGetter := p.Descriptors().ByIDWithLeased(p.Txn())
86+
if avoidLeased {
87+
dbGetter = p.Descriptors().ByIDWithoutLeased(p.Txn())
88+
}
89+
if db, err := dbGetter.Get().Database(ctx, run.table.GetParentID()); err != nil {
7290
return inspectRun{}, err
7391
} else {
7492
run.db = db
7593
}
7694
case tree.InspectDatabase:
77-
if db, err := p.Descriptors().ByNameWithLeased(p.Txn()).Get().Database(ctx, stmt.Database.ToUnresolvedName().String()); err != nil {
95+
dbGetter := p.Descriptors().ByNameWithLeased(p.Txn())
96+
if avoidLeased {
97+
dbGetter = p.Descriptors().ByName(p.Txn())
98+
}
99+
if db, err := dbGetter.Get().Database(ctx, stmt.Database.ToUnresolvedName().String()); err != nil {
78100
return inspectRun{}, err
79101
} else {
80102
run.db = db
@@ -105,7 +127,11 @@ func newInspectRun(
105127
// Named indexes specified.
106128
switch stmt.Typ {
107129
case tree.InspectTable:
108-
schema, err := p.Descriptors().ByIDWithLeased(p.Txn()).Get().Schema(ctx, run.table.GetParentSchemaID())
130+
schemaGetter := p.Descriptors().ByIDWithLeased(p.Txn())
131+
if avoidLeased {
132+
schemaGetter = p.Descriptors().ByIDWithoutLeased(p.Txn())
133+
}
134+
schema, err := schemaGetter.Get().Schema(ctx, run.table.GetParentSchemaID())
109135
if err != nil {
110136
return inspectRun{}, err
111137
}
@@ -133,14 +159,6 @@ func newInspectRun(
133159
}
134160
}
135161

136-
if stmt.AsOf.Expr != nil {
137-
asOf, err := p.EvalAsOfTimestamp(ctx, stmt.AsOf)
138-
if err != nil {
139-
return inspectRun{}, err
140-
}
141-
run.asOfTimestamp = asOf.Timestamp
142-
}
143-
144162
return run, nil
145163
}
146164

pkg/sql/inspect_job.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,15 @@ func TriggerInspectJob(
104104
func InspectChecksForDatabase(
105105
ctx context.Context, p PlanHookState, db catalog.DatabaseDescriptor,
106106
) ([]*jobspb.InspectDetails_Check, error) {
107-
tables, err := p.Descriptors().ByNameWithLeased(p.Txn()).Get().GetAllTablesInDatabase(ctx, p.Txn(), db)
107+
avoidLeased := false
108+
if aost := p.ExtendedEvalContext().AsOfSystemTime; aost != nil {
109+
avoidLeased = true
110+
}
111+
byNameGetter := p.Descriptors().ByNameWithLeased(p.Txn())
112+
if avoidLeased {
113+
byNameGetter = p.Descriptors().ByName(p.Txn())
114+
}
115+
tables, err := byNameGetter.Get().GetAllTablesInDatabase(ctx, p.Txn(), db)
108116
if err != nil {
109117
return nil, err
110118
}

pkg/sql/logictest/testdata/logic_test/inspect

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,18 @@
55
# LogicTest: !fakedist !fakedist-vec-off !fakedist-disk !local-mixed-25.2 !local-mixed-25.3
66

77

8+
# Helper view to show the most recent INSPECT job.
9+
# We order by job ID (not creation time) because jobs run with AS OF SYSTEM
10+
# TIME use the AOST value as their creation timestamp rather than the current
11+
# time.
812
statement ok
913
CREATE VIEW last_inspect_job AS
1014
SELECT status, jsonb_array_length(
1115
crdb_internal.pb_to_json('cockroach.sql.jobs.jobspb.Payload', payload)->'inspectDetails'->'checks'
1216
) AS num_checks
1317
FROM crdb_internal.system_jobs
1418
WHERE job_type = 'INSPECT'
15-
ORDER BY created DESC
19+
ORDER BY id DESC
1620
LIMIT 1
1721

1822
# Make job adoption faster. This is needed to speed up DETACHED jobs.
@@ -22,6 +26,9 @@ SET CLUSTER SETTING jobs.registry.interval.adopt = '500ms';
2226
statement ok
2327
CREATE TABLE foo (c1 INT, c2 INT, INDEX idx_c1 (c1), INDEX idx_c2 (c2));
2428

29+
let $foo_created_ts
30+
SELECT now()
31+
2532
statement error pq: INSPECT is an experimental feature and is disabled by default
2633
INSPECT TABLE foo;
2734

@@ -138,9 +145,8 @@ SELECT * FROM last_inspect_job;
138145
----
139146
succeeded 1
140147

141-
# TODO(155483): use a really old timestamp from before 'foo' was created.
142148
statement ok
143-
INSPECT TABLE foo AS OF SYSTEM TIME '-50ms' WITH OPTIONS INDEX (idx_c1,idx_c2);
149+
INSPECT TABLE foo AS OF SYSTEM TIME '$foo_created_ts' WITH OPTIONS INDEX (idx_c1,idx_c2);
144150

145151
query TI
146152
SELECT * FROM last_inspect_job;
@@ -533,3 +539,28 @@ statement ok
533539
DROP TABLE refcursor_tbl;
534540

535541
subtest end
542+
543+
subtest inspect_old_aost
544+
545+
let $inspect_old_ts
546+
SELECT now()
547+
548+
statement ok
549+
CREATE TABLE t_aost (c1 INT, INDEX idx_c1 (c1));
550+
551+
statement ok
552+
INSERT INTO t_aost VALUES (1), (2), (3);
553+
554+
# Test INSPECT with an AOST that predates the table.
555+
# This should fail because the table descriptor doesn't exist at that timestamp.
556+
statement error pq: relation "t_aost" does not exist
557+
INSPECT TABLE t_aost AS OF SYSTEM TIME '$inspect_old_ts' WITH OPTIONS INDEX (idx_c1);
558+
559+
# Test INSPECT with an AOST that predates the database.
560+
statement error pq: database "test" does not exist
561+
INSPECT TABLE t_aost AS OF SYSTEM TIME '1' WITH OPTIONS INDEX (idx_c1);
562+
563+
statement ok
564+
DROP TABLE t_aost;
565+
566+
subtest end

0 commit comments

Comments
 (0)