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

Commit72d6111

Browse files
committed
Allow REPLICA IDENTITY to be set on an index that's not (yet) valid.
The motivation for this change is that when pg_dump dumps apartitioned index that's marked REPLICA IDENTITY, it generates acommand sequence that applies REPLICA IDENTITY before the partitionedindex has been marked valid, causing restore to fail. We couldperhaps change pg_dump to not do it like that, but that would bedifficult and would not fix existing dump files with the problem.There seems to be very little reason for the backend to disallowthis anyway --- the code ignores indisreplident when the indexisn't valid --- so instead let's fix it by allowing the case.Commit9511fb3 previously expressed a concern that allowingindisreplident to be set on invalid indexes might allow us towind up in a situation where a table could have indisreplidentset on multiple indexes. I'm not sure I follow that concernexactly, but in any case the only way that could happen is becauserelation_mark_replica_identity is too trusting about the existing setof markings being valid. Let's just rip out its early-exit code path(which sure looks like premature optimization anyway; what are wedoing expending code to make redundant ALTER TABLE ... REPLICAIDENTITY commands marginally faster and not-redundant ones marginallyslower?) and fix it to positively guarantee that no more than oneindex is marked indisreplident.The pg_dump failure can be demonstrated in all supported branches,so back-patch all the way. I chose to back-patch9511fb3 as well,just to keep indisreplident handling the same in all branches.Per bug #17756 from Sergey Belyashov.Discussion:https://postgr.es/m/17756-dd50e8e0c8dd4a40@postgresql.org
1 parenta9bccff commit72d6111

File tree

4 files changed

+85
-44
lines changed

4 files changed

+85
-44
lines changed

‎src/backend/catalog/index.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1568,7 +1568,6 @@ index_concurrently_swap(Oid newIndexId, Oid oldIndexId, const char *oldName)
15681568

15691569
/* Preserve indisreplident in the new index */
15701570
newIndexForm->indisreplident=oldIndexForm->indisreplident;
1571-
oldIndexForm->indisreplident= false;
15721571

15731572
/* Preserve indisclustered in the new index */
15741573
newIndexForm->indisclustered=oldIndexForm->indisclustered;
@@ -1580,6 +1579,7 @@ index_concurrently_swap(Oid newIndexId, Oid oldIndexId, const char *oldName)
15801579
newIndexForm->indisvalid= true;
15811580
oldIndexForm->indisvalid= false;
15821581
oldIndexForm->indisclustered= false;
1582+
oldIndexForm->indisreplident= false;
15831583

15841584
CatalogTupleUpdate(pg_index,&oldIndexTuple->t_self,oldIndexTuple);
15851585
CatalogTupleUpdate(pg_index,&newIndexTuple->t_self,newIndexTuple);
@@ -3463,10 +3463,12 @@ index_set_state_flags(Oid indexId, IndexStateFlagsAction action)
34633463
* CONCURRENTLY that failed partway through.)
34643464
*
34653465
* Note: the CLUSTER logic assumes that indisclustered cannot be
3466-
* set on any invalid index, so clear that flag too.
3466+
* set on any invalid index, so clear that flag too. For
3467+
* cleanliness, also clear indisreplident.
34673468
*/
34683469
indexForm->indisvalid= false;
34693470
indexForm->indisclustered= false;
3471+
indexForm->indisreplident= false;
34703472
break;
34713473
caseINDEX_DROP_SET_DEAD:
34723474

@@ -3478,6 +3480,8 @@ index_set_state_flags(Oid indexId, IndexStateFlagsAction action)
34783480
* the index at all.
34793481
*/
34803482
Assert(!indexForm->indisvalid);
3483+
Assert(!indexForm->indisclustered);
3484+
Assert(!indexForm->indisreplident);
34813485
indexForm->indisready= false;
34823486
indexForm->indislive= false;
34833487
break;

‎src/backend/commands/tablecmds.c

Lines changed: 22 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -14892,7 +14892,10 @@ ATExecDropOf(Relation rel, LOCKMODE lockmode)
1489214892
* relation_mark_replica_identity: Update a table's replica identity
1489314893
*
1489414894
* Iff ri_type = REPLICA_IDENTITY_INDEX, indexOid must be the Oid of a suitable
14895-
* index. Otherwise, it should be InvalidOid.
14895+
* index. Otherwise, it must be InvalidOid.
14896+
*
14897+
* Caller had better hold an exclusive lock on the relation, as the results
14898+
* of running two of these concurrently wouldn't be pretty.
1489614899
*/
1489714900
static void
1489814901
relation_mark_replica_identity(Relation rel, char ri_type, Oid indexOid,
@@ -14904,7 +14907,6 @@ relation_mark_replica_identity(Relation rel, char ri_type, Oid indexOid,
1490414907
HeapTuplepg_index_tuple;
1490514908
Form_pg_class pg_class_form;
1490614909
Form_pg_index pg_index_form;
14907-
1490814910
ListCell *index;
1490914911

1491014912
/*
@@ -14926,29 +14928,7 @@ relation_mark_replica_identity(Relation rel, char ri_type, Oid indexOid,
1492614928
heap_freetuple(pg_class_tuple);
1492714929

1492814930
/*
14929-
* Check whether the correct index is marked indisreplident; if so, we're
14930-
* done.
14931-
*/
14932-
if (OidIsValid(indexOid))
14933-
{
14934-
Assert(ri_type == REPLICA_IDENTITY_INDEX);
14935-
14936-
pg_index_tuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexOid));
14937-
if (!HeapTupleIsValid(pg_index_tuple))
14938-
elog(ERROR, "cache lookup failed for index %u", indexOid);
14939-
pg_index_form = (Form_pg_index) GETSTRUCT(pg_index_tuple);
14940-
14941-
if (pg_index_form->indisreplident)
14942-
{
14943-
ReleaseSysCache(pg_index_tuple);
14944-
return;
14945-
}
14946-
ReleaseSysCache(pg_index_tuple);
14947-
}
14948-
14949-
/*
14950-
* Clear the indisreplident flag from any index that had it previously,
14951-
* and set it for any index that should have it now.
14931+
* Update the per-index indisreplident flags correctly.
1495214932
*/
1495314933
pg_index = table_open(IndexRelationId, RowExclusiveLock);
1495414934
foreach(index, RelationGetIndexList(rel))
@@ -14962,19 +14942,23 @@ relation_mark_replica_identity(Relation rel, char ri_type, Oid indexOid,
1496214942
elog(ERROR, "cache lookup failed for index %u", thisIndexOid);
1496314943
pg_index_form = (Form_pg_index) GETSTRUCT(pg_index_tuple);
1496414944

14965-
/*
14966-
* Unset the bit if set. We know it's wrong because we checked this
14967-
* earlier.
14968-
*/
14969-
if (pg_index_form->indisreplident)
14945+
if (thisIndexOid == indexOid)
1497014946
{
14971-
dirty = true;
14972-
pg_index_form->indisreplident = false;
14947+
/* Set the bit if not already set. */
14948+
if (!pg_index_form->indisreplident)
14949+
{
14950+
dirty = true;
14951+
pg_index_form->indisreplident = true;
14952+
}
1497314953
}
14974-
else if (thisIndexOid == indexOid)
14954+
else
1497514955
{
14976-
dirty = true;
14977-
pg_index_form->indisreplident = true;
14956+
/* Unset the bit if set. */
14957+
if (pg_index_form->indisreplident)
14958+
{
14959+
dirty = true;
14960+
pg_index_form->indisreplident = false;
14961+
}
1497814962
}
1497914963

1498014964
if (dirty)
@@ -14985,7 +14969,9 @@ relation_mark_replica_identity(Relation rel, char ri_type, Oid indexOid,
1498514969
/*
1498614970
* Invalidate the relcache for the table, so that after we commit
1498714971
* all sessions will refresh the table's replica identity index
14988-
* before attempting any UPDATE or DELETE on the table.
14972+
* before attempting any UPDATE or DELETE on the table. (If we
14973+
* changed the table's pg_class row above, then a relcache inval
14974+
* is already queued due to that; but we might not have.)
1498914975
*/
1499014976
CacheInvalidateRelcache(rel);
1499114977
}
@@ -15071,12 +15057,6 @@ ATExecReplicaIdentity(Relation rel, ReplicaIdentityStmt *stmt, LOCKMODE lockmode
1507115057
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
1507215058
errmsg("cannot use partial index \"%s\" as replica identity",
1507315059
RelationGetRelationName(indexRel))));
15074-
/* And neither are invalid indexes. */
15075-
if (!indexRel->rd_index->indisvalid)
15076-
ereport(ERROR,
15077-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
15078-
errmsg("cannot use invalid index \"%s\" as replica identity",
15079-
RelationGetRelationName(indexRel))));
1508015060

1508115061
/* Check index for nullable columns. */
1508215062
for (key = 0; key < IndexRelationGetNumberOfKeyAttributes(indexRel); key++)

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,44 @@ Indexes:
227227
-- used as replica identity.
228228
ALTER TABLE test_replica_identity3 ALTER COLUMN id DROP NOT NULL;
229229
ERROR: column "id" is in index used as replica identity
230+
--
231+
-- Test that replica identity can be set on an index that's not yet valid.
232+
-- (This matches the way pg_dump will try to dump a partitioned table.)
233+
--
234+
CREATE TABLE test_replica_identity4(id integer NOT NULL) PARTITION BY LIST (id);
235+
CREATE TABLE test_replica_identity4_1(id integer NOT NULL);
236+
ALTER TABLE ONLY test_replica_identity4
237+
ATTACH PARTITION test_replica_identity4_1 FOR VALUES IN (1);
238+
ALTER TABLE ONLY test_replica_identity4
239+
ADD CONSTRAINT test_replica_identity4_pkey PRIMARY KEY (id);
240+
ALTER TABLE ONLY test_replica_identity4
241+
REPLICA IDENTITY USING INDEX test_replica_identity4_pkey;
242+
ALTER TABLE ONLY test_replica_identity4_1
243+
ADD CONSTRAINT test_replica_identity4_1_pkey PRIMARY KEY (id);
244+
\d+ test_replica_identity4
245+
Partitioned table "public.test_replica_identity4"
246+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
247+
--------+---------+-----------+----------+---------+---------+--------------+-------------
248+
id | integer | | not null | | plain | |
249+
Partition key: LIST (id)
250+
Indexes:
251+
"test_replica_identity4_pkey" PRIMARY KEY, btree (id) INVALID REPLICA IDENTITY
252+
Partitions: test_replica_identity4_1 FOR VALUES IN (1)
253+
254+
ALTER INDEX test_replica_identity4_pkey
255+
ATTACH PARTITION test_replica_identity4_1_pkey;
256+
\d+ test_replica_identity4
257+
Partitioned table "public.test_replica_identity4"
258+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
259+
--------+---------+-----------+----------+---------+---------+--------------+-------------
260+
id | integer | | not null | | plain | |
261+
Partition key: LIST (id)
262+
Indexes:
263+
"test_replica_identity4_pkey" PRIMARY KEY, btree (id) REPLICA IDENTITY
264+
Partitions: test_replica_identity4_1 FOR VALUES IN (1)
265+
230266
DROP TABLE test_replica_identity;
231267
DROP TABLE test_replica_identity2;
232268
DROP TABLE test_replica_identity3;
269+
DROP TABLE test_replica_identity4;
233270
DROP TABLE test_replica_identity_othertable;

‎src/test/regress/sql/replica_identity.sql

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,27 @@ ALTER TABLE test_replica_identity3 ALTER COLUMN id TYPE bigint;
9898
-- used as replica identity.
9999
ALTERTABLE test_replica_identity3 ALTER COLUMN id DROPNOT NULL;
100100

101+
--
102+
-- Test that replica identity can be set on an index that's not yet valid.
103+
-- (This matches the way pg_dump will try to dump a partitioned table.)
104+
--
105+
CREATETABLEtest_replica_identity4(idintegerNOT NULL) PARTITION BY LIST (id);
106+
CREATETABLEtest_replica_identity4_1(idintegerNOT NULL);
107+
ALTERTABLE ONLY test_replica_identity4
108+
ATTACH PARTITION test_replica_identity4_1 FORVALUESIN (1);
109+
ALTERTABLE ONLY test_replica_identity4
110+
ADDCONSTRAINT test_replica_identity4_pkeyPRIMARY KEY (id);
111+
ALTERTABLE ONLY test_replica_identity4
112+
REPLICA IDENTITY USING INDEX test_replica_identity4_pkey;
113+
ALTERTABLE ONLY test_replica_identity4_1
114+
ADDCONSTRAINT test_replica_identity4_1_pkeyPRIMARY KEY (id);
115+
\d+ test_replica_identity4
116+
ALTERINDEX test_replica_identity4_pkey
117+
ATTACH PARTITION test_replica_identity4_1_pkey;
118+
\d+ test_replica_identity4
119+
101120
DROPTABLE test_replica_identity;
102121
DROPTABLE test_replica_identity2;
103122
DROPTABLE test_replica_identity3;
123+
DROPTABLE test_replica_identity4;
104124
DROPTABLE test_replica_identity_othertable;

0 commit comments

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