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

At Tue, 21 Dec 2021 09:30:42 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote in
> 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.

I agree to the discussion above thus it seems to me right that we just
remove the assertion. We never have a partition of un-attachable
relkind so such kind of relations are surely rejected just after. The
assertion on "== RELKIND_PARTITIONED_TABLE" is still valid since we
don't reach there for the case of the traditional inheritance.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center