| 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
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | Zhijie Hou | 2026-11-05 09:42:06 | Re: Proposal: Conflict log history table for Logical Replication |