Horton

Re: Proposal: Conflict log history table for Logical Replication

Peter Smith <smithpb2250@gmail.com>

View thread
From: Peter Smith <smithpb2250@gmail.com>
To: vignesh C <vignesh21@gmail.com>
Cc: shveta malik <shveta.malik@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Nisha Moond <nisha.moond412@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, shveta malik <shvetamalik@gmail.com>
Date: 2026-05-26T06:28:43Z
Lists: pgsql-hackers 
Hi Vignesh.

I had only one trivial review comment for v40-0001/0002 combined.

======
src/backend/commands/subscriptioncmds.c

1.
+ if (OidIsValid(subconflictlogrelid))
+ {
+ ObjectAddress object;
+ char *conflictrelname;
+
+ /* Drop any dependent conflict log table */
+ conflictrelname = get_rel_name(subconflictlogrelid);

That "Drop any..." comment doesn't have anything to do with the
statement that follows it. I think this comment belongs outside the
if.

e.g.
/* Drop any dependent conflict log table */
if (OidIsValid(subconflictlogrelid))
{
  ...
}

======
Kind Regards,
Peter Smith.
Fujitsu Australia