Re: BUG #18500: Detaching a partition with an index manually attached to the parent's index triggers Assert

On Wed, Jun 12, 2024 at 01:49:32PM +0800, Tender Wang wrote:
> Hi, all
>
> I found another crash case as below:
> CREATE TABLE t (a integer) PARTITION BY RANGE (a);
> CREATE TABLE tp (a integer PRIMARY KEY);
> CREATE UNIQUE INDEX t_a_idx ON t (a);
> ALTER TABLE t ATTACH PARTITION tp FOR VALUES FROM (0) TO (1000);
> ALTER TABLE t DETACH PARTITION tp;
>
> If index of parent is not created by adding constraint, it will trigger
> assert fail.

The 4th command is I think incorrect to attach "t_a_idx" to "tp"'s
primary key index because "t" does not have an equivalent entry in
pg_constraint.  So that does not address the actual issue.

> I look through the DetachPartitionFinalize(). The logic of detach indexes
> finds a pg_inherits tuple,
> then it goes to detach the constraint between parent and child. But in
> AttachPartitionEnsureIndexes(),
> it check whether constraint of parent is valid or not. If it is valid, then
> the code will build constraint
> connection between parent and child.
>
> I think we can do this check in DetachPartitionFinalize(). We skip to
> detach the constraint if parent actually
> does not have one. I try to fix theses issue in attached patch.

Are you sure that this is correct?  This still makes "tp_pkey" a
partition of "t_a_idx" while the parent table "t" has no constraint
equivalent to "tp"'s primary key.  It seems to me that the correct
answer in your command sequence is to *not* make "tp_pkey" a partition
of the partitioned index "t_a_idx", and leave it alone.  Attaching

test=# alter table tp drop constraint tp_pkey;
ERROR:  cannot drop index tp_pkey because index t_a_idx requires it

"tp_pkey" as a partition of "t_a_idx" is only OK as long as this index
is used in a constraint equivalent to "tp_pkey".

Having a PK on a partition while the parent does not have one is
covered by a regression test in indexing.sql (see "Constraint-related
indexes" and "primary key on child is okay").
--
Michael