On Fri, Mar 14, 2025 at 5:43 PM Nisha Moond <nisha.moond...@gmail.com> wrote: > > Hi Shubham, > > Here are a few comments for the v12 patch. > > doc/src/sgml/ref/pg_createsubscriber.sgml : > > 1. > + <para> > + For all source server non-template databases create subscriptions for > + create subscriptions for databases with the same names on the > + target server. > > is “create subscriptions for” repeated? The sentence doesn’t make sense. >
Fixed. > 2. Typo > + switches. This option cannot be together with <option>--all</option>. > > /cannot be together/cannot be used together/ > ~~~ > Fixed. > 3. src/bin/pg_basebackup/pg_createsubscriber.c : > + /* Establish a connection to the PostgreSQL server */ > + conn = connect_database(opt->pub_conninfo_str, true); > > I think comment will be more clear if say “ Establish a connection to > the primary server */ or “source server” > ~~~ > Fixed. > src/bin/pg_basebackup/t/042_all_option.pl : > > 4. As per Ashutosh's comment at [1], tests need to be added to verify > logical replication behavior after using the --all option. > ~~~~ > > Please refer to the attached top-up fix patch, which includes the > above changes along with some cosmetic fixes in the test file > 042_all_option.pl. > > [1] > https://www.postgresql.org/message-id/CAExHW5uJHYAge99oS_iPfGWwZ_eCr2xFCNnifQkGs2GXeMQKGQ%40mail.gmail.com > > -- Fixed. The attached patch contains the suggested changes. Thanks and regards, Shubham Khanna.
v13-0001-Enhance-pg_createsubscriber-to-fetch-and-append-.patch
Description: Binary data
