Re: Add support for specifying tables in pg_createsubscriber.

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: Shubham Khanna <khannashubham1197(at)gmail(dot)com>
Cc: "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add support for specifying tables in pg_createsubscriber.
Date: 2025-09-24 23:29:52
Message-ID: CAHut+PsCQxWoPh-UXBUWu=6Pc6GuEQ4wnHZtDOwUnZN=krMxvQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Shubham.

On Wed, Sep 24, 2025 at 4:37 PM Shubham Khanna
<khannashubham1197(at)gmail(dot)com> wrote:
>
...
>
> In the attached patch, I have removed that test case. It’s not really
> valid to combine multiple switches in this way, since using
> --publication together with --clean=publications is effectively the
> same as running pg_createsubscriber twice (once with each switch).
> That makes the extra combination test redundant, so I have dropped it.

I disagree with the "it's not really valid to combine..." part. IMO,
it should be perfectly valid to mix multiple switches if the tool
accepts them; otherwise, the tool ought to give an error for any
invalid combinations. OTOH, I agree that there is no point in
burdening this patch with redundant test cases.

//////////

Here are some v11 review comments.

======
src/sgml/ref/pg_createsubscriber.sgml

1.
+ <para>
+ If a publication with the specified name already exists on the
publisher,
+ it will be reused as-is with its current configuration, including its
+ table list, row filters, column filters, and all other settings.
+ If a publication does not exist, it will be created automatically with
+ <literal>FOR ALL TABLES</literal>.
+ </para>

Below is a minor rewording suggestion for the first sentence by AI,
which I felt was a small improvement.

SUGGESTION:
If a publication with the specified name already exists on the
publisher, it will be reused with its current configuration, including
its table list, row filters, and column filters.

======
.../t/040_pg_createsubscriber.pl

2.
+# Confirm ALL publications were removed by --clean=publications
+my $remaining_pubs_db1 = $node_s3->safe_psql($db1,
+ "SELECT pubname FROM pg_publication ORDER BY pubname");
+is($remaining_pubs_db1, '',
+ 'all publications were removed from db1 by --clean=publications');
+
+my $remaining_pubs_db2 = $node_s3->safe_psql($db2,
+ "SELECT pubname FROM pg_publication ORDER BY pubname");
+is($remaining_pubs_db2, '',
+ 'all publications were removed from db2 by --clean=publications');
+

Since you are expecting no results, the "ORDER BY pubname" clauses are
redundant here. In hindsight, since you expect zero results, just
"SELECT COUNT(*) FROM pg_publications;" would also be ok.

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Chao Li 2025-09-24 23:56:42 Re: Trivial fix for comment of function table_tuple_lock
Previous Message Michael Paquier 2025-09-24 23:28:02 Re: [PATCH] Add tests for Bitmapset