From a66c346e2a6ac9e36dbde5d6e89cd0dcf5d3b27b Mon Sep 17 00:00:00 2001 From: Dilip Kumar Date: Sun, 1 Nov 2026 06:11:09 +0000 Subject: [PATCH v2] Ignore invalid indexes on partitions during CREATE INDEX When DefineIndex() creates an index on a partitioned table (without ONLY), it recurses to each partition and scans existing indexes on the partition to see if a matching, unattached index can be reused and attached to the newly created parent index. Previously, this check only compared the structural index definition via CompareIndexInfo() without checking pg_index.indisvalid. As a result, an invalid index left behind on a partition by a previously failed CREATE [UNIQUE] INDEX CONCURRENTLY (with indisvalid = false and possibly indisready = false), or an incomplete partitioned index on a sub-partitioned table created with CREATE [UNIQUE] INDEX ON ONLY, would be attached as a child index, marking the parent partitioned index indisvalid = false and skipping index creation on the partition (or its sub-partitions). In particular, for CREATE UNIQUE INDEX (or ALTER TABLE ADD UNIQUE / PRIMARY KEY), the command would report success even when the partition contained duplicate values, and when indisready = false on the reused leaf index, subsequent inserts and updates would not enforce the unique constraint either (since ExecInsertIndexTuples() skips !indisready indexes). Furthermore, once attached to the parent index, the broken child index could no longer be dropped directly with DROP INDEX due to its partition dependency. Commit e90e9275f56 fixed the analogous issue in AttachPartitionEnsureIndexes() for ALTER TABLE ... ATTACH PARTITION by skipping invalid indexes on the partition being attached. Apply the same rule in DefineIndex(): skip existing partition indexes where !indisvalid, so that a new valid index is built on the partition instead. Reported-by: chunling qin Authored-by: Dilip Kumar Reviewed-by: Kirill Reshke --- doc/src/sgml/ref/create_index.sgml | 6 +- src/backend/commands/indexcmds.c | 76 ++++++---------------- src/test/regress/expected/indexing.out | 89 +++++++++++++++----------- src/test/regress/sql/indexing.sql | 32 +++++---- 4 files changed, 93 insertions(+), 110 deletions(-) diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml index bb7505d171b..1aca4add5dc 100644 --- a/doc/src/sgml/ref/create_index.sgml +++ b/doc/src/sgml/ref/create_index.sgml @@ -752,14 +752,14 @@ Indexes: When CREATE INDEX is invoked on a partitioned table, the default behavior is to recurse to all partitions to ensure they all have matching indexes. - Each partition is first checked to determine whether an equivalent + Each partition is first checked to determine whether a valid equivalent index already exists, and if so, that index will become attached as a partition index to the index being created, which will become its parent index. - If no matching index exists, a new index will be created and + If no matching valid index exists, a new index will be created and automatically attached; the name of the new index in each partition will be determined as if no index name had been specified in the - command. + command. Invalid indexes on the partitions are skipped. If the ONLY option is specified, no recursion is done, and the index is marked invalid. (ALTER INDEX ... ATTACH PARTITION marks the index diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 5a0312fe772..7dec7f66c1e 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1339,7 +1339,6 @@ DefineIndex(ParseState *pstate, { int nparts = partdesc->nparts; Oid *part_oids = palloc_array(Oid, nparts); - bool invalidate_parent = false; Relation parentIndex; TupleDesc parentDesc; @@ -1391,9 +1390,9 @@ DefineIndex(ParseState *pstate, parentDesc = RelationGetDescr(rel); /* - * For each partition, scan all existing indexes; if one matches - * our index definition and is not already attached to some other - * parent index, attach it to the one we just created. + * For each partition, scan all existing indexes; if one is valid, + * matches our index definition, and is not already attached to + * some other parent index, attach it to the one we just created. * * If none matches, build a new index by calling ourselves * recursively with the same options (except for the index name). @@ -1458,6 +1457,14 @@ DefineIndex(ParseState *pstate, continue; cldidx = index_open(cldidxid, lockmode); + + /* If this index is invalid, can't use it */ + if (!cldidx->rd_index->indisvalid) + { + index_close(cldidx, lockmode); + continue; + } + cldIdxInfo = BuildIndexInfo(cldidx); if (CompareIndexInfo(cldIdxInfo, indexInfo, cldidx->rd_indcollation, @@ -1496,9 +1503,6 @@ DefineIndex(ParseState *pstate, createdConstraintId, childRelid); - if (!cldidx->rd_index->indisvalid) - invalidate_parent = true; - found = true; /* @@ -1532,7 +1536,6 @@ DefineIndex(ParseState *pstate, if (!found) { IndexStmt *childStmt; - ObjectAddress childAddr; /* * Build an IndexStmt describing the desired child index @@ -1553,62 +1556,23 @@ DefineIndex(ParseState *pstate, Assert(GetUserId() == child_save_userid); SetUserIdAndSecContext(root_save_userid, root_save_sec_context); - childAddr = - DefineIndex(NULL, /* original pstate not applicable */ - childRelid, childStmt, - InvalidOid, /* no predefined OID */ - indexRelationId, /* this is our child */ - createdConstraintId, - -1, - is_alter_table, check_rights, - check_not_in_use, - skip_build, quiet); + DefineIndex(NULL, /* original pstate not applicable */ + childRelid, childStmt, + InvalidOid, /* no predefined OID */ + indexRelationId, /* this is our child */ + createdConstraintId, + -1, + 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; } free_attrmap(attmap); } index_close(parentIndex, lockmode); - - /* - * The pg_index row we inserted for this index was marked - * indisvalid=true. But if we attached an existing index that is - * invalid, this is incorrect, so update our row to invalid too. - */ - if (invalidate_parent) - { - Relation pg_index = table_open(IndexRelationId, RowExclusiveLock); - HeapTuple tup, - newtup; - - tup = SearchSysCache1(INDEXRELID, - ObjectIdGetDatum(indexRelationId)); - if (!HeapTupleIsValid(tup)) - elog(ERROR, "cache lookup failed for index %u", - indexRelationId); - newtup = heap_copytuple(tup); - ((Form_pg_index) GETSTRUCT(newtup))->indisvalid = false; - CatalogTupleUpdate(pg_index, &tup->t_self, newtup); - ReleaseSysCache(tup); - table_close(pg_index, RowExclusiveLock); - heap_freetuple(newtup); - - /* - * CCI here to make this update visible, in case this recurses - * across multiple partition levels. - */ - CommandCounterIncrement(); - } } /* diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out index 4a0a652e9f6..734f3801733 100644 --- a/src/test/regress/expected/indexing.out +++ b/src/test/regress/expected/indexing.out @@ -292,18 +292,8 @@ create table idxpart21 partition of idxpart2 for values from (100) to (200); create table idxpart22 partition of idxpart2 for values from (200) to (300); create index on idxpart22 (a); create index on only idxpart2 (a); -create index on idxpart (a); --- Here we expect that idxpart1 and idxpart2 have a new index, but idxpart21 --- does not; also, idxpart22 is not attached. -\d idxpart1 - Table "public.idxpart1" - Column | Type | Collation | Nullable | Default ---------+---------+-----------+----------+--------- - a | integer | | | -Partition of: idxpart FOR VALUES FROM (0) TO (100) -Indexes: - "idxpart1_a_idx" btree (a) - +-- Here we expect that idxpart2 has a new index, but idxpart21 does not; +-- also, idxpart22 is not attached. \d idxpart2 Partitioned table "public.idxpart2" Column | Type | Collation | Nullable | Default @@ -326,13 +316,11 @@ select indexrelid::regclass, indrelid::regclass, inhparent::regclass from pg_index idx left join pg_inherits inh on (idx.indexrelid = inh.inhrelid) where indexrelid::regclass::text like 'idxpart%' order by indexrelid::regclass::text collate "C"; - indexrelid | indrelid | inhparent ------------------+-----------+--------------- - idxpart1_a_idx | idxpart1 | idxpart_a_idx + indexrelid | indrelid | inhparent +-----------------+-----------+----------- idxpart22_a_idx | idxpart22 | - idxpart2_a_idx | idxpart2 | idxpart_a_idx - idxpart_a_idx | idxpart | -(4 rows) + idxpart2_a_idx | idxpart2 | +(2 rows) alter index idxpart2_a_idx attach partition idxpart22_a_idx; select indexrelid::regclass, indrelid::regclass, inhparent::regclass @@ -341,11 +329,9 @@ where indexrelid::regclass::text like 'idxpart%' order by indexrelid::regclass::text collate "C"; indexrelid | indrelid | inhparent -----------------+-----------+---------------- - idxpart1_a_idx | idxpart1 | idxpart_a_idx idxpart22_a_idx | idxpart22 | idxpart2_a_idx - idxpart2_a_idx | idxpart2 | idxpart_a_idx - idxpart_a_idx | idxpart | -(4 rows) + idxpart2_a_idx | idxpart2 | +(2 rows) -- attaching idxpart22 is not enough to set idxpart22_a_idx valid ... alter index idxpart2_a_idx attach partition idxpart22_a_idx; @@ -374,6 +360,20 @@ Indexes: "idxpart2_a_idx" btree (a) Number of partitions: 2 (Use \d+ to list them.) +create index on idxpart (a); +select indexrelid::regclass, indrelid::regclass, inhparent::regclass + from pg_index idx left join pg_inherits inh on (idx.indexrelid = inh.inhrelid) +where indexrelid::regclass::text like 'idxpart%' + order by indexrelid::regclass::text collate "C"; + indexrelid | indrelid | inhparent +-----------------+-----------+---------------- + idxpart1_a_idx | idxpart1 | idxpart_a_idx + idxpart21_a_idx | idxpart21 | idxpart2_a_idx + idxpart22_a_idx | idxpart22 | idxpart2_a_idx + idxpart2_a_idx | idxpart2 | idxpart_a_idx + idxpart_a_idx | idxpart | +(5 rows) + drop table idxpart; -- When a table is attached a partition and it already has an index, a -- duplicate index should not get created, but rather the index becomes @@ -1661,9 +1661,8 @@ 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. +-- Check that invalid indexes are not selected when creating an index on a +-- partitioned table. 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); @@ -1674,26 +1673,38 @@ create table parted_isvalid_tab_11 partition of parted_isvalid_tab_1 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)); +insert into parted_isvalid_tab_11 values (1, 1), (1, 1); +create unique index concurrently parted_isvalid_idx_11 on parted_isvalid_tab_11 (a); +ERROR: could not create unique index "parted_isvalid_idx_11" +DETAIL: Key (a)=(1) is duplicated. +-- The previous invalid index is skipped, so attempting to build a new index +-- on the partitioned table fails on the duplicate rows. +create unique index parted_isvalid_idx on parted_isvalid_tab (a); +ERROR: could not create unique index "parted_isvalid_tab_11_a_idx" +DETAIL: Key (a)=(1) is duplicated. +delete from parted_isvalid_tab_11; +-- Now creating the index on the partitioned table succeeds and creates new +-- valid indexes, discarding the invalid index created previously. +create unique index parted_isvalid_idx on parted_isvalid_tab (a); +-- Uniqueness is enforced on the partition: +insert into parted_isvalid_tab values (1, 1), (1, 2); +ERROR: duplicate key value violates unique constraint "parted_isvalid_tab_11_a_idx" +DETAIL: Key (a)=(1) already exists. 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_a_b_idx - parted_isvalid_tab_12_a_b_idx | t | parted_isvalid_tab_12 | parted_isvalid_tab_1_a_b_idx - parted_isvalid_tab_1_a_b_idx | f | parted_isvalid_tab_1 | parted_isvalid_idx - parted_isvalid_tab_2_a_b_idx | t | parted_isvalid_tab_2 | parted_isvalid_idx -(5 rows) + indexrelid | indisvalid | indrelid | inhparent +-----------------------------+------------+-----------------------+---------------------------- + parted_isvalid_idx | t | parted_isvalid_tab | + parted_isvalid_idx_11 | f | parted_isvalid_tab_11 | + parted_isvalid_tab_11_a_idx | t | parted_isvalid_tab_11 | parted_isvalid_tab_1_a_idx + parted_isvalid_tab_12_a_idx | t | parted_isvalid_tab_12 | parted_isvalid_tab_1_a_idx + parted_isvalid_tab_1_a_idx | t | parted_isvalid_tab_1 | parted_isvalid_idx + parted_isvalid_tab_2_a_idx | t | parted_isvalid_tab_2 | parted_isvalid_idx +(6 rows) drop table parted_isvalid_tab; -- Check state of replica indexes when attaching a partition. diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql index bbcfb365281..7c56b2f0aa2 100644 --- a/src/test/regress/sql/indexing.sql +++ b/src/test/regress/sql/indexing.sql @@ -165,10 +165,8 @@ create table idxpart21 partition of idxpart2 for values from (100) to (200); create table idxpart22 partition of idxpart2 for values from (200) to (300); create index on idxpart22 (a); create index on only idxpart2 (a); -create index on idxpart (a); --- Here we expect that idxpart1 and idxpart2 have a new index, but idxpart21 --- does not; also, idxpart22 is not attached. -\d idxpart1 +-- Here we expect that idxpart2 has a new index, but idxpart21 does not; +-- also, idxpart22 is not attached. \d idxpart2 \d idxpart21 select indexrelid::regclass, indrelid::regclass, inhparent::regclass @@ -187,6 +185,11 @@ alter index idxpart2_a_idx attach partition idxpart22_a_idx; create index on idxpart21 (a); alter index idxpart2_a_idx attach partition idxpart21_a_idx; \d idxpart2 +create index on idxpart (a); +select indexrelid::regclass, indrelid::regclass, inhparent::regclass + from pg_index idx left join pg_inherits inh on (idx.indexrelid = inh.inhrelid) +where indexrelid::regclass::text like 'idxpart%' + order by indexrelid::regclass::text collate "C"; drop table idxpart; -- When a table is attached a partition and it already has an index, a @@ -909,9 +912,8 @@ select indexrelid::regclass, indisvalid, 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. +-- Check that invalid indexes are not selected when creating an index on a +-- partitioned table. 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); @@ -922,11 +924,17 @@ create table parted_isvalid_tab_11 partition of parted_isvalid_tab_1 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)); +insert into parted_isvalid_tab_11 values (1, 1), (1, 1); +create unique index concurrently parted_isvalid_idx_11 on parted_isvalid_tab_11 (a); +-- The previous invalid index is skipped, so attempting to build a new index +-- on the partitioned table fails on the duplicate rows. +create unique index parted_isvalid_idx on parted_isvalid_tab (a); +delete from parted_isvalid_tab_11; +-- Now creating the index on the partitioned table succeeds and creates new +-- valid indexes, discarding the invalid index created previously. +create unique index parted_isvalid_idx on parted_isvalid_tab (a); +-- Uniqueness is enforced on the partition: +insert into parted_isvalid_tab values (1, 1), (1, 2); select indexrelid::regclass, indisvalid, indrelid::regclass, inhparent::regclass from pg_index idx left join -- 2.56.0.317.g454f8302e1-goog