Allow ALTER INDEX .. ATTACH PARTITION to validate a parent index

This commit tweaks ALTER INDEX .. ATTACH PARTITION to attempt a
validation of a parent index in the case where an index is already
attached but the parent is not yet valid.  This occurs in cases where a
parent index was created invalid such as with CREATE INDEX ONLY, but was
left invalid after an invalid child index was attached (partitioned
indexes set indisvalid to false if at least one partition is
!indisvalid, indisvalid is true in a partitioned table iff all
partitions are indisvalid).  This could leave a partition tree in a
situation where a user could not bring the parent index back to valid
after fixing the child index, as there is no built-in mechanism to do
so.  This commit relies on the fact that repeated ATTACH PARTITION
commands on the same index silently succeed.

An invalid parent index is more than just a passive issue.  It causes
for example ON CONFLICT on a partitioned table if the invalid parent
index is used to enforce a unique constraint.

Some test cases are added to track some of problematic patterns, using a
set of partition trees with combinations of invalid indexes and ATTACH
PARTITION.

Reported-by: Mohamed Ali <moali.pg@gmail.com>
Author: Sami Imseih <sanmimseih@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Haibo Yan <tristan.yim@gmail.com>
Discussion: http://postgr.es/m/CAGnOmWqi1D9ycBgUeOGf6mOCd2Dcf=6sKhbf4sHLs5xAcKVCMQ@mail.gmail.com
Backpatch-through: 14

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7f62e89342e1332122494845face867f93d15c88..cabe22d48be1e2361ffebcfc0620d2a4578ed11b 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -21671,7 +21671,10 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name)
 
        ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partIdx));
 
-       /* Silently do nothing if already in the right state */
+       /*
+        * Check if the index is already attached to the correct parent,
+        * ultimately attempting one round of validation if already the case.
+        */
        currParent = partIdx->rd_rel->relispartition ?
                get_partition_parent(partIdxId, false) : InvalidOid;
        if (currParent != RelationGetRelid(parentIdx))
@@ -21780,6 +21783,14 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name)
 
                validatePartitionedIndex(parentIdx, parentTbl);
        }
+       else if (!parentIdx->rd_index->indisvalid)
+       {
+               /*
+                * The index is attached, but the parent is still invalid; see if it
+                * can be validated now.
+                */
+               validatePartitionedIndex(parentIdx, parentTbl);
+       }
 
        relation_close(parentTbl, AccessShareLock);
        /* keep these locks till commit */

diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index bcf1db11d731d0062ae8d6ef45e49d5ab2452939..b8d7d3047bee744891f4ba592c9b75ca7243b1df 100644 (file)
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -540,6 +540,111 @@ select relname, indisvalid from pg_class join pg_index on indexrelid = oid
  idxpart_a_idx   | t
 (3 rows)
 
+drop table idxpart;
+-- Verify that re-attaching an already-attached partition index can
+-- validate the parent index if it was still invalid, including
+-- indirect ancestors in subpartitions.
+create table idxpart (a int, b int) partition by range (a);
+create table idxpart1 partition of idxpart for values from (0) to (1000) partition by range (a);
+create table idxpart11 partition of idxpart1 for values from (0) to (500);
+-- Partitioned table with no partitions
+create table idxpart2 partition of idxpart for values from (1000) to (2000) partition by range (a);
+-- create parent indexes
+create index on only idxpart ((a/b));
+create index on only idxpart1 ((a/b));
+create index on only idxpart2 ((a/b));
+-- fail, leaves behind an invalid index on the leaf partition
+insert into idxpart11 values (1, 0);
+create index concurrently on idxpart11 ((a/b));
+ERROR:  division by zero
+select relname, indisvalid from pg_class join pg_index on indexrelid = oid
+   where relname like 'idxpart%' order by relname;
+      relname       | indisvalid 
+--------------------+------------
+ idxpart11_expr_idx | f
+ idxpart1_expr_idx  | f
+ idxpart2_expr_idx  | t
+ idxpart_expr_idx   | f
+(4 rows)
+
+-- attach the indexes; parents stay invalid
+alter index idxpart1_expr_idx attach partition idxpart11_expr_idx;
+alter index idxpart_expr_idx attach partition idxpart1_expr_idx;
+alter index idxpart_expr_idx attach partition idxpart2_expr_idx;
+select relname, indisvalid from pg_class join pg_index on indexrelid = oid
+   where relname like 'idxpart%' order by relname;
+      relname       | indisvalid 
+--------------------+------------
+ idxpart11_expr_idx | f
+ idxpart1_expr_idx  | f
+ idxpart2_expr_idx  | t
+ idxpart_expr_idx   | f
+(4 rows)
+
+-- fix the index on the leaf partition
+delete from idxpart11 where b = 0;
+reindex index concurrently idxpart11_expr_idx;
+-- reattach the leaf partition index; parents should now be valid
+alter index idxpart1_expr_idx attach partition idxpart11_expr_idx;
+select relname, indisvalid from pg_class join pg_index on indexrelid = oid
+   where relname like 'idxpart%' order by relname;
+      relname       | indisvalid 
+--------------------+------------
+ idxpart11_expr_idx | t
+ idxpart1_expr_idx  | t
+ idxpart2_expr_idx  | t
+ idxpart_expr_idx   | t
+(4 rows)
+
+drop table idxpart;
+-- Verify that re-attaching does not validate the parent when another
+-- child index is still invalid.
+create table idxpart (a int, b int) partition by range (a);
+create table idxpart1 partition of idxpart for values from (0) to (500);
+create table idxpart2 partition of idxpart for values from (500) to (1000);
+create index on only idxpart ((a/b));
+-- create invalid indexes on both children
+insert into idxpart1 values (1, 0);
+insert into idxpart2 values (501, 0);
+create index concurrently on idxpart1 ((a/b));
+ERROR:  division by zero
+create index concurrently on idxpart2 ((a/b));
+ERROR:  division by zero
+select relname, indisvalid from pg_class join pg_index on indexrelid = oid
+   where relname like 'idxpart%' order by relname;
+      relname      | indisvalid 
+-------------------+------------
+ idxpart1_expr_idx | f
+ idxpart2_expr_idx | f
+ idxpart_expr_idx  | f
+(3 rows)
+
+-- attach both; parent stays invalid
+alter index idxpart_expr_idx attach partition idxpart1_expr_idx;
+alter index idxpart_expr_idx attach partition idxpart2_expr_idx;
+select relname, indisvalid from pg_class join pg_index on indexrelid = oid
+   where relname like 'idxpart%' order by relname;
+      relname      | indisvalid 
+-------------------+------------
+ idxpart1_expr_idx | f
+ idxpart2_expr_idx | f
+ idxpart_expr_idx  | f
+(3 rows)
+
+-- fix only idxpart1's index, leave idxpart2's still invalid
+delete from idxpart1 where b = 0;
+reindex index concurrently idxpart1_expr_idx;
+-- re-attach the fixed child; parent should stay invalid
+alter index idxpart_expr_idx attach partition idxpart1_expr_idx;
+select relname, indisvalid from pg_class join pg_index on indexrelid = oid
+   where relname like 'idxpart%' order by relname;
+      relname      | indisvalid 
+-------------------+------------
+ idxpart1_expr_idx | t
+ idxpart2_expr_idx | f
+ idxpart_expr_idx  | f
+(3 rows)
+
 drop table idxpart;
 -- verify dependency handling during ALTER TABLE DETACH PARTITION
 create table idxpart (a int) partition by range (a);

diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index b5cb01c2d7097cb95b833fd0d92d2a99b53f81b8..706a6ec04ac26f3357fc9d9a3fdf60f6d06d8cd2 100644 (file)
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -246,6 +246,65 @@ select relname, indisvalid from pg_class join pg_index on indexrelid = oid
    where relname like 'idxpart%' order by relname;
 drop table idxpart;
 
+-- Verify that re-attaching an already-attached partition index can
+-- validate the parent index if it was still invalid, including
+-- indirect ancestors in subpartitions.
+create table idxpart (a int, b int) partition by range (a);
+create table idxpart1 partition of idxpart for values from (0) to (1000) partition by range (a);
+create table idxpart11 partition of idxpart1 for values from (0) to (500);
+-- Partitioned table with no partitions
+create table idxpart2 partition of idxpart for values from (1000) to (2000) partition by range (a);
+-- create parent indexes
+create index on only idxpart ((a/b));
+create index on only idxpart1 ((a/b));
+create index on only idxpart2 ((a/b));
+-- fail, leaves behind an invalid index on the leaf partition
+insert into idxpart11 values (1, 0);
+create index concurrently on idxpart11 ((a/b));
+select relname, indisvalid from pg_class join pg_index on indexrelid = oid
+   where relname like 'idxpart%' order by relname;
+-- attach the indexes; parents stay invalid
+alter index idxpart1_expr_idx attach partition idxpart11_expr_idx;
+alter index idxpart_expr_idx attach partition idxpart1_expr_idx;
+alter index idxpart_expr_idx attach partition idxpart2_expr_idx;
+select relname, indisvalid from pg_class join pg_index on indexrelid = oid
+   where relname like 'idxpart%' order by relname;
+-- fix the index on the leaf partition
+delete from idxpart11 where b = 0;
+reindex index concurrently idxpart11_expr_idx;
+-- reattach the leaf partition index; parents should now be valid
+alter index idxpart1_expr_idx attach partition idxpart11_expr_idx;
+select relname, indisvalid from pg_class join pg_index on indexrelid = oid
+   where relname like 'idxpart%' order by relname;
+drop table idxpart;
+
+-- Verify that re-attaching does not validate the parent when another
+-- child index is still invalid.
+create table idxpart (a int, b int) partition by range (a);
+create table idxpart1 partition of idxpart for values from (0) to (500);
+create table idxpart2 partition of idxpart for values from (500) to (1000);
+create index on only idxpart ((a/b));
+-- create invalid indexes on both children
+insert into idxpart1 values (1, 0);
+insert into idxpart2 values (501, 0);
+create index concurrently on idxpart1 ((a/b));
+create index concurrently on idxpart2 ((a/b));
+select relname, indisvalid from pg_class join pg_index on indexrelid = oid
+   where relname like 'idxpart%' order by relname;
+-- attach both; parent stays invalid
+alter index idxpart_expr_idx attach partition idxpart1_expr_idx;
+alter index idxpart_expr_idx attach partition idxpart2_expr_idx;
+select relname, indisvalid from pg_class join pg_index on indexrelid = oid
+   where relname like 'idxpart%' order by relname;
+-- fix only idxpart1's index, leave idxpart2's still invalid
+delete from idxpart1 where b = 0;
+reindex index concurrently idxpart1_expr_idx;
+-- re-attach the fixed child; parent should stay invalid
+alter index idxpart_expr_idx attach partition idxpart1_expr_idx;
+select relname, indisvalid from pg_class join pg_index on indexrelid = oid
+   where relname like 'idxpart%' order by relname;
+drop table idxpart;
+
 -- verify dependency handling during ALTER TABLE DETACH PARTITION
 create table idxpart (a int) partition by range (a);
 create table idxpart1 (like idxpart);