Re: BUG #17339: Assert failed on attempt to detach a sequence concurrently

On Mon, Dec 20, 2021 at 09:57:47PM +0100, Peter Eisentraut wrote:
> On 20.12.21 13:38, Michael Paquier wrote:
>> Attached is a patch, with some regression tests.
>
> Is it possible for child tables in partitioned tables to have different
> ownerships? Then this change would introduce a new failure mode.

Hmm. Yes, you are right here. It is possible to change the ownership
of a partition after it gets attached, so this could cause a
regression. I recalled that this was not possible, so my memories
were wrong.

At the end, perhaps we should just remove the assertion that assumes
which relkind is right for the partition, as of:
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -15013,8 +15013,6 @@ MarkInheritDetached(Relation child_rel,
Relation parent_rel)
HeapTuple inheritsTuple;
bool found = false;

- Assert(child_rel->rd_rel->relkind == RELKIND_RELATION ||
- child_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE);
Assert(parent_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE);

This would also fail when attempting to detach a foreign table, as
well, and these are legal relkinds in a partition tree. Once we do
that, we fall down into the same failure as for the non-concurrent
mode in RemoveInheritance(), telling that the relation is not a member
of the partition tree.
--
Michael