| From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
|---|---|
| To: | Shubham Khanna <khannashubham1197(at)gmail(dot)com> |
| Cc: | vignesh C <vignesh21(at)gmail(dot)com>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(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-11-30 22:48:18 |
| Message-ID: | CAHut+PtoAyG0RzWYd2CPxh7LTVYy3P9PwgRf54eB03v8Wuitsg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jelte Fennema-Nio | 2025-11-30 22:57:12 | Re: Early December Commitfest app release |
| Previous Message | David E. Wheeler | 2025-11-30 21:16:34 | Re: PATCH: jsonpath string methods: lower, upper, initcap, l/r/btrim, replace, split_part |