]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Fix marking of indisvalid for partitioned indexes at creation
authorMichael Paquier <michael@paquier.xyz>
Fri, 30 Jun 2023 04:55:07 +0000 (13:55 +0900)
committerMichael Paquier <michael@paquier.xyz>
Fri, 30 Jun 2023 04:55:07 +0000 (13:55 +0900)
The logic that introduced partitioned indexes missed a few things when
invalidating a partitioned index when these are created, still the code
is written to handle recursions:
1) If created from scratch because a mapping index could not be found,
the new index created could be itself invalid, if for example it was a
partitioned index with one of its leaves invalid.
2) A CCI was missing when indisvalid is set for a parent index, leading
to inconsistent trees when recursing across more than one level for a
partitioned index creation if an invalidation of the parent was
required.

This could lead to the creation of a partition index tree where some of
the partitioned indexes are marked as invalid, but some of the parents
are marked valid, which is not something that should happen (as
validatePartitionedIndex() defines, indisvalid is switched to true for a
partitioned index iff all its partitions are themselves valid).

This patch makes sure that indisvalid is set to false on a partitioned
index if at least one of its partition is invalid.  The flag is set to
true if *all* its partitions are valid.

The regression test added in this commit abuses of a failed concurrent
index creation, marked as invalid, that maps with an index created on
its partitioned table afterwards.

Reported-by: Alexander Lakhin
Reviewed-by: Alexander Lakhin
Discussion: https://postgr.es/m/14987634-43c0-0cb3-e075-94d423607e08@gmail.com
Backpatch-through: 11

src/backend/commands/indexcmds.c
src/backend/utils/cache/lsyscache.c
src/include/utils/lsyscache.h
src/test/regress/expected/indexing.out
src/test/regress/sql/indexing.sql

index d679fef951cc9a32d976f87599f7f07098ac4d69..b8d0a5449ccbee0cbb853e2df0ab78147bd8e492 100644 (file)
@@ -1160,6 +1160,7 @@ DefineIndex(Oid relationId,
                                        IndexStmt  *childStmt = copyObject(stmt);
                                        bool            found_whole_row;
                                        ListCell   *lc;
+                                       ObjectAddress childAddr;
 
                                        /*
                                         * We can't use the same index name for the child index,
@@ -1212,14 +1213,24 @@ DefineIndex(Oid relationId,
                                        Assert(GetUserId() == child_save_userid);
                                        SetUserIdAndSecContext(root_save_userid,
                                                                                   root_save_sec_context);
-                                       DefineIndex(childRelid, childStmt,
-                                                               InvalidOid, /* no predefined OID */
-                                                               indexRelationId,        /* this is our child */
-                                                               createdConstraintId,
-                                                               is_alter_table, check_rights, check_not_in_use,
-                                                               skip_build, quiet);
+                                       childAddr =
+                                               DefineIndex(childRelid, childStmt,
+                                                                       InvalidOid, /* no predefined OID */
+                                                                       indexRelationId,        /* this is our child */
+                                                                       createdConstraintId,
+                                                                       is_alter_table, check_rights,
+                                                                       check_not_in_use,
+                                                                       skip_build, quiet);
                                        SetUserIdAndSecContext(child_save_userid,
                                                                                   child_save_sec_context);
+
+                                       /*
+                                        * Check if the index just created is valid or not, as it
+                                        * could be possible that it has been switched as invalid
+                                        * when recursing across multiple partition levels.
+                                        */
+                                       if (!get_index_isvalid(childAddr.objectId))
+                                               invalidate_parent = true;
                                }
 
                                pfree(attmap);
@@ -1249,6 +1260,12 @@ DefineIndex(Oid relationId,
                                ReleaseSysCache(tup);
                                heap_close(pg_index, RowExclusiveLock);
                                heap_freetuple(newtup);
+
+                               /*
+                                * CCI here to make this update visible, in case this recurses
+                                * across multiple partition levels.
+                                */
+                               CommandCounterIncrement();
                        }
                }
 
index 913cad35105782aa315c8473efbdf55f61a651e3..d3fca7129a959a1a4ce54cb90d090f3c75ab5cfd 100644 (file)
@@ -3204,3 +3204,26 @@ get_index_isclustered(Oid index_oid)
 
        return isclustered;
 }
+
+/*
+ * get_index_isvalid
+ *
+ *             Given the index OID, return pg_index.indisvalid.
+ */
+bool
+get_index_isvalid(Oid index_oid)
+{
+       bool            isvalid;
+       HeapTuple       tuple;
+       Form_pg_index rd_index;
+
+       tuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(index_oid));
+       if (!HeapTupleIsValid(tuple))
+               elog(ERROR, "cache lookup failed for index %u", index_oid);
+
+       rd_index = (Form_pg_index) GETSTRUCT(tuple);
+       isvalid = rd_index->indisvalid;
+       ReleaseSysCache(tuple);
+
+       return isvalid;
+}
index 21ec4cbd355c2f7cac08cf793ee8b8d98659d2f4..c9d6aa7f1c365759330b0a25c3dea282f3331157 100644 (file)
@@ -179,6 +179,7 @@ extern char *get_namespace_name_or_temp(Oid nspid);
 extern Oid     get_range_subtype(Oid rangeOid);
 extern Oid     get_range_collation(Oid rangeOid);
 extern bool    get_index_isreplident(Oid index_oid);
+extern bool get_index_isvalid(Oid index_oid);
 extern bool get_index_isclustered(Oid index_oid);
 
 #define type_is_array(typid)  (get_element_type(typid) != InvalidOid)
index 2544f389e2fb7bc9991ad6bad3554a314a238ec7..3eec572e917405542e23677b918d646b6a4e0422 100644 (file)
@@ -1537,3 +1537,38 @@ select indexrelid::regclass, indisvalid,
 (5 rows)
 
 drop table parted_inval_tab;
+-- Check setup of indisvalid across a complex partition tree on index
+-- creation.  If one index in a partition index is invalid, so should its
+-- partitioned index.
+create table parted_isvalid_tab (a int, b int) partition by range (a);
+create table parted_isvalid_tab_1 partition of parted_isvalid_tab
+  for values from (1) to (10) partition by range (a);
+create table parted_isvalid_tab_2 partition of parted_isvalid_tab
+  for values from (10) to (20) partition by range (a);
+create table parted_isvalid_tab_11 partition of parted_isvalid_tab_1
+  for values from (1) to (5);
+create table parted_isvalid_tab_12 partition of parted_isvalid_tab_1
+  for values from (5) to (10);
+-- create an invalid index on one of the partitions.
+insert into parted_isvalid_tab_11 values (1, 0);
+create index concurrently parted_isvalid_idx_11 on parted_isvalid_tab_11 ((a/b));
+ERROR:  division by zero
+-- The previous invalid index is selected, invalidating all the indexes up to
+-- the top-most parent.
+create index parted_isvalid_idx on parted_isvalid_tab ((a/b));
+select indexrelid::regclass, indisvalid,
+       indrelid::regclass, inhparent::regclass
+  from pg_index idx left join
+       pg_inherits inh on (idx.indexrelid = inh.inhrelid)
+  where indexrelid::regclass::text like 'parted_isvalid%'
+  order by indexrelid::regclass::text collate "C";
+           indexrelid           | indisvalid |       indrelid        |           inhparent           
+--------------------------------+------------+-----------------------+-------------------------------
+ parted_isvalid_idx             | f          | parted_isvalid_tab    | 
+ parted_isvalid_idx_11          | f          | parted_isvalid_tab_11 | parted_isvalid_tab_1_expr_idx
+ parted_isvalid_tab_12_expr_idx | t          | parted_isvalid_tab_12 | parted_isvalid_tab_1_expr_idx
+ parted_isvalid_tab_1_expr_idx  | f          | parted_isvalid_tab_1  | parted_isvalid_idx
+ parted_isvalid_tab_2_expr_idx  | t          | parted_isvalid_tab_2  | parted_isvalid_idx
+(5 rows)
+
+drop table parted_isvalid_tab;
index 6f8233dd4457104606d95a0c1ef970da8464dac6..5e89a0bcd9b079e6d36effcb2e777e6c9f6bd62e 100644 (file)
@@ -817,3 +817,29 @@ select indexrelid::regclass, indisvalid,
   where indexrelid::regclass::text like 'parted_inval%'
   order by indexrelid::regclass::text collate "C";
 drop table parted_inval_tab;
+
+-- Check setup of indisvalid across a complex partition tree on index
+-- creation.  If one index in a partition index is invalid, so should its
+-- partitioned index.
+create table parted_isvalid_tab (a int, b int) partition by range (a);
+create table parted_isvalid_tab_1 partition of parted_isvalid_tab
+  for values from (1) to (10) partition by range (a);
+create table parted_isvalid_tab_2 partition of parted_isvalid_tab
+  for values from (10) to (20) partition by range (a);
+create table parted_isvalid_tab_11 partition of parted_isvalid_tab_1
+  for values from (1) to (5);
+create table parted_isvalid_tab_12 partition of parted_isvalid_tab_1
+  for values from (5) to (10);
+-- create an invalid index on one of the partitions.
+insert into parted_isvalid_tab_11 values (1, 0);
+create index concurrently parted_isvalid_idx_11 on parted_isvalid_tab_11 ((a/b));
+-- The previous invalid index is selected, invalidating all the indexes up to
+-- the top-most parent.
+create index parted_isvalid_idx on parted_isvalid_tab ((a/b));
+select indexrelid::regclass, indisvalid,
+       indrelid::regclass, inhparent::regclass
+  from pg_index idx left join
+       pg_inherits inh on (idx.indexrelid = inh.inhrelid)
+  where indexrelid::regclass::text like 'parted_isvalid%'
+  order by indexrelid::regclass::text collate "C";
+drop table parted_isvalid_tab;