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"

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


Reply via email to