Re: [PATCH v1] Fix initial sync of zero-column published tables

From: vignesh C <vignesh21(at)gmail(dot)com>
To: Mingjie Gao <mingjie(dot)gmj(at)gmail(dot)com>
Cc: Peter Smith <smithpb2250(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH v1] Fix initial sync of zero-column published tables
Date: 2026-11-05 10:07:47
Message-ID: CALDaNm055zMVU2wqZMq=FdNFbFSJTy9h68vYFY6=4fHtu_VUpA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, 5 Nov 2026 at 08:18, Mingjie Gao <mingjie(dot)gmj(at)gmail(dot)com> wrote:
>
> Hi Peter,
>
> Thanks for reproducing the issue and testing the patch.
>
>
> > Now that BeginCopyFrom is just a thin "wrapper" should all that
> > commentary describing each of the function parameters be moved to
> > BeginCopyFromExtended function-comment where all the work is done?
>
> Agreed. I've moved the parameter descriptions to the comment for BeginCopyFromExtended().
>
>
> > The TAP test code for this patch seems more elaborate than usual.
> > AFAIK it is normal to accept that regressions show up as a timeout.
>
> I've simplified the test to use wait_for_subscription_sync(). It still checks both text and binary modes, and retains the incremental insert check alongside the initial sync check.
>
> Please find the updated v2 patch attached.

Thanks for the patch.
Few minor suggestions for the test:
1) Can we create the table in publisher and subscriber outside the
loop instead of doing it twice, similarly the publication can be
created outside the loop:
+# A zero-column publisher still sends rows. Subscriber-only columns must
+# get their defaults during initial COPY as well as incremental replication.
+foreach my $binary ('false', 'true')
+{
+ $node_publisher->safe_psql(
+ 'postgres', q{
+ CREATE TABLE tab_nocols ();
+ INSERT INTO tab_nocols DEFAULT VALUES;
+ INSERT INTO tab_nocols DEFAULT VALUES;
+ CREATE PUBLICATION pub_nocols FOR TABLE tab_nocols
+ WITH (publish = 'insert');
+ });
+ $node_subscriber->safe_psql('postgres',
+ "CREATE TABLE tab_nocols (local_value text DEFAULT
'subscriber-default')"
+ );

2) With the first changes, we can truncate the tables instead of dropping them:
+ $node_publisher->safe_psql('postgres',
+ 'DROP PUBLICATION pub_nocols; DROP TABLE tab_nocols');
+ $node_subscriber->safe_psql('postgres', 'DROP TABLE tab_nocols');

3) Till which branch will this patch be applied?

Regards,
Vignesh

In response to

Browse pgsql-hackers by date

  From Date Subject
Previous Message Zhijie Hou 2026-11-05 09:42:06 Re: Proposal: Conflict log history table for Logical Replication