Partitioned index marked valid while a partition is pending detach
Hi hackers,
A partitioned unique index can be marked valid while one partition has
no index. That partition then accepts duplicate keys.
create table t (a int) partition by list (a);
create table p1 partition of t for values in (1);
create table p2 partition of t for values in (2);
create table p3 partition of t for values in (3);
create unique index ti on only t (a);
create unique index p1_a on p1 (a);
create unique index p2_a on p2 (a);
alter index ti attach partition p1_a;
In another session, open a transaction and read from t. Run "alter
table t detach partition p1 concurrently". It waits for that
transaction. Cancel it, and p1 stays pending detach. Then:
alter index ti attach partition p2_a;
Now ti is valid, but p3 has no index.
Root cause:
validatePartitionedIndex() marks ti valid once it has as many valid
child indexes as t has partitions. It sees two child indexes, p1_a and
p2_a. It also sees two partitions, p2 and p3. It skips p1 because p1 is
pending detach. So two equals two and ti becomes valid, even though p3
has no index.
So we just need a one liner fix which is use false to count all partitions.
0001 is the fix and 0002 is the test on the existing detach test.
This need to go back to 14
Thanks,
Shihao
Attachments:
t254547_1v1-0002-Test-partitioned-index-validity-while-a-partition-is.patchapplication/octet-stream; name=v1-0002-Test-partitioned-index-validity-while-a-partition-is.patchDownload+63-2
v1-0001-Count-partitions-pending-detach-when-validating-a-pa.patchapplication/octet-stream; name=v1-0001-Count-partitions-pending-detach-when-validating-a-pa.patchDownload+4-2
On Tue, Nov 03, 2026 at 09:46:35PM -0500, shihao zhong wrote:
A partitioned unique index can be marked valid while one partition has
no index. That partition then accepts duplicate keys.In another session, open a transaction and read from t. Run "alter
table t detach partition p1 concurrently". It waits for that
transaction. Cancel it, and p1 stays pending detach. Then:alter index ti attach partition p2_a;
Now ti is valid, but p3 has no index.
Obviously that's wrong. indisvalid on a partitioned index should only
be switched to true if all its partition indexes are valid. If a
partitioned table has an index but no partitions, the flag could be
true.
validatePartitionedIndex() marks ti valid once it has as many valid
child indexes as t has partitions. It sees two child indexes, p1_a and
p2_a. It also sees two partitions, p2 and p3. It skips p1 because p1 is
pending detach. So two equals two and ti becomes valid, even though p3
has no index.So we just need a one liner fix which is use false to count all partitions.
0001 is the fix and 0002 is the test on the existing detach test.
Yeah, I think that you are right here. I was wondering if the test
should be simplified a bit, without a third partition, but what you
are doing may be cleaner overall. The last "s2detachfinal s1valid"
steps in the new permutation don't seem strongly necessary if you want
to check your case. Or do you specifically want to have coverage of a
FINALIZE with an attached index? I would cut these two steps, just
wondering if you have an argument in favor of keeping them.
On top of that, indexcmds.c uses RelationGetPartitionDesc(true) as
well. Could that be a source of problems when creating a unique index
on a partitioned table, for example? It also looks like we could
finish with a broken unique constraint with duplicated data inserted
due to a miscount of the number of partitions, with a indisflag marked
as valid when we should not mark it as such? Perhaps this should have
a separate investigation.
--
Michael
Hi Michael,
Thanks for reviewing my patch..
The last "s2detachfinal s1valid" steps in the new permutation don't
seem strongly necessary if you want to check your case. Or do you
specifically want to have coverage of a FINALIZE with an attached
index? I would cut these two steps, just wondering if you have an
argument in favor of keeping them.
Agreed. Those two steps check the index after FINALIZE,
and they pass without the fix. So v2 drops them.
On top of that, indexcmds.c uses RelationGetPartitionDesc(true) as
well. Could that be a source of problems when creating a unique index
on a partitioned table, for example?
Yes. If CREATE UNIQUE INDEX runs while p1 is pending detach, p1 gets
no index but the parent index is marked valid. An old snapshot can
then insert duplicate keys into p1 through t. CREATE INDEX ON ONLY
has the same problem.
Building the index on p1 does not work, because the rest of ALTER
TABLE skips p1. ADD PRIMARY KEY would fail, since p1 gets no NOT NULL
constraint. So 0003 makes CREATE INDEX on t fail while a partition
is pending detach, with the same hint ALTER TABLE gives.
This is a little trick because it did change the behavior, I dont think user
want to create an incorrect PK and running FINALIZE first is easy,
so I think this is fine to backport.
Thanks,
Shihao
Attachments:
t254547_3v2-0002-Test-partitioned-index-validity-while-a-partition.patchapplication/octet-stream; name=v2-0002-Test-partitioned-index-validity-while-a-partition.patchDownload+54-2
v2-0001-Count-partitions-pending-detach-when-validating-a.patchapplication/octet-stream; name=v2-0001-Count-partitions-pending-detach-when-validating-a.patchDownload+4-2
v2-0003-Reject-CREATE-INDEX-while-a-partition-has-an-inco.patchapplication/octet-stream; name=v2-0003-Reject-CREATE-INDEX-while-a-partition-has-an-inco.patchDownload+50-1
On Wed, Nov 04, 2026 at 11:54:26PM -0500, shihao zhong wrote:
Agreed. Those two steps check the index after FINALIZE,
and they pass without the fix. So v2 drops them.
The comments still feel a bit confusing to me, but that can be tweaked
infinitely. Saying that, the permutation steps look much cleaner now.
I'll go process this part, barring objections and/or comments of any
kind.
On top of that, indexcmds.c uses RelationGetPartitionDesc(true) as
well. Could that be a source of problems when creating a unique index
on a partitioned table, for example?Yes. If CREATE UNIQUE INDEX runs while p1 is pending detach, p1 gets
no index but the parent index is marked valid. An old snapshot can
then insert duplicate keys into p1 through t. CREATE INDEX ON ONLY
has the same problem.This is a little trick because it did change the behavior, I dont think user
want to create an incorrect PK and running FINALIZE first is easy,
so I think this is fine to backport.
+ if (PartitionHasPendingDetach(pd->oids[i]))
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("cannot create index on partitioned table \"%s\"",
+ RelationGetRelationName(rel)),
+ errdetail("Partition \"%s\" has an incomplete detach.",
+ get_rel_name(pd->oids[i])),
+ errhint("Use ALTER TABLE ... DETACH PARTITION ... FINALIZE to complete the pending detach operation.")));
Still returning an error may be better than breaking a tree
consistency and even worse, break constraints under-the-hood. I am
not sure if there is a better option here.. Alvaro, what do you
think?
--
Michael
On 2026-Nov-05, Michael Paquier wrote:
On Wed, Nov 04, 2026 at 11:54:26PM -0500, shihao zhong wrote:
This is a little trick because it did change the behavior, I dont think user
want to create an incorrect PK and running FINALIZE first is easy,
so I think this is fine to backport.+ if (PartitionHasPendingDetach(pd->oids[i])) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot create index on partitioned table \"%s\"", + RelationGetRelationName(rel)), + errdetail("Partition \"%s\" has an incomplete detach.", + get_rel_name(pd->oids[i])), + errhint("Use ALTER TABLE ... DETACH PARTITION ... FINALIZE to complete the pending detach operation.")));Still returning an error may be better than breaking a tree
consistency and even worse, break constraints under-the-hood. I am
not sure if there is a better option here.. Alvaro, what do you
think?
I think this idea is good generally speaking, but we shouldn't limit it
to create index, but instead apply it to pretty much any DDL that you
want to run on such a table. The risk of inconsistencies is just too
high and there's no functional reason to let users do anything before
finishing that detach.
I'm happy to listen to example scenarios where my argument fails, to
assess if any exceptions are needed.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Por suerte hoy explotó el califont porque si no me habría muerto
de aburrido" (Papelucho)