On Mon, Sep 7, 2026 at 3:56 PM Amit Kapila <[email protected]> wrote: > > 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.
Yes, that's what I thought initially, but the test case validates otherwise. See the test in patch-001: the publication is for ALL TABLES, but the partition being detached is not replicated. I think this patch focuses on fixing the crash without clarifying the intent for various cases: whether to publish or not publish the detached partition. > > > ~~ > > > > 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.
