On Mon, Dec 1, 2025 at 9:48 AM Peter Smith <[email protected]> wrote: > > Hi Shubham. > > Some minor review comments for v20-0001. > > ====== > 1. > +# Test publication reuse and creation behavior with --dry-run. > +# This should reuse existing 'test_pub_existing' and create new > 'test_pub_new', > +# demonstrating mixed publication handling without actual changes. > +($stdout, $stderr) = run_command( > + [ > + 'pg_createsubscriber', > + '--verbose', > + '--dry-run', > + '--pgdata' => $node_s2->data_dir, > + '--publisher-server' => $node_p->connstr($db1), > + '--socketdir' => $node_s2->host, > + '--subscriber-port' => $node_s2->port, > + '--database' => $db1, > + '--database' => $db2, > + '--publication' => 'test_pub_existing', > + '--publication' => 'test_pub_new', > + ], > + 'run pg_createsubscriber --dry-run on node S2'); > + > +like( > + $stderr, > + qr/use existing publication "test_pub_existing"/, > + 'dry-run logs reuse of existing publication'); > +like( > + $stderr, > + qr/dry-run: would create publication "test_pub_new"/, > + 'dry-run logs creation of new publication'); > + > +# Verify dry-run did not modify publisher state > +$result = $node_p->safe_psql($db1, > + "SELECT pubname FROM pg_publication ORDER BY pubname"); > +is( $result, qq(pub1 > +test_pub1 > +test_pub2 > +test_pub_existing), > + "existing publication remains unchanged after dry-run in db1"); > + > +$result = $node_p->safe_psql($db2, > + "SELECT pubname FROM pg_publication ORDER BY pubname"); > +is($result, 'pub2', "dry-run did not actually create publications in db2"); > + > > 1a. > To verify that the dry-run is benign, I felt this test case should be > doing a "SELECT pubname FROM pg_publication ORDER BY pubname" *before* > the dry-run. And then do the same SELECT *after* the dry-run. Then > just check that those before/after results are identical. > > I guess you can ignore this review comment if you prefer to hardwire > the expected *after* values (like the current patch does), but IMO the > actual values are unimportant -- what matters is only that nothing got > changed. > > ~ > > 1b. > Those 2 test name messages ought to be almost identical. e.g. > > BEFORE > "existing publication remains unchanged after dry-run in db1" > "dry-run did not actually create publications in db2" > > SUGGESTION > "dry-run does not remove or create publications in db1" > "dry-run does not remove or create publications in db2" >
Here is the latest v21-0001 patch to address the above comments. ====== Kind Regards, Peter Smith. Fujitsu Australia
v21-0001-Support-existing-publications-in-pg_createsubscr.patch
Description: Binary data
