On Wed, Dec 10, 2025 at 10:32 AM Euler Taveira <[email protected]> wrote: > > On Sun, Dec 7, 2025, at 11:52 PM, Peter Smith wrote: > > > > OK. Done as suggested in v23. > > > > I took another look at this patch. I noticed that it increased the test time > in > 3 or 4s. I started writing the suggestions to this email but since it > increases > a lot, it is better to provide a patch and explain the main points. Regarding > the tests, there are 2 new nodes (one used to test this feature -- node_s2 -- > and another one with no real function -- node_s3). There is no need to add new > nodes, just use the created publication into one of the existing tests. That's > what I did (see test_pub3). I also added a new table to ensure the publication > only contains tbl1 and not not_replicated table. I removed the dry run part > because it only increases the total time and doesn't improve coverage. > Finally, > coverage stays the same and a small increment in the execution time (less than > 1s). > > $ tail -n 8 src/bin/pg_basebackup/pg_createsubscriber.c.gcov.out > File 'pg_createsubscriber.c' > Lines executed:83.81% of 951 > Branches executed:93.15% of 467 > Taken at least once:74.73% of 467 > Calls executed:73.48% of 675 > Creating 'pg_createsubscriber.c.gcov' > > Lines executed:83.81% of 951 > > I also changed the code a bit. In particular, I don't like > check_publication_exists and find_publication seems cleaner to me so I renamed > it. I replaced the psprintf because the pattern is to use PQExpBuffer > functions > for queries. Added pg_catalog to the query. I changed pg_fatal with > pg_log_error > plus disconnect_database -- see previous discussion about it. I added a > comment > explaining why made_publication = false. Finally, I refactored the logic that > drops publications; the if (!drop_all_pubs || dry_run) is rather confusing. > Documentation was also changed a bit. One paragraph instead of two because it > is > talking about the same topic. > > Here is your v23 plus the fixes that I described above. IMO it is good to go > so > I set it to Ready for Committer. Additional comments are always welcome. > >
Hi Euler. I checked your change suggestions and they LGTM; now they are all merged into the latest patch. I made only some very minor tweaks to the test code. BTW, I didn't know what "previous discussion" you were referring to about the pg_fatal change you made. PSA v25. ====== Kind Regards, Peter Smith. Fujitsu Australia
v25-0001-Support-existing-publications-in-pg_createsubscr.patch
Description: Binary data
