Re: Add support for specifying tables in pg_createsubscriber.

On Mon, Dec 1, 2025 at 9:48 AM Peter Smith <smithpb2250(at)gmail(dot)com> 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