Fix updates of indisvalid for partitioned indexes

indisvalid is switched to true for partitioned indexes when all its
partitions have valid indexes when attaching a new partition, up to the
top-most parent if all its leaves are themselves valid when dealing with
multiple layers of partitions.

The copy of the tuple from pg_index used to switch indisvalid to true
came from the relation cache, which is incorrect.  Particularly, in the
case reported by Shruthi Gowda, executing a series of commands in a
single transaction would cause the validation of partitioned indexes to
use an incorrect version of a pg_index tuple, as indexes are reloaded
after an invalidation request with RelationReloadIndexInfo(), a much
faster version than a full index cache rebuild.  In this case, the
limited information updated in the cache leads to an incorrect version
of the tuple used.  One of the symptoms reported was the following
error, with a replica identity update, for instance:
"ERROR: attempted to update invisible tuple"

This is incorrect since 8b08f7d, so backpatch all the way down.

Reported-by: Shruthi Gowda
Author: Michael Paquier
Reviewed-by: Shruthi Gowda, Dilip Kumar
Discussion: https://postgr.es/m/CAASxf_PBcxax0wW-3gErUyftZ0XrCs3Lrpuhq4-Z3Fak1DoW7Q@mail.gmail.com
Backpatch-through: 11

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 05409ff6f9fc1843dda86266e53dc38b63b94ef0..385616bdb5486d593129606bded963fc5ba559d7 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -16390,17 +16390,24 @@ validatePartitionedIndex(Relation partedIdx, Relation partedTbl)
        if (tuples == RelationGetPartitionDesc(partedTbl)->nparts)
        {
                Relation        idxRel;
-               HeapTuple       newtup;
+               HeapTuple       indTup;
+               Form_pg_index indexForm;
 
                idxRel = heap_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);
 
                heap_close(idxRel, RowExclusiveLock);
+               heap_freetuple(indTup);
        }
 
        /*

diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index 3eec572e917405542e23677b918d646b6a4e0422..24d3ecad5a54619c54474d41ec3b9427af5556f4 100644 (file)
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -1572,3 +1572,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 5e89a0bcd9b079e6d36effcb2e777e6c9f6bd62e..7401c7cfb992a4158e3c031ae1c240a374abab4d 100644 (file)
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -843,3 +843,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;