Skip to content

Commit 1f429d7

Browse files
spilchenspilchen
committed
sql/inspect: filter SHOW INSPECT ERRORS by job payload
Previously, SHOW INSPECT ERRORS FOR TABLE <table> would select the most recent completed INSPECT job that had errors for that table by joining with the inspect_errors table. This approach had a flaw: if a more recent job inspected the table but found no errors, it would be ignored in favor of an older job with errors. This changes that by query the INSPECT job payload to determine which jobs actually inspected a given table. Informs #148287 Release note: none Epic: CRDB-30356
1 parent 0c9c77f commit 1f429d7

File tree

2 files changed

+162
-20
lines changed

2 files changed

+162
-20
lines changed

‎pkg/sql/delegate/show_inspect_errors.go‎

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -57,25 +57,37 @@ func (d *delegator) delegateShowInspectErrors(n *tree.ShowInspectErrors) (tree.S
5757
query.WriteString(fmt.Sprintf(" AND id = %d", *n.JobID))
5858
}
5959

60-
// TODO(148287): query the inspect job payload to figure out if a job touches a particular table or database ID
61-
// If a table was specified, only consider jobs that reported errors on it.
62-
// If a job ID was specified, only consider that job. The records from the
63-
// most recent completed job that satisfies those criteria is used.
6460
query.WriteString(`),
65-
job_id AS (
66-
SELECT max(inspect_jobs.id) as id
61+
filtered_jobs AS (
62+
SELECT inspect_jobs.id
6763
FROM inspect_jobs
68-
JOIN crdb_internal.cluster_inspect_errors ie ON inspect_jobs.id = ie.job_id
69-
WHERE 1=1
64+
JOIN crdb_internal.system_jobs sj ON inspect_jobs.id = sj.id
7065
`)
66+
// If a table is specified, limit results to jobs that include checks
67+
// on that table. The table is matched by inspecting the job payload.
7168
if tableID != catid.InvalidDescID {
72-
query.WriteString(fmt.Sprintf(" AND ie.id = %d", tableID))
73-
}
74-
if n.JobID != nil {
75-
query.WriteString(fmt.Sprintf(" AND ie.job_id = %d", *n.JobID))
69+
query.WriteString(fmt.Sprintf(`
70+
WHERE EXISTS (
71+
SELECT 1
72+
FROM jsonb_array_elements(
73+
COALESCE(
74+
crdb_internal.pb_to_json('cockroach.sql.jobs.jobspb.Payload', sj.payload)
75+
-> 'inspectDetails' -> 'checks',
76+
'[]'::JSONB
77+
)
78+
) AS c
79+
WHERE (c ->> 'tableId')::INT = %d
80+
)`, tableID))
7681
}
82+
// Reports on a single job. If multiple match, use the most recent one.
83+
query.WriteString(`),
84+
job_id AS (
85+
SELECT max(id) AS id
86+
FROM filtered_jobs
87+
)
88+
`)
7789

78-
query.WriteString(`)
90+
query.WriteString(`
7991
SELECT
8092
ie.error_type,
8193
COALESCE(t.database_name, '<unknown>') AS database_name,

‎pkg/sql/logictest/testdata/logic_test/show_inspect_errors‎

Lines changed: 137 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,21 +40,48 @@ SELECT '2025-09-23-11:02:14-04:00'::timestamp
4040
statement ok
4141
INSERT INTO system.jobs (id, owner, status, job_type)
4242
VALUES (555, 'testuser', 'failed', 'INSPECT');
43-
INSERT INTO system.job_info (job_id, info_key)
44-
VALUES (555, 'legacy_payload');
43+
INSERT INTO system.job_info (job_id, info_key, value)
44+
VALUES (
45+
555,
46+
'legacy_payload',
47+
crdb_internal.json_to_pb(
48+
'cockroach.sql.jobs.jobspb.Payload',
49+
json_build_object(
50+
'inspectDetails', json_build_object(
51+
'checks', json_build_array(
52+
json_build_object('tableId', $foo_table_id),
53+
json_build_object('tableId', $bar_table_id)
54+
)
55+
)
56+
)
57+
)
58+
);
4559

4660
skipif config local-mixed-25.2 local-mixed-25.3
4761
statement ok
4862
INSERT INTO system.inspect_errors (job_id, error_type, aost, database_id, schema_id, id, details)
49-
VALUES
63+
VALUES
5064
(555, '555_foo', '$aost', $database_id, $schema_id, $foo_table_id, '{"detail":"555\"foo"}'),
5165
(555, '555_bar', '$aost', $database_id, $schema_id, $bar_table_id, '{"detail":"555\"bar"}');
5266

5367
statement ok
5468
INSERT INTO system.jobs (id, owner, status, job_type)
5569
VALUES (666, 'testuser', 'failed', 'INSPECT');
56-
INSERT INTO system.job_info (job_id, info_key)
57-
VALUES (666, 'legacy_payload');
70+
INSERT INTO system.job_info (job_id, info_key, value)
71+
VALUES (
72+
666,
73+
'legacy_payload',
74+
crdb_internal.json_to_pb(
75+
'cockroach.sql.jobs.jobspb.Payload',
76+
json_build_object(
77+
'inspectDetails', json_build_object(
78+
'checks', json_build_array(
79+
json_build_object('tableId', $foo_table_id)
80+
)
81+
)
82+
)
83+
)
84+
);
5885

5986
skipif config local-mixed-25.2 local-mixed-25.3
6087
statement ok
@@ -64,8 +91,21 @@ VALUES (666, '666_foo', '$aost', $database_id, $schema_id, $foo_table_id, '{"det
6491
statement ok
6592
INSERT INTO system.jobs (id, owner, status, job_type)
6693
VALUES (777, 'testuser', 'running', 'INSPECT');
67-
INSERT INTO system.job_info (job_id, info_key)
68-
VALUES (777, 'legacy_payload');
94+
INSERT INTO system.job_info (job_id, info_key, value)
95+
VALUES (
96+
777,
97+
'legacy_payload',
98+
crdb_internal.json_to_pb(
99+
'cockroach.sql.jobs.jobspb.Payload',
100+
json_build_object(
101+
'inspectDetails', json_build_object(
102+
'checks', json_build_array(
103+
json_build_object('tableId', $foo_table_id)
104+
)
105+
)
106+
)
107+
)
108+
);
69109

70110
skipif config local-mixed-25.2 local-mixed-25.3
71111
statement ok
@@ -140,3 +180,93 @@ error_type database_name schema_name table_name primary_key job_id aost
140180
555_foo test public foo NULL 555 2025-09-23 04:00:00.000000 {
141181
"detail": "555\"foo"
142182
}
183+
184+
# Test SHOW selects the most recent job for a table even if it has no errors.
185+
subtest show_after_clean_inspect
186+
187+
user root
188+
189+
# Job 888 inspects foo (payload check) but finds no errors (no inspect_errors
190+
# records).
191+
statement ok
192+
INSERT INTO system.jobs (id, owner, status, job_type)
193+
VALUES (888, 'testuser', 'succeeded', 'INSPECT');
194+
INSERT INTO system.job_info (job_id, info_key, value)
195+
VALUES (
196+
888,
197+
'legacy_payload',
198+
crdb_internal.json_to_pb(
199+
'cockroach.sql.jobs.jobspb.Payload',
200+
json_build_object(
201+
'inspectDetails', json_build_object(
202+
'checks', json_build_array(
203+
json_build_object('tableId', $foo_table_id)
204+
)
205+
)
206+
)
207+
)
208+
);
209+
210+
# No errors inserted for job 888 - it inspected foo but found no problems.
211+
212+
user testuser
213+
214+
# Now query for foo errors without specifying a job.
215+
# Even though job 666 has errors for foo, job 888 is the most recent completed job
216+
# that inspected foo, so we should get no results.
217+
skipif config local-mixed-25.2 local-mixed-25.3
218+
query TTTTTIT
219+
SHOW INSPECT ERRORS FOR TABLE foo
220+
----
221+
222+
# Verify we can still get job 666's errors by specifying the job explicitly.
223+
skipif config local-mixed-25.2 local-mixed-25.3
224+
query TTTTTIT
225+
SHOW INSPECT ERRORS FOR TABLE foo FOR JOB 666
226+
----
227+
666_foo test public foo NULL 666 2025-09-23 04:00:00.000000
228+
229+
subtest end
230+
231+
# Verify the SHOW query works if an INSPECT job is created with empty checks.
232+
subtest empty_checks_array
233+
234+
user root
235+
236+
# Job 999 has an empty checks array in its payload.
237+
statement ok
238+
INSERT INTO system.jobs (id, owner, status, job_type)
239+
VALUES (999, 'testuser', 'succeeded', 'INSPECT');
240+
INSERT INTO system.job_info (job_id, info_key, value)
241+
VALUES (
242+
999,
243+
'legacy_payload',
244+
crdb_internal.json_to_pb(
245+
'cockroach.sql.jobs.jobspb.Payload',
246+
json_build_object(
247+
'inspectDetails', json_build_object(
248+
'checks', json_build_array()
249+
)
250+
)
251+
)
252+
);
253+
254+
user testuser
255+
256+
skipif config local-mixed-25.2 local-mixed-25.3
257+
query TTTTTIT
258+
SHOW INSPECT ERRORS FOR TABLE foo
259+
----
260+
261+
skipif config local-mixed-25.2 local-mixed-25.3
262+
query TTTTTIT
263+
SHOW INSPECT ERRORS FOR TABLE bar
264+
----
265+
555_bar test public bar NULL 555 2025-09-23 04:00:00.000000
266+
267+
skipif config local-mixed-25.2 local-mixed-25.3
268+
query TTTTTIT
269+
SHOW INSPECT ERRORS FOR JOB 999
270+
----
271+
272+
subtest end

0 commit comments

Comments
 (0)