mirror of
https://github.com/postgres/postgres.git
synced 2025-06-14 18:42:34 +03:00
Fix assorted bugs in CREATE/DROP INDEX CONCURRENTLY.
Commit 8cb53654db
, which introduced DROP
INDEX CONCURRENTLY, managed to break CREATE INDEX CONCURRENTLY via a poor
choice of catalog state representation. The pg_index state for an index
that's reached the final pre-drop stage was the same as the state for an
index just created by CREATE INDEX CONCURRENTLY. This meant that the
(necessary) change to make RelationGetIndexList ignore about-to-die indexes
also made it ignore freshly-created indexes; which is catastrophic because
the latter do need to be considered in HOT-safety decisions. Failure to
do so leads to incorrect index entries and subsequently wrong results from
queries depending on the concurrently-created index.
To fix, add an additional boolean column "indislive" to pg_index, so that
the freshly-created and about-to-die states can be distinguished. (This
change obviously is only possible in HEAD. This patch will need to be
back-patched, but in 9.2 we'll use a kluge consisting of overloading the
formerly-impossible state of indisvalid = true and indisready = false.)
In addition, change CREATE/DROP INDEX CONCURRENTLY so that the pg_index
flag changes they make without exclusive lock on the index are made via
heap_inplace_update() rather than a normal transactional update. The
latter is not very safe because moving the pg_index tuple could result in
concurrent SnapshotNow scans finding it twice or not at all, thus possibly
resulting in index corruption. This is a pre-existing bug in CREATE INDEX
CONCURRENTLY, which was copied into the DROP code.
In addition, fix various places in the code that ought to check to make
sure that the indexes they are manipulating are valid and/or ready as
appropriate. These represent bugs that have existed since 8.2, since
a failed CREATE INDEX CONCURRENTLY could leave a corrupt or invalid
index behind, and we ought not try to do anything that might fail with
such an index.
Also fix RelationReloadIndexInfo to ensure it copies all the pg_index
columns that are allowed to change after initial creation. Previously we
could have been left with stale values of some fields in an index relcache
entry. It's not clear whether this actually had any user-visible
consequences, but it's at least a bug waiting to happen.
In addition, do some code and docs review for DROP INDEX CONCURRENTLY;
some cosmetic code cleanup but mostly addition and revision of comments.
This will need to be back-patched, but in a noticeably different form,
so I'm committing it to HEAD before working on the back-patch.
Problem reported by Amit Kapila, diagnosis by Pavan Deolassee,
fix by Tom Lane and Andres Freund.
This commit is contained in:
35
src/backend/utils/cache/relcache.c
vendored
35
src/backend/utils/cache/relcache.c
vendored
@ -1731,9 +1731,23 @@ RelationReloadIndexInfo(Relation relation)
|
||||
RelationGetRelid(relation));
|
||||
index = (Form_pg_index) GETSTRUCT(tuple);
|
||||
|
||||
/*
|
||||
* Basically, let's just copy all the bool fields. There are one or
|
||||
* two of these that can't actually change in the current code, but
|
||||
* it's not worth it to track exactly which ones they are. None of
|
||||
* the array fields are allowed to change, though.
|
||||
*/
|
||||
relation->rd_index->indisunique = index->indisunique;
|
||||
relation->rd_index->indisprimary = index->indisprimary;
|
||||
relation->rd_index->indisexclusion = index->indisexclusion;
|
||||
relation->rd_index->indimmediate = index->indimmediate;
|
||||
relation->rd_index->indisclustered = index->indisclustered;
|
||||
relation->rd_index->indisvalid = index->indisvalid;
|
||||
relation->rd_index->indcheckxmin = index->indcheckxmin;
|
||||
relation->rd_index->indisready = index->indisready;
|
||||
relation->rd_index->indislive = index->indislive;
|
||||
|
||||
/* Copy xmin too, as that is needed to make sense of indcheckxmin */
|
||||
HeapTupleHeaderSetXmin(relation->rd_indextuple->t_data,
|
||||
HeapTupleHeaderGetXmin(tuple->t_data));
|
||||
|
||||
@ -3299,6 +3313,10 @@ CheckConstraintFetch(Relation relation)
|
||||
* so that we must recompute the index list on next request. This handles
|
||||
* creation or deletion of an index.
|
||||
*
|
||||
* Indexes that are marked not IndexIsLive are omitted from the returned list.
|
||||
* Such indexes are expected to be dropped momentarily, and should not be
|
||||
* touched at all by any caller of this function.
|
||||
*
|
||||
* The returned list is guaranteed to be sorted in order by OID. This is
|
||||
* needed by the executor, since for index types that we obtain exclusive
|
||||
* locks on when updating the index, all backends must lock the indexes in
|
||||
@ -3358,9 +3376,12 @@ RelationGetIndexList(Relation relation)
|
||||
bool isnull;
|
||||
|
||||
/*
|
||||
* Ignore any indexes that are currently being dropped
|
||||
* Ignore any indexes that are currently being dropped. This will
|
||||
* prevent them from being searched, inserted into, or considered in
|
||||
* HOT-safety decisions. It's unsafe to touch such an index at all
|
||||
* since its catalog entries could disappear at any instant.
|
||||
*/
|
||||
if (!index->indisvalid && !index->indisready)
|
||||
if (!IndexIsLive(index))
|
||||
continue;
|
||||
|
||||
/* Add index's OID to result list in the proper order */
|
||||
@ -3379,7 +3400,8 @@ RelationGetIndexList(Relation relation)
|
||||
indclass = (oidvector *) DatumGetPointer(indclassDatum);
|
||||
|
||||
/* Check to see if it is a unique, non-partial btree index on OID */
|
||||
if (index->indnatts == 1 &&
|
||||
if (IndexIsValid(index) &&
|
||||
index->indnatts == 1 &&
|
||||
index->indisunique && index->indimmediate &&
|
||||
index->indkey.values[0] == ObjectIdAttributeNumber &&
|
||||
indclass->values[0] == OID_BTREE_OPS_OID &&
|
||||
@ -3674,6 +3696,13 @@ RelationGetIndexAttrBitmap(Relation relation)
|
||||
|
||||
/*
|
||||
* For each index, add referenced attributes to indexattrs.
|
||||
*
|
||||
* Note: we consider all indexes returned by RelationGetIndexList, even if
|
||||
* they are not indisready or indisvalid. This is important because an
|
||||
* index for which CREATE INDEX CONCURRENTLY has just started must be
|
||||
* included in HOT-safety decisions (see README.HOT). If a DROP INDEX
|
||||
* CONCURRENTLY is far enough along that we should ignore the index, it
|
||||
* won't be returned at all by RelationGetIndexList.
|
||||
*/
|
||||
indexattrs = NULL;
|
||||
foreach(l, indexoidlist)
|
||||
|
Reference in New Issue
Block a user