Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit3dbb317

Browse files
committed
Fix potential assertion failure when reindexing a pg_class index.
When reindexing individual indexes on pg_class it was possible toeither trigger an assertion failure:TRAP: FailedAssertion("!(!ReindexIsProcessingIndex(((index)->rd_id)))That's because reindex_index() called SetReindexProcessing() - whichenables an asserts ensuring no index insertions happen into the index- before calling RelationSetNewRelfilenode(). That not correct forindexes on pg_class, because RelationSetNewRelfilenode() updates therelevant pg_class row, which needs to update the indexes.The are two reasons this wasn't noticed earlier. Firstly the bugdoesn't trigger when reindexing all of pg_class, as reindex_relationhas code "hiding" all yet-to-be-reindexed indexes. Secondly, the bugonly triggers when the the update to pg_class doesn't turn out to be aHOT update - otherwise there's no index insertion to trigger thebug. Most of the time there's enough space, making this bug hard totrigger.To fix, move RelationSetNewRelfilenode() to before theSetReindexProcessing() (and, together with some other code, to outsideof the PG_TRY()).To make sure the error checking intended by SetReindexProcessing() ismore robust, modify CatalogIndexInsert() to checkReindexIsProcessingIndex() even when the update is a HOT update.Also add a few regression tests for REINDEXing of system catalogs.The last two improvements would have prevented some of the issuesfixed in5c15606 from being introduced in the first place.Reported-By: Michael PaquierDiagnosed-By: Tom Lane and Andres FreundAuthor: Andres FreundReviewed-By: Tom LaneDiscussion:https://postgr.es/m/20190418011430.GA19133@paquier.xyzBackpatch: 9.4-, the bug is present in all branches
1 parent5c15606 commit3dbb317

File tree

6 files changed

+90
-19
lines changed

6 files changed

+90
-19
lines changed

‎contrib/test_decoding/expected/rewrite.out

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,10 @@ COMMIT;
103103
-- repeated rewrites in different transactions
104104
VACUUM FULL pg_class;
105105
VACUUM FULL pg_class;
106+
-- reindexing of important relations / indexes
107+
REINDEX TABLE pg_class;
108+
REINDEX INDEX pg_class_oid_index;
109+
REINDEX INDEX pg_class_tblspc_relfilenode_index;
106110
INSERT INTO replication_example(somedata, testcolumn1) VALUES (5, 3);
107111
BEGIN;
108112
INSERT INTO replication_example(somedata, testcolumn1) VALUES (6, 4);

‎contrib/test_decoding/sql/rewrite.sql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ COMMIT;
7474
VACUUM FULL pg_class;
7575
VACUUM FULL pg_class;
7676

77+
-- reindexing of important relations / indexes
78+
REINDEX TABLE pg_class;
79+
REINDEX INDEX pg_class_oid_index;
80+
REINDEX INDEX pg_class_tblspc_relfilenode_index;
81+
7782
INSERT INTO replication_example(somedata, testcolumn1)VALUES (5,3);
7883

7984
BEGIN;

‎src/backend/catalog/index.c

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3316,28 +3316,34 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
33163316
*/
33173317
TransferPredicateLocksToHeapRelation(iRel);
33183318

3319+
/* Fetch info needed for index_build */
3320+
indexInfo=BuildIndexInfo(iRel);
3321+
3322+
/* If requested, skip checking uniqueness/exclusion constraints */
3323+
if (skip_constraint_checks)
3324+
{
3325+
if (indexInfo->ii_Unique||indexInfo->ii_ExclusionOps!=NULL)
3326+
skipped_constraint= true;
3327+
indexInfo->ii_Unique= false;
3328+
indexInfo->ii_ExclusionOps=NULL;
3329+
indexInfo->ii_ExclusionProcs=NULL;
3330+
indexInfo->ii_ExclusionStrats=NULL;
3331+
}
3332+
3333+
/*
3334+
* Build a new physical relation for the index. Need to do that before
3335+
* "officially" starting the reindexing with SetReindexProcessing -
3336+
* otherwise the necessary pg_class changes cannot be made with
3337+
* encountering assertions.
3338+
*/
3339+
RelationSetNewRelfilenode(iRel,persistence);
3340+
3341+
/* ensure SetReindexProcessing state isn't leaked */
33193342
PG_TRY();
33203343
{
33213344
/* Suppress use of the target index while rebuilding it */
33223345
SetReindexProcessing(heapId,indexId);
33233346

3324-
/* Fetch info needed for index_build */
3325-
indexInfo=BuildIndexInfo(iRel);
3326-
3327-
/* If requested, skip checking uniqueness/exclusion constraints */
3328-
if (skip_constraint_checks)
3329-
{
3330-
if (indexInfo->ii_Unique||indexInfo->ii_ExclusionOps!=NULL)
3331-
skipped_constraint= true;
3332-
indexInfo->ii_Unique= false;
3333-
indexInfo->ii_ExclusionOps=NULL;
3334-
indexInfo->ii_ExclusionProcs=NULL;
3335-
indexInfo->ii_ExclusionStrats=NULL;
3336-
}
3337-
3338-
/* We'll build a new physical relation for the index */
3339-
RelationSetNewRelfilenode(iRel,persistence);
3340-
33413347
/* Initialize the index and rebuild */
33423348
/* Note: we do not need to re-establish pkey setting */
33433349
index_build(heapRelation,iRel,indexInfo, true, true);

‎src/backend/catalog/indexing.c

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,15 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple)
8282
Datumvalues[INDEX_MAX_KEYS];
8383
boolisnull[INDEX_MAX_KEYS];
8484

85-
/* HOT update does not require index inserts */
85+
/*
86+
* HOT update does not require index inserts. But with asserts enabled we
87+
* want to check that it'd be legal to currently insert into the
88+
* table/index.
89+
*/
90+
#ifndefUSE_ASSERT_CHECKING
8691
if (HeapTupleIsHeapOnly(heapTuple))
8792
return;
93+
#endif
8894

8995
/*
9096
* Get information from the state structure. Fall out if nothing to do.
@@ -107,8 +113,10 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple)
107113
for (i=0;i<numIndexes;i++)
108114
{
109115
IndexInfo*indexInfo;
116+
Relationindex;
110117

111118
indexInfo=indexInfoArray[i];
119+
index=relationDescs[i];
112120

113121
/* If the index is marked as read-only, ignore it */
114122
if (!indexInfo->ii_ReadyForInserts)
@@ -121,9 +129,18 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple)
121129
Assert(indexInfo->ii_Expressions==NIL);
122130
Assert(indexInfo->ii_Predicate==NIL);
123131
Assert(indexInfo->ii_ExclusionOps==NULL);
124-
Assert(relationDescs[i]->rd_index->indimmediate);
132+
Assert(index->rd_index->indimmediate);
125133
Assert(indexInfo->ii_NumIndexKeyAttrs!=0);
126134

135+
/* see earlier check above */
136+
#ifdefUSE_ASSERT_CHECKING
137+
if (HeapTupleIsHeapOnly(heapTuple))
138+
{
139+
Assert(!ReindexIsProcessingIndex(RelationGetRelid(index)));
140+
continue;
141+
}
142+
#endif/* USE_ASSERT_CHECKING */
143+
127144
/*
128145
* FormIndexDatum fills in its values and isnull parameters with the
129146
* appropriate values for the column(s) of the index.

‎src/test/regress/expected/create_index.out

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1939,6 +1939,24 @@ INFO: index "reindex_verbose_pkey" was reindexed
19391939
\set VERBOSITY default
19401940
DROP TABLE reindex_verbose;
19411941
--
1942+
-- check that system tables can be reindexed
1943+
--
1944+
-- whole tables
1945+
REINDEX TABLE pg_class; -- mapped, non-shared, critical
1946+
REINDEX TABLE pg_index; -- non-mapped, non-shared, critical
1947+
REINDEX TABLE pg_operator; -- non-mapped, non-shared, critical
1948+
REINDEX TABLE pg_database; -- mapped, shared, critical
1949+
REINDEX TABLE pg_shdescription; -- mapped, shared non-critical
1950+
-- Check that individual system indexes can be reindexed. That's a bit
1951+
-- different from the entire-table case because reindex_relation
1952+
-- treats e.g. pg_class special.
1953+
REINDEX INDEX pg_class_oid_index; -- mapped, non-shared, critical
1954+
REINDEX INDEX pg_class_relname_nsp_index; -- mapped, non-shared, non-critical
1955+
REINDEX INDEX pg_index_indexrelid_index; -- non-mapped, non-shared, critical
1956+
REINDEX INDEX pg_index_indrelid_index; -- non-mapped, non-shared, non-critical
1957+
REINDEX INDEX pg_database_oid_index; -- mapped, shared, critical
1958+
REINDEX INDEX pg_shdescription_o_c_index; -- mapped, shared, non-critical
1959+
--
19421960
-- REINDEX CONCURRENTLY
19431961
--
19441962
CREATE TABLE concur_reindex_tab (c1 int);

‎src/test/regress/sql/create_index.sql

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -747,6 +747,27 @@ REINDEX (VERBOSE) TABLE reindex_verbose;
747747
\set VERBOSITY default
748748
DROPTABLE reindex_verbose;
749749

750+
--
751+
-- check that system tables can be reindexed
752+
--
753+
754+
-- whole tables
755+
REINDEX TABLE pg_class;-- mapped, non-shared, critical
756+
REINDEX TABLE pg_index;-- non-mapped, non-shared, critical
757+
REINDEX TABLE pg_operator;-- non-mapped, non-shared, critical
758+
REINDEX TABLE pg_database;-- mapped, shared, critical
759+
REINDEX TABLE pg_shdescription;-- mapped, shared non-critical
760+
761+
-- Check that individual system indexes can be reindexed. That's a bit
762+
-- different from the entire-table case because reindex_relation
763+
-- treats e.g. pg_class special.
764+
REINDEX INDEX pg_class_oid_index;-- mapped, non-shared, critical
765+
REINDEX INDEX pg_class_relname_nsp_index;-- mapped, non-shared, non-critical
766+
REINDEX INDEX pg_index_indexrelid_index;-- non-mapped, non-shared, critical
767+
REINDEX INDEX pg_index_indrelid_index;-- non-mapped, non-shared, non-critical
768+
REINDEX INDEX pg_database_oid_index;-- mapped, shared, critical
769+
REINDEX INDEX pg_shdescription_o_c_index;-- mapped, shared, non-critical
770+
750771
--
751772
-- REINDEX CONCURRENTLY
752773
--

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp