Re: A new C function `get_partition_root`.

On Thu, Jul 30, 2026 at 11:16 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>
>
> > On Jul 30, 2026, at 08:04, Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> >
> > On Wed, Jul 29, 2026 at 7:18 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >>
> >> On Wed, Jul 29, 2026 at 2:28 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> > ...
> >>> BTW, although I think using sanity Assert was correct, just in case
> >>> there is some unanticipated way to reach the C function with a bad
> >>> relid, I've changed to use an errlog(ERROR).
> >>> Thoughts?
> >>
> >> I don't immediately see any such possibility. I feel Assert is better.
> >>
> >
> > OK. PSA v4, which is the same as v3, but uses Assert instead of elog.
> >
> > ======
> > Kind Regards,
> > Peter Smith.
> > Fujitsu Australia
> > <v4-0001-Add-C-function-get_partition_root.patch>
>
>
> I just reviewed v4 and got a doubt:
> ```
> + /* Sanity check: The root must be a partitioned table */
> + Assert(RELKIND_HAS_PARTITIONS(get_rel_relkind(root_relid)));
> ```
>
> Looking into get_partition_ancestors(), it returns NIL in two cases:
>
> 1) No more parent
> 2) detach_pending is true
>
> Case 1 is an expected case, I doubt case 2 may fire the Assert.
>
> Say, partitioned table p has a leaf partition p1, now p1 is being detached. get_partition_ancestors(p1) may return NIL because of detach_pending, so root_relid is set to p1, but p1 is not a partitioned table, then this Assert is fired.
>

Good catch. I can reproduce the Assert using this:

CREATE TABLE t1 (a int) PARTITION BY RANGE (a);
CREATE TABLE t1_part PARTITION OF t1 FOR VALUES FROM (1) TO (10);

Session A:
BEGIN;
SELECT * FROM t1;
--do not commit

Session B:
--This blocks on 'A' post detach but before making some catalog changes
ALTER TABLE t1 DETACH PARTITION t1_part CONCURRENTLY;

Session C, while B is blocked:
select pg_partition_root('t1_part');
TRAP: failed Assert("RELKIND_HAS_PARTITIONS(get_rel_relkind(root_relid))"),
File: "partition.c", Line: 151, PID: 165955

get_partition_ancestors() returned NULL. The flow did not return from
check_rel_can_be_partition() itself as t1_part was still marked as
parition (relispartition=true).
~~

How can we fix it? Should we get rid of Assert and rely on caller to
pass correct input similar to how get_partition_ancestors() expect?

thanks
Shveta