On Sat, Aug 15, 2026 at 8:54 PM Mihail Nikalayeu
<[email protected]> wrote:
>
> Hello, everyone! (Added CC to authors of patches that introduced the
> issues described below, sorry if it is duplicate for you).
>
> I was working on stress-suite and discovered a few crashes caused
> mainly by the same issue.
>
> DETACH PARTITION ... CONCURRENTLY marks the partition and commits, and
> only then waits. If that wait is interrupted the mark remains.
> Only PARTITION ... FINALIZE clears it. Reaching this needs no race,
> and the state outlives the command that made it.
>
> In that state pg_class still says relispartition, while
> get_partition_ancestors() already reports nothing.
>
> Three places read the former as meaning the latter is not empty and
> ask for its last element, which causes an assert() or NPE:
>
> - get_rel_sync_entry()
> - getIdentitySequence()
> - RelationBuildPublicationDesc()
>
> The last one is a v19 regression, so I think it may be added to the
> open items list.
>
> Patches are attached, one for each place; each includes a test that
> reproduces the crash.


Thank You Mikhail for the patches.

I would like to discuss the design and the intent of patch001 alone.

1. My initial thought was whether get_partition_ancestors() itself
should explicitly handle partitions undergoing concurrent detach,
similar to get_partition_parent(). This would let all callers
recognize the detaching state and handle it appropriately, rather than
discovering a NIL ancestor list only after hitting an Assert or crash.
Have you considered it this way?

2. I am also unclear on the intent. When a partition is undergoing
concurrent detach, do we ultimately want to publish its changes or
not?

~~

I am slightly confused by the code comments and the test case behavior:

a)
In get_rel_sync_entry(), the comment states that the partition is
handled like a standalone table once its ancestors report NIL. If it
is treated as a normal table, what prevents it from being published
under a FOR ALL TABLES publication? Specifically, if
get_rel_sync_entry() allows it, why does the test case comment note
"the part1 change is not replicated"? Which exact code part filters
out or blocks this partition from being streamed in that state?

b)
Inconsistency with is_table_publishable_in_publication():  We updated
get_rel_sync_entry() to handle a NULL ancestor list, but
is_table_publishable_in_publication() remains unchanged. Won't
is_table_publishable_in_publication() return false for this partition
during a concurrent detach for a pub created for 'FOR TBALE ROOT'? If
so, aren't get_rel_sync_entry() and
is_table_publishable_in_publication() out of sync, and what practical
side-effects does this discrepancy cause?

thanks
Shveta


Reply via email to