On Fri, Sep 4, 2026 at 11:45 AM shveta malik <[email protected]> wrote:
>
>
> 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?
>

Yes, that is another way of dealing with the issue but it will be
better to fix the issue without changing the interface. Alvaro also
mentioned the same [1]. We can discuss changing the interface as a
separate HEAD-only patch.

> 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?
>

It depends. For example, if it is ALL TABLES publication then the
in-process detach partition should be replicated as an individual
table.

> ~~
>
> 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?
>

Unless I am missing something, the comment needs adjustment.

> 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?
>

I have tried by reproducing it with a test but it appears to be an
issue that should be fixed as well.

[1] - https://www.postgresql.org/message-id/aoCXNrzbb-yryPg9%40alvherre.pgsql

-- 
With Regards,
Amit Kapila.


Reply via email to