Re: [PATCH] Fix replica identity mismatch for partitioned tables with publish_via_partition_root

Hi, Mikhail!

On Tue, Aug 12, 2025 at 3:02 PM Mikhail Kharitonov <
mikhail(dot)kharitonov(dot)dev(at)gmail(dot)com> wrote:
> I've rebased this series onto the latest master.
>
> Changes in v3:
>
> Patch 1/2 adds two new functions: logicalrep_write_update_extended,
> logicalrep_write_delete_extended to logicalproto.
> These are now used in pgoutput and allow correct old-tuple flag
handling
> when publish_via_partition_root = true.
> The old functions remain as wrappers to preserve compatibility.
> A short documentation note was added to explain the new behaviour.
>
> Patch 2/2 moves the TAP test into a separate commit,
> so the code change and test are isolated.

Thank you for catching this. I've some notes about your patches.

Assert(leafrel->rd_rel->relreplident == REPLICA_IDENTITY_DEFAULT ||
leafrel->rd_rel->relreplident == REPLICA_IDENTITY_FULL ||
leafrel->rd_rel->relreplident == REPLICA_IDENTITY_INDEX);

This assert was in the beginning of the function. You've moved it into the
middle of message forming. If that's an intentional change, it must be
motivated.

# 2: second partition has REPLICA IDENTITY DEFAULT - only keys expected.
if ($wal =~ /U.*K.*second/s)
{
pass("Tag K correctly used for partition with REPLICA IDENTITY
DEFAULT");
}
elsif ($wal =~ /(U.*O.*second)/s)
{
my $blk = $1;
my $count = () = $blk =~ /second/g;
is($count, 2, "Tag O used but this partition with REPLICA IDENTITY
DEFAULT");
}

This check is very lossy. I think it must be explicit on what count of
which records it expects. Currently, the test silently passes if no
branches of above are taken. Also, regexes don't look reliable, they could
easily match cross-record. You might either write more reliable regexes,
or even better parse individual records before matching them.

Also, please note that in your test subscriber does nothing expect waiting
for sync. If you're introducing subscriber, it worths to at least check
its final state.

------
Regards,
Alexander Korotkov
Supabase