On Monday, March 10, 2025 9:12 PM Kuroda, Hayato <kuroda.hay...@fujitsu.com> wrote: > > > Currently, only the leaf partition is invalidated when the published > > table is partitioned. However, I think pgoutput could cache both the > > partitioned table and the leaf partition table as relsync entries. > > > > For INSERT/UPDATE/DELETE on a partitioned table, only the leaf > > partition's relsync entry is used in pgoutput, but the TRUNCATE > > references the parent table's relsync entry. > > I think your analysis is correct. PSA new version. Below part contains my > analysis. > > In ExecuteTruncate(), if the specified relation has children, all of them are > checked via find_all_inheritors() and listed as target. Also, > ExecuteTruncateGuts() serializes both a parent and children in > XLOG_HEAP_TRUNCATE WAL record. > Decoding layer passes relations as-is. These facts mean that output plugins > can store caches on the memory.
Thanks for updating the patch. I have reviewed patch 0001 and did not find issues, aside from a few issues for code comments that were mentioned in Amit's email. Here are some analyses and insights gathered during the review of 0001: The functions and variables in the 0001 patch uses 'Relsync' (e.g., RegisterRelsyncInvalidation) instead of the longer 'RelsyncCache'. After internal discussions, we think it's OK, as using 'RelsyncCache' could unnecessarily lengthen the names. Furthermore, considering we're introducing a new invalidation message for the RelationSyncCache within pgoutput, which is an output plugin, we discussed whether a more general invalidation name should be adopted in case other output plugins might use it. However, after reviewing all the plugins listed in Wiki[1], we did not find any other plugins that reference the built-in publication catalog. Therefore, this scenario appears to be uncommon. Additionally, the current naming convention is sufficiently intuitive for output plugin developers. Hence, we feel it is reasonable to retain the existing names. For patch 0002, I think the implementation could be improved. The current patch introduces a new function, RenamePublication, to replace the existing generic approach in ExecRenameStmt->AlterObjectRename_internal. However, this creates inconsistency because the original code uses AccessExclusiveLock for publication renaming, making concurrent renaming impossible. The current patch seems to overlook this aspect. Additionally, introducing a new function results in code duplication, which can be avoided. After further consideration, handling the publication rename separately seems unnecessary, given it requires only sending a few extra invalidation messages. Therefore, I suggest reusing the generic handling and simply extending AlterObjectRename_internal to include the additional invalidation messages. I've attached a diff with the proposed changes for 0002. Best Regards, Hou zj
0001-refactor.patch
Description: 0001-refactor.patch