On Wed, Jul 12, 2023 at 09:38:41AM +0900, Michael Paquier wrote: > While working recently on what has led to cfc43ae and fc55c7f, I > really got the feeling that there could be some command sequences that > lacked some CCIs (or CommandCounterIncrement calls) to make sure that > the catalog updates are visible in any follow-up steps in the same > transaction.
Wait a minute. The validation of a partitioned index uses a copy of
the pg_index tuple from the relcache, which be out of date:
newtup = heap_copytuple(partedIdx->rd_indextuple);
((Form_pg_index) GETSTRUCT(newtup))->indisvalid = true;
And it seems to me that we should do the catalog update based on a
copy of a tuple coming from the syscache, no? Attached is a patch
that fixes your issue with more advanced regression tests that use two
levels of partitioning, looping twice through an update of indisvalid
when attaching the leaf index (the test reproduces the problem on
HEAD, as well).
--
Michael
From 685e63dbbb57a3339926a1e8eb84ffdd010c5a51 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Wed, 12 Jul 2023 14:36:07 +0900
Subject: [PATCH] Fix validation update of partitioned indexes
---
src/backend/commands/tablecmds.c | 14 ++++--
src/test/regress/expected/indexing.out | 63 ++++++++++++++++++++++++++
src/test/regress/sql/indexing.sql | 43 ++++++++++++++++++
3 files changed, 116 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8fff036b73..f740306578 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -19175,15 +19175,21 @@ validatePartitionedIndex(Relation partedIdx, Relation partedTbl)
if (tuples == RelationGetPartitionDesc(partedTbl, true)->nparts)
{
Relation idxRel;
- HeapTuple newtup;
+ HeapTuple indTup;
+ Form_pg_index indexForm;
idxRel = table_open(IndexRelationId, RowExclusiveLock);
+ indTup = SearchSysCacheCopy1(INDEXRELID,
+ ObjectIdGetDatum(RelationGetRelid(partedIdx)));
+ if (!HeapTupleIsValid(indTup))
+ elog(ERROR, "cache lookup failed for index %u",
+ RelationGetRelid(partedIdx));
+ indexForm = (Form_pg_index) GETSTRUCT(indTup);
- newtup = heap_copytuple(partedIdx->rd_indextuple);
- ((Form_pg_index) GETSTRUCT(newtup))->indisvalid = true;
+ indexForm->indisvalid = true;
updated = true;
- CatalogTupleUpdate(idxRel, &partedIdx->rd_indextuple->t_self, newtup);
+ CatalogTupleUpdate(idxRel, &indTup->t_self, indTup);
table_close(idxRel, RowExclusiveLock);
}
diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index 3e5645c2ab..368e735de2 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -1486,3 +1486,66 @@ select indexrelid::regclass, indisvalid,
(5 rows)
drop table parted_isvalid_tab;
+-- Check state of replica indexes when attaching a partition.
+begin;
+create table parted_replica_tab (id int not null) partition by range (id);
+create table parted_replica_tab_1 partition of parted_replica_tab
+ for values from (1) to (10) partition by range (id);
+create table parted_replica_tab_11 partition of parted_replica_tab_1
+ for values from (1) to (5);
+create unique index parted_replica_idx
+ on only parted_replica_tab using btree (id);
+create unique index parted_replica_idx_1
+ on only parted_replica_tab_1 using btree (id);
+-- This triggers an update of pg_index.indisreplident for parted_replica_idx.
+alter table only parted_replica_tab_1 replica identity
+ using index parted_replica_idx_1;
+create unique index parted_replica_idx_11 on parted_replica_tab_11 USING btree (id);
+select indexrelid::regclass, indisvalid, indisreplident,
+ indrelid::regclass, inhparent::regclass
+ from pg_index idx left join
+ pg_inherits inh on (idx.indexrelid = inh.inhrelid)
+ where indexrelid::regclass::text like 'parted_replica%'
+ order by indexrelid::regclass::text collate "C";
+ indexrelid | indisvalid | indisreplident | indrelid | inhparent
+-----------------------+------------+----------------+-----------------------+-----------
+ parted_replica_idx | f | f | parted_replica_tab |
+ parted_replica_idx_1 | f | t | parted_replica_tab_1 |
+ parted_replica_idx_11 | t | f | parted_replica_tab_11 |
+(3 rows)
+
+-- parted_replica_idx is not valid yet here, because parted_replica_idx_1
+-- is not valid.
+alter index parted_replica_idx ATTACH PARTITION parted_replica_idx_1;
+select indexrelid::regclass, indisvalid, indisreplident,
+ indrelid::regclass, inhparent::regclass
+ from pg_index idx left join
+ pg_inherits inh on (idx.indexrelid = inh.inhrelid)
+ where indexrelid::regclass::text like 'parted_replica%'
+ order by indexrelid::regclass::text collate "C";
+ indexrelid | indisvalid | indisreplident | indrelid | inhparent
+-----------------------+------------+----------------+-----------------------+--------------------
+ parted_replica_idx | f | f | parted_replica_tab |
+ parted_replica_idx_1 | f | t | parted_replica_tab_1 | parted_replica_idx
+ parted_replica_idx_11 | t | f | parted_replica_tab_11 |
+(3 rows)
+
+-- parted_replica_idx becomes valid here.
+alter index parted_replica_idx_1 ATTACH PARTITION parted_replica_idx_11;
+alter table only parted_replica_tab_1 replica identity
+ using index parted_replica_idx_1;
+commit;
+select indexrelid::regclass, indisvalid, indisreplident,
+ indrelid::regclass, inhparent::regclass
+ from pg_index idx left join
+ pg_inherits inh on (idx.indexrelid = inh.inhrelid)
+ where indexrelid::regclass::text like 'parted_replica%'
+ order by indexrelid::regclass::text collate "C";
+ indexrelid | indisvalid | indisreplident | indrelid | inhparent
+-----------------------+------------+----------------+-----------------------+----------------------
+ parted_replica_idx | t | f | parted_replica_tab |
+ parted_replica_idx_1 | t | t | parted_replica_tab_1 | parted_replica_idx
+ parted_replica_idx_11 | t | f | parted_replica_tab_11 | parted_replica_idx_1
+(3 rows)
+
+drop table parted_replica_tab;
diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index d6e5a06d95..6f60d1dc0f 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -808,3 +808,46 @@ select indexrelid::regclass, indisvalid,
where indexrelid::regclass::text like 'parted_isvalid%'
order by indexrelid::regclass::text collate "C";
drop table parted_isvalid_tab;
+
+-- Check state of replica indexes when attaching a partition.
+begin;
+create table parted_replica_tab (id int not null) partition by range (id);
+create table parted_replica_tab_1 partition of parted_replica_tab
+ for values from (1) to (10) partition by range (id);
+create table parted_replica_tab_11 partition of parted_replica_tab_1
+ for values from (1) to (5);
+create unique index parted_replica_idx
+ on only parted_replica_tab using btree (id);
+create unique index parted_replica_idx_1
+ on only parted_replica_tab_1 using btree (id);
+-- This triggers an update of pg_index.indisreplident for parted_replica_idx.
+alter table only parted_replica_tab_1 replica identity
+ using index parted_replica_idx_1;
+create unique index parted_replica_idx_11 on parted_replica_tab_11 USING btree (id);
+select indexrelid::regclass, indisvalid, indisreplident,
+ indrelid::regclass, inhparent::regclass
+ from pg_index idx left join
+ pg_inherits inh on (idx.indexrelid = inh.inhrelid)
+ where indexrelid::regclass::text like 'parted_replica%'
+ order by indexrelid::regclass::text collate "C";
+-- parted_replica_idx is not valid yet here, because parted_replica_idx_1
+-- is not valid.
+alter index parted_replica_idx ATTACH PARTITION parted_replica_idx_1;
+select indexrelid::regclass, indisvalid, indisreplident,
+ indrelid::regclass, inhparent::regclass
+ from pg_index idx left join
+ pg_inherits inh on (idx.indexrelid = inh.inhrelid)
+ where indexrelid::regclass::text like 'parted_replica%'
+ order by indexrelid::regclass::text collate "C";
+-- parted_replica_idx becomes valid here.
+alter index parted_replica_idx_1 ATTACH PARTITION parted_replica_idx_11;
+alter table only parted_replica_tab_1 replica identity
+ using index parted_replica_idx_1;
+commit;
+select indexrelid::regclass, indisvalid, indisreplident,
+ indrelid::regclass, inhparent::regclass
+ from pg_index idx left join
+ pg_inherits inh on (idx.indexrelid = inh.inhrelid)
+ where indexrelid::regclass::text like 'parted_replica%'
+ order by indexrelid::regclass::text collate "C";
+drop table parted_replica_tab;
--
2.40.1
signature.asc
Description: PGP signature
