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
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 |