Skip to content

Commit d70b176

Browse files
tvondratvondra
committed
amcheck: Move common routines into a separate module
Before performing checks on an index, we need to take some safety measures that apply to all index AMs. This includes: * verifying that the index can be checked - Only selected AMs are supported by amcheck (right now only B-Tree). The index has to be valid and not a temporary index from another session. * changing (and then restoring) user's security context * obtaining proper locks on the index (and table, if needed) * discarding GUC changes from the index functions Until now this was implemented in the B-Tree amcheck module, but it's something every AM will have to do. So relocate the code into a new module verify_common for reuse. The shared steps are implemented by amcheck_lock_relation_and_check(), receiving the AM-specific verification as a callback. Custom parameters may be supplied using a pointer. Author: Andrey Borodin <amborodin@acm.org> Reviewed-By: José Villanova <jose.arthur@gmail.com> Reviewed-By: Aleksander Alekseev <aleksander@timescale.com> Reviewed-By: Nikolay Samokhvalov <samokhvalov@gmail.com> Reviewed-By: Andres Freund <andres@anarazel.de> Reviewed-By: Tomas Vondra <tomas@vondra.me> Reviewed-By: Mark Dilger <mark.dilger@enterprisedb.com> Reviewed-By: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Kirill Reshke <reshkekirill@gmail.com> Discussion: https://postgr.es/m/45AC9B0A-2B45-40EE-B08F-BDCF5739D1E1%40yandex-team.ru
1 parent fb9dff7 commit d70b176

File tree

7 files changed

+297
-199
lines changed

7 files changed

+297
-199
lines changed

‎contrib/amcheck/Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
MODULE_big = amcheck
44
OBJS = \
55
$(WIN32RES) \
6+
verify_common.o \
67
verify_heapam.o \
78
verify_nbtree.o
89

‎contrib/amcheck/expected/check_btree.out

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ ERROR: could not open relation with OID 17
5757
BEGIN;
5858
CREATE INDEX bttest_a_brin_idx ON bttest_a USING brin(id);
5959
SELECT bt_index_parent_check('bttest_a_brin_idx');
60-
ERROR: only B-Tree indexes are supported as targets for verification
61-
DETAIL: Relation "bttest_a_brin_idx" is not a B-Tree index.
60+
ERROR: expected "btree" index as targets for verification
61+
DETAIL: Relation "bttest_a_brin_idx" is a brin index.
6262
ROLLBACK;
6363
-- normal check outside of xact
6464
SELECT bt_index_check('bttest_a_idx');

‎contrib/amcheck/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Copyright (c) 2022-2025, PostgreSQL Global Development Group
22

33
amcheck_sources = files(
4+
'verify_common.c',
45
'verify_heapam.c',
56
'verify_nbtree.c',
67
)

‎contrib/amcheck/verify_common.c

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
/*-------------------------------------------------------------------------
2+
*
3+
* verify_common.c
4+
* Utility functions common to all access methods.
5+
*
6+
* Copyright (c) 2016-2025, PostgreSQL Global Development Group
7+
*
8+
* IDENTIFICATION
9+
* contrib/amcheck/verify_common.c
10+
*
11+
*-------------------------------------------------------------------------
12+
*/
13+
#include "postgres.h"
14+
15+
#include "access/genam.h"
16+
#include "access/table.h"
17+
#include "access/tableam.h"
18+
#include "verify_common.h"
19+
#include "catalog/index.h"
20+
#include "catalog/pg_am.h"
21+
#include "commands/tablecmds.h"
22+
#include "utils/guc.h"
23+
#include "utils/syscache.h"
24+
25+
static bool amcheck_index_mainfork_expected(Relation rel);
26+
27+
28+
/*
29+
* Check if index relation should have a file for its main relation fork.
30+
* Verification uses this to skip unlogged indexes when in hot standby mode,
31+
* where there is simply nothing to verify.
32+
*
33+
* NB: Caller should call index_checkable() before calling here.
34+
*/
35+
static bool
36+
amcheck_index_mainfork_expected(Relation rel)
37+
{
38+
if (rel->rd_rel->relpersistence != RELPERSISTENCE_UNLOGGED ||
39+
!RecoveryInProgress())
40+
return true;
41+
42+
ereport(NOTICE,
43+
(errcode(ERRCODE_READ_ONLY_SQL_TRANSACTION),
44+
errmsg("cannot verify unlogged index \"%s\" during recovery, skipping",
45+
RelationGetRelationName(rel))));
46+
47+
return false;
48+
}
49+
50+
/*
51+
* Amcheck main workhorse.
52+
* Given index relation OID, lock relation.
53+
* Next, take a number of standard actions:
54+
* 1) Make sure the index can be checked
55+
* 2) change the context of the user,
56+
* 3) keep track of GUCs modified via index functions
57+
* 4) execute callback function to verify integrity.
58+
*/
59+
void
60+
amcheck_lock_relation_and_check(Oid indrelid,
61+
Oid am_id,
62+
IndexDoCheckCallback check,
63+
LOCKMODE lockmode,
64+
void *state)
65+
{
66+
Oid heapid;
67+
Relation indrel;
68+
Relation heaprel;
69+
Oid save_userid;
70+
int save_sec_context;
71+
int save_nestlevel;
72+
73+
/*
74+
* We must lock table before index to avoid deadlocks. However, if the
75+
* passed indrelid isn't an index then IndexGetRelation() will fail.
76+
* Rather than emitting a not-very-helpful error message, postpone
77+
* complaining, expecting that the is-it-an-index test below will fail.
78+
*
79+
* In hot standby mode this will raise an error when parentcheck is true.
80+
*/
81+
heapid = IndexGetRelation(indrelid, true);
82+
if (OidIsValid(heapid))
83+
{
84+
heaprel = table_open(heapid, lockmode);
85+
86+
/*
87+
* Switch to the table owner's userid, so that any index functions are
88+
* run as that user. Also lock down security-restricted operations
89+
* and arrange to make GUC variable changes local to this command.
90+
*/
91+
GetUserIdAndSecContext(&save_userid, &save_sec_context);
92+
SetUserIdAndSecContext(heaprel->rd_rel->relowner,
93+
save_sec_context | SECURITY_RESTRICTED_OPERATION);
94+
save_nestlevel = NewGUCNestLevel();
95+
}
96+
else
97+
{
98+
heaprel = NULL;
99+
/* Set these just to suppress "uninitialized variable" warnings */
100+
save_userid = InvalidOid;
101+
save_sec_context = -1;
102+
save_nestlevel = -1;
103+
}
104+
105+
/*
106+
* Open the target index relations separately (like relation_openrv(), but
107+
* with heap relation locked first to prevent deadlocking). In hot
108+
* standby mode this will raise an error when parentcheck is true.
109+
*
110+
* There is no need for the usual indcheckxmin usability horizon test
111+
* here, even in the heapallindexed case, because index undergoing
112+
* verification only needs to have entries for a new transaction snapshot.
113+
* (If this is a parentcheck verification, there is no question about
114+
* committed or recently dead heap tuples lacking index entries due to
115+
* concurrent activity.)
116+
*/
117+
indrel = index_open(indrelid, lockmode);
118+
119+
/*
120+
* Since we did the IndexGetRelation call above without any lock, it's
121+
* barely possible that a race against an index drop/recreation could have
122+
* netted us the wrong table.
123+
*/
124+
if (heaprel == NULL || heapid != IndexGetRelation(indrelid, false))
125+
ereport(ERROR,
126+
(errcode(ERRCODE_UNDEFINED_TABLE),
127+
errmsg("could not open parent table of index \"%s\"",
128+
RelationGetRelationName(indrel))));
129+
130+
/* Check that relation suitable for checking */
131+
if (index_checkable(indrel, am_id))
132+
check(indrel, heaprel, state, lockmode == ShareLock);
133+
134+
/* Roll back any GUC changes executed by index functions */
135+
AtEOXact_GUC(false, save_nestlevel);
136+
137+
/* Restore userid and security context */
138+
SetUserIdAndSecContext(save_userid, save_sec_context);
139+
140+
/*
141+
* Release locks early. That's ok here because nothing in the called
142+
* routines will trigger shared cache invalidations to be sent, so we can
143+
* relax the usual pattern of only releasing locks after commit.
144+
*/
145+
index_close(indrel, lockmode);
146+
if (heaprel)
147+
table_close(heaprel, lockmode);
148+
}
149+
150+
/*
151+
* Basic checks about the suitability of a relation for checking as an index.
152+
*
153+
*
154+
* NB: Intentionally not checking permissions, the function is normally not
155+
* callable by non-superusers. If granted, it's useful to be able to check a
156+
* whole cluster.
157+
*/
158+
bool
159+
index_checkable(Relation rel, Oid am_id)
160+
{
161+
if (rel->rd_rel->relkind != RELKIND_INDEX ||
162+
rel->rd_rel->relam != am_id)
163+
{
164+
HeapTuple amtup;
165+
HeapTuple amtuprel;
166+
167+
amtup = SearchSysCache1(AMOID, ObjectIdGetDatum(am_id));
168+
amtuprel = SearchSysCache1(AMOID, ObjectIdGetDatum(rel->rd_rel->relam));
169+
ereport(ERROR,
170+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
171+
errmsg("expected \"%s\" index as targets for verification", NameStr(((Form_pg_am) GETSTRUCT(amtup))->amname)),
172+
errdetail("Relation \"%s\" is a %s index.",
173+
RelationGetRelationName(rel), NameStr(((Form_pg_am) GETSTRUCT(amtuprel))->amname))));
174+
}
175+
176+
if (RELATION_IS_OTHER_TEMP(rel))
177+
ereport(ERROR,
178+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
179+
errmsg("cannot access temporary tables of other sessions"),
180+
errdetail("Index \"%s\" is associated with temporary relation.",
181+
RelationGetRelationName(rel))));
182+
183+
if (!rel->rd_index->indisvalid)
184+
ereport(ERROR,
185+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
186+
errmsg("cannot check index \"%s\"",
187+
RelationGetRelationName(rel)),
188+
errdetail("Index is not valid.")));
189+
190+
return amcheck_index_mainfork_expected(rel);
191+
}

‎contrib/amcheck/verify_common.h

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*-------------------------------------------------------------------------
2+
*
3+
* amcheck.h
4+
* Shared routines for amcheck verifications.
5+
*
6+
* Copyright (c) 2016-2025, PostgreSQL Global Development Group
7+
*
8+
* IDENTIFICATION
9+
* contrib/amcheck/amcheck.h
10+
*
11+
*-------------------------------------------------------------------------
12+
*/
13+
#include "storage/bufpage.h"
14+
#include "storage/lmgr.h"
15+
#include "storage/lockdefs.h"
16+
#include "utils/relcache.h"
17+
#include "miscadmin.h"
18+
19+
/* Typedefs for callback functions for amcheck_lock_relation */
20+
typedef void (*IndexCheckableCallback) (Relation index);
21+
typedef void (*IndexDoCheckCallback) (Relation rel,
22+
Relation heaprel,
23+
void *state,
24+
bool readonly);
25+
26+
extern void amcheck_lock_relation_and_check(Oid indrelid,
27+
Oid am_id,
28+
IndexDoCheckCallback check,
29+
LOCKMODE lockmode, void *state);
30+
31+
extern bool index_checkable(Relation rel, Oid am_id);

0 commit comments

Comments
 (0)