A new C function `get_partition_root`.

On Fri, Jul 17, 2026 at 9:41 AM Peter Smith <smithpb2250@gmail.com> wrote:

This patch introduces a new C function `get_partition_root`.

WHY?

1. It is the C equivalent of the already-existing SQL function
`pg_partition_root`

2. Some code is not interested in the partition "ancestors", yet it
still had to declare/fetch/free an ancestors list to obtain the root
relid at llast_oid(ancestors).

3. The logical replication FOR TABLES IN SCHEMA EXCEPT (...) patches
[1] (still in development) have many more code fragments like this
where the logic wants only the partition root OID -- This function
would simplify all those.

I agree that the patch you mentioned will benefit from such a
function. So before we commit that patch ( FOR TABLES IN SCHEMA
EXCEPT), this function addition can be considered. A few comments:

1)
+ * Note: This should only be called when it is known that the relation is a
+ * partition (see function get_partition_ancestors).

The comment does not match the actual flow through
pg_partition_root(). This function can also be reached when the input
relation is not a partition but a partitioned table (the root itself).
We should update the comment to reflect the behavior enforced by
check_rel_can_be_partition() in pg_partition_root().

2)
Looking only at get_partition_root(), it doesn't seem to be
self-contained in validating that the caller has supplied a valid
input.

Since this internal function is expected to be used more widely (in
the thread you referenced), I would expect it to handle all input
cases consistently:

--Return NULL if the input relation is neither a partition nor a
partitioned table. Or may be Assert will be better?
--Return the same relation if the input is already the partitioned
table (the root).
--Return the root partitioned table if the input is a partition.

The second and third cases currently work as expected. But the first
and second cases are not distinguishable. Both a regular table and a
partitioned table results in the input relation's OID being returned
by get_partition_root (verified by pg_partition_root() by bypassing
check_rel_can_be_partition()). I think get_partition_root needs some
improvement for validaitng and distinguishing above cases.

thanks
Shveta

Hi Shveta.

Thanks for your review!

On Tue, Jul 28, 2026 at 3:41 PM shveta malik <shveta.malik@gmail.com> wrote:

...

I agree that the patch you mentioned will benefit from such a
function. So before we commit that patch ( FOR TABLES IN SCHEMA
EXCEPT), this function addition can be considered. A few comments:

1)
+ * Note: This should only be called when it is known that the relation is a
+ * partition (see function get_partition_ancestors).

The comment does not match the actual flow through
pg_partition_root(). This function can also be reached when the input
relation is not a partition but a partitioned table (the root itself).
We should update the comment to reflect the behavior enforced by
check_rel_can_be_partition() in pg_partition_root().

OK. I've changed the function comment to say the specified relid may
be a partitioned table.

2)
Looking only at get_partition_root(), it doesn't seem to be
self-contained in validating that the caller has supplied a valid
input.

Since this internal function is expected to be used more widely (in
the thread you referenced), I would expect it to handle all input
cases consistently:

--Return NULL if the input relation is neither a partition nor a
partitioned table. Or may be Assert will be better?
--Return the same relation if the input is already the partitioned
table (the root).
--Return the root partitioned table if the input is a partition.

The second and third cases currently work as expected. But the first
and second cases are not distinguishable. Both a regular table and a
partitioned table results in the input relation's OID being returned
by get_partition_root (verified by pg_partition_root() by bypassing
check_rel_can_be_partition()). I think get_partition_root needs some
improvement for validaitng and distinguishing above cases.

You are right, there was some quirky behaviour for regular tables.

The original code fragment of pg_get_partition_root (below)
------
if (ancestors == NIL)
PG_RETURN_OID(relid);
------
only made sense because the relid had already been verified up-front
to be a partition or partitioned table.

I wanted my new function to be more like `get_partition_ancestors`,
which has expectations on the caller, rather than adding a lot of
up-front parameter validation like `pg_get_partition_root` has. OTOH,
I agree with your point that a regular table should not be returned
from this function, so I have added the necessary Assert to fix that.

~~~

PSA patch v2.

This still passes make check-world, and I verified different inputs
manually using the same technique that you described.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

On Wed, Jul 29, 2026 at 9:53 AM Peter Smith <smithpb2250@gmail.com> wrote:

Hi Shveta.

Thanks for your review!

On Tue, Jul 28, 2026 at 3:41 PM shveta malik <shveta.malik@gmail.com> wrote:

...

I agree that the patch you mentioned will benefit from such a
function. So before we commit that patch ( FOR TABLES IN SCHEMA
EXCEPT), this function addition can be considered. A few comments:

1)
+ * Note: This should only be called when it is known that the relation is a
+ * partition (see function get_partition_ancestors).

The comment does not match the actual flow through
pg_partition_root(). This function can also be reached when the input
relation is not a partition but a partitioned table (the root itself).
We should update the comment to reflect the behavior enforced by
check_rel_can_be_partition() in pg_partition_root().

OK. I've changed the function comment to say the specified relid may
be a partitioned table.

2)
Looking only at get_partition_root(), it doesn't seem to be
self-contained in validating that the caller has supplied a valid
input.

Since this internal function is expected to be used more widely (in
the thread you referenced), I would expect it to handle all input
cases consistently:

--Return NULL if the input relation is neither a partition nor a
partitioned table. Or may be Assert will be better?
--Return the same relation if the input is already the partitioned
table (the root).
--Return the root partitioned table if the input is a partition.

The second and third cases currently work as expected. But the first
and second cases are not distinguishable. Both a regular table and a
partitioned table results in the input relation's OID being returned
by get_partition_root (verified by pg_partition_root() by bypassing
check_rel_can_be_partition()). I think get_partition_root needs some
improvement for validaitng and distinguishing above cases.

You are right, there was some quirky behaviour for regular tables.

The original code fragment of pg_get_partition_root (below)
------
if (ancestors == NIL)
PG_RETURN_OID(relid);
------
only made sense because the relid had already been verified up-front
to be a partition or partitioned table.

I wanted my new function to be more like `get_partition_ancestors`,
which has expectations on the caller, rather than adding a lot of
up-front parameter validation like `pg_get_partition_root` has. OTOH,
I agree with your point that a regular table should not be returned
from this function, so I have added the necessary Assert to fix that.

~~~

PSA patch v2.

This still passes make check-world, and I verified different inputs
manually using the same technique that you described.

Thanks for addressing comments Peter.

1)
CREATE TABLE p (id int);
CREATE TABLE c () INHERITS (p);

select pg_partition_root('c') retruns NULL, while if we experiment
with get_partition_root('c') alone, it returns 'p'.

The diffefrence in behaviour boils down to get_partition_ancestors()
returning valid ancestor list for 'c' (as you stated that this expects
caller to ensure input is a parition). So what do you suggest here?
More Assert in ' if (ancestors)' or leave it like this?

2)
- List    *ancestors = get_partition_ancestors(relid);
+ Oid root_relid = get_partition_root(relid);
  const char *attname = get_attname(relid, attnum, false);

- relid = llast_oid(ancestors);
+ relid = root_relid;

Should we get rid of root_relid and simply do:
relid = get_partition_root(relid);

thanks
Shveta

On Wed, Jul 29, 2026 at 3:23 PM shveta malik <shveta.malik@gmail.com> wrote:
...

1)
CREATE TABLE p (id int);
CREATE TABLE c () INHERITS (p);

select pg_partition_root('c') retruns NULL, while if we experiment
with get_partition_root('c') alone, it returns 'p'.

The diffefrence in behaviour boils down to get_partition_ancestors()
returning valid ancestor list for 'c' (as you stated that this expects
caller to ensure input is a parition). So what do you suggest here?
More Assert in ' if (ancestors)' or leave it like this?

No need to leave as-is. The sanity check in the C function can be
easily moved to cover this case too, so have done this.

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?

2)
- List    *ancestors = get_partition_ancestors(relid);
+ Oid root_relid = get_partition_root(relid);
const char *attname = get_attname(relid, attnum, false);

- relid = llast_oid(ancestors);
+ relid = root_relid;

Should we get rid of root_relid and simply do:
relid = get_partition_root(relid);

Touché. Done as suggested.

~~~

PSA patch v3.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

On Wed, Jul 29, 2026 at 2:28 PM Peter Smith <smithpb2250@gmail.com> wrote:

On Wed, Jul 29, 2026 at 3:23 PM shveta malik <shveta.malik@gmail.com> wrote:
...

1)
CREATE TABLE p (id int);
CREATE TABLE c () INHERITS (p);

select pg_partition_root('c') retruns NULL, while if we experiment
with get_partition_root('c') alone, it returns 'p'.

The diffefrence in behaviour boils down to get_partition_ancestors()
returning valid ancestor list for 'c' (as you stated that this expects
caller to ensure input is a parition). So what do you suggest here?
More Assert in ' if (ancestors)' or leave it like this?

No need to leave as-is. The sanity check in the C function can be
easily moved to cover this case too, so have done this.

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.

On Wed, Jul 29, 2026 at 7:18 PM shveta malik <shveta.malik@gmail.com> wrote:

On Wed, Jul 29, 2026 at 2:28 PM Peter Smith <smithpb2250@gmail.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

On Thu, Jul 30, 2026 at 5:35 AM Peter Smith <smithpb2250@gmail.com> wrote:

On Wed, Jul 29, 2026 at 7:18 PM shveta malik <shveta.malik@gmail.com> wrote:

On Wed, Jul 29, 2026 at 2:28 PM Peter Smith <smithpb2250@gmail.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.

Looks good.

thanks
Shveta

On Jul 30, 2026, at 08:04, Peter Smith <smithpb2250@gmail.com> wrote:

On Wed, Jul 29, 2026 at 7:18 PM shveta malik <shveta.malik@gmail.com> wrote:

On Wed, Jul 29, 2026 at 2:28 PM Peter Smith <smithpb2250@gmail.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.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Thu, Jul 30, 2026 at 11:16 AM Chao Li <li.evan.chao@gmail.com> wrote:

On Jul 30, 2026, at 08:04, Peter Smith <smithpb2250@gmail.com> wrote:

On Wed, Jul 29, 2026 at 7:18 PM shveta malik <shveta.malik@gmail.com> wrote:

On Wed, Jul 29, 2026 at 2:28 PM Peter Smith <smithpb2250@gmail.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

On Thu, Jul 30, 2026 at 3:46 PM Chao Li <li.evan.chao@gmail.com> wrote:

On Jul 30, 2026, at 08:04, Peter Smith <smithpb2250@gmail.com> wrote:

On Wed, Jul 29, 2026 at 7:18 PM shveta malik <shveta.malik@gmail.com> wrote:

On Wed, Jul 29, 2026 at 2:28 PM Peter Smith <smithpb2250@gmail.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.

Hi Chao-San.

Thanks for reporting that issue.

I've dealt with that now by exposing the `detach_pending` so now the
`get_partition_root` can know whether a detach was the cause of
ancestors == NIL.

I also added another flag `even_if_detached` to `get_partition_root`
so callers can decide what to do if a detach is in progress. That's
analogous to other code that has a similar parameter.

It seems ok using Shveta's 3 sessions example for testing.

~~~

I was wondering if I should have taken this further and similarly
changed the `get_partition_ancestors` signature to optionally expose a
detach_pending flag.
That could be used to protect some of the existing code that AFAICT
has potential to crash:
ancestors = get_partition_ancestors(relid);
last_ancestor_relid = llast_oid(ancestors);

But, that would be a bit more invasive so I wanted some 2nd opinions
before going too far.

Thoughts?

======
Kind Regards,
Peter Smith
Fujitsu Australia

On Aug 3, 2026, at 11:59, Peter Smith <smithpb2250@gmail.com> wrote:

On Thu, Jul 30, 2026 at 3:46 PM Chao Li <li.evan.chao@gmail.com> wrote:

On Jul 30, 2026, at 08:04, Peter Smith <smithpb2250@gmail.com> wrote:

On Wed, Jul 29, 2026 at 7:18 PM shveta malik <shveta.malik@gmail.com> wrote:

On Wed, Jul 29, 2026 at 2:28 PM Peter Smith <smithpb2250@gmail.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.

Hi Chao-San.

Thanks for reporting that issue.

I've dealt with that now by exposing the `detach_pending` so now the
`get_partition_root` can know whether a detach was the cause of
ancestors == NIL.

I also added another flag `even_if_detached` to `get_partition_root`
so callers can decide what to do if a detach is in progress. That's
analogous to other code that has a similar parameter.

It seems ok using Shveta's 3 sessions example for testing.

Thanks for updating the patch.

Given the comment:
```
+ * Note: This should only be called when it is known that the relation is a
+ * partition or partitioned table.
```

Does it make sense to add an Assert for that, like:
```
Assert(get_rel_relispartition(relid) ||
RELKIND_HAS_PARTITIONS(get_rel_relkind(relid)));
```

Then, maybe we don’t need the final sanity check assert.

Otherwise v5 looks good to me. The new parameter even_if_detached matches the existing get_partition_parent().

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Mon, Aug 3, 2026 at 5:47 PM Chao Li <li.evan.chao@gmail.com> wrote:

...

Thanks for updating the patch.

Given the comment:
```
+ * Note: This should only be called when it is known that the relation is a
+ * partition or partitioned table.
```

Does it make sense to add an Assert for that, like:
```
Assert(get_rel_relispartition(relid) ||
RELKIND_HAS_PARTITIONS(get_rel_relkind(relid)));
```

Then, maybe we don’t need the final sanity check assert.

Otherwise v5 looks good to me. The new parameter even_if_detached matches the existing get_partition_parent().

Until now I had been resisting doing up front validation because:
1. then the delegation from the SQL `pg_partition_root` would be doing
2x validation.
2. get_partition_ancestors was not doing any validation

Anyway, in v6 I've done the following:
1. changed to add a wrapper to avoid 2x validation when called from
the SQL function
2. decided not to worry about validation overheads because Assert is
NOP for a production build anyhow

~~

PSA v6

======
Kind Regards,
Peter Smith.
Fujitsu Australia

Hi,

I'd say this looks okay, but why do you need get_partition_root_guts()
exposed in partition.h? In fact, it's not clear to me why you need a
second routine at all. Why isn't enough to have just get_partition_root()?

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Estoy de acuerdo contigo en que la verdad absoluta no existe...
El problema es que la mentira sí existe y tu estás mintiendo" (G. Lama)

On Mon, Aug 3, 2026 at 2:34 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:

Hi,

I'd say this looks okay, but why do you need get_partition_root_guts()
exposed in partition.h? In fact, it's not clear to me why you need a
second routine at all. Why isn't enough to have just get_partition_root()?

I think to avoid performing the validation twice in
pg_partition_root(): first via check_rel_can_be_partition(), and then
again in get_partition_root (see [1]+ /* Validate relid is member of a partition tree */ + Assert(get_rel_relispartition(relid) || + RELKIND_HAS_PARTITIONS(get_rel_relkind(relid)));), get_partition_root_guts() is
introduced and exposed in partition.h.

[1]:
+ /* Validate relid is member of a partition tree */
+ Assert(get_rel_relispartition(relid) ||
+    RELKIND_HAS_PARTITIONS(get_rel_relkind(relid)));

thanks
Shveta

On 2026-Aug-03, shveta malik wrote:

On Mon, Aug 3, 2026 at 2:34 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:

I'd say this looks okay, but why do you need get_partition_root_guts()
exposed in partition.h? In fact, it's not clear to me why you need a
second routine at all. Why isn't enough to have just get_partition_root()?

I think to avoid performing the validation twice in
pg_partition_root(): first via check_rel_can_be_partition(), and then
again in get_partition_root (see [1]), get_partition_root_guts() is
introduced and exposed in partition.h.

[1]:
+ /* Validate relid is member of a partition tree */
+ Assert(get_rel_relispartition(relid) ||
+    RELKIND_HAS_PARTITIONS(get_rel_relkind(relid)));

This seems wrong actually (having this as an assert rather than
if/elog), because it means no validation at all occur on normal builds.
Surely that's the wrong thing?

Redundant asserts are no cause for concern IMO. I would certainly not
create two routines just to avoid an assert, which is nothing at all in
production builds.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
Thou shalt check the array bounds of all strings (indeed, all arrays), for
surely where thou typest "foo" someone someday shall type
"supercalifragilisticexpialidocious" (5th Commandment for C programmers)

On Mon, Aug 3, 2026 at 3:55 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:

On 2026-Aug-03, shveta malik wrote:

On Mon, Aug 3, 2026 at 2:34 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:

I'd say this looks okay, but why do you need get_partition_root_guts()
exposed in partition.h? In fact, it's not clear to me why you need a
second routine at all. Why isn't enough to have just get_partition_root()?

I think to avoid performing the validation twice in
pg_partition_root(): first via check_rel_can_be_partition(), and then
again in get_partition_root (see [1]), get_partition_root_guts() is
introduced and exposed in partition.h.

[1]:
+ /* Validate relid is member of a partition tree */
+ Assert(get_rel_relispartition(relid) ||
+    RELKIND_HAS_PARTITIONS(get_rel_relkind(relid)));

This seems wrong actually (having this as an assert rather than
if/elog), because it means no validation at all occur on normal builds.

yes, thats right, no validation for normal builds.

Surely that's the wrong thing?

Here we have tried to mimic the behaviour of
'get_partition_ancestors'. get_partition_ancestors() does not have
even basic Asserts in place and completely relies on user to verify
the input. So we were not sure whether we should use elog in
get_partition_root().

Also do you think get_partition_root() should give elog even when
'even_if_detached'=false and 'detach is pending' similar to what we do
in get_partition_parent (see [1]/messages/by-id/CAJpy0uAdF3K0QDANkT2HgiMzPzOpHKiMXQsM8bZfaaaCKYfxtQ@mail.gmail.com).

[1]: /messages/by-id/CAJpy0uAdF3K0QDANkT2HgiMzPzOpHKiMXQsM8bZfaaaCKYfxtQ@mail.gmail.com
get_partition_parent:
if (detach_pending && !even_if_detached)
elog(ERROR, "relation %u has no parent because it's
being detached",
relid);

Redundant asserts are no cause for concern IMO. I would certainly not
create two routines just to avoid an assert, which is nothing at all in
production builds.

Okay.
~

Since we are on this topic, Peter and myself had discussed it
internally, whether get_partition_ancestors() should be enhanced to
have similar input validations and also should accept
'even_if_detached' input? This will help avoiding potential issues
caused by get_partition_ancestors() on HEAD. One of such issues is
discussed at [1]/messages/by-id/CAJpy0uAdF3K0QDANkT2HgiMzPzOpHKiMXQsM8bZfaaaCKYfxtQ@mail.gmail.com

[1]: /messages/by-id/CAJpy0uAdF3K0QDANkT2HgiMzPzOpHKiMXQsM8bZfaaaCKYfxtQ@mail.gmail.com
/messages/by-id/CAJpy0uAdF3K0QDANkT2HgiMzPzOpHKiMXQsM8bZfaaaCKYfxtQ@mail.gmail.com

thanks
Shveta