Thread

Re: Re: Add support for specifying tables in pg_createsubscriber.

vignesh C <vignesh21@gmail.com> — 2025-12-04T02:56:42Z

On Thu, 4 Dec 2025 at 07:47, Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Wed, Dec 3, 2025 at 4:47 PM tianbing <tian_bing_0531@163.com> wrote:
> >
> > Hi， Peter，
> > I have reviewed the v21 patch and noticed that there seems to be a memory leak.
> >
> > +static bool
> > +check_publication_exists(PGconn *conn, const char *pubname, const char *dbname)
> > +{
> > + PGresult *res;
> > + bool exists;
> > + char *query;
> > +
> > + query = psprintf("SELECT 1 FROM pg_publication WHERE pubname = %s",
> > + PQescapeLiteral(conn, pubname, strlen(pubname)));
> > + res = PQexec(conn, query);
> > +
> > + if (PQresultStatus(res) != PGRES_TUPLES_OK)
> > + pg_fatal("could not check for publication \"%s\" in database \"%s\": %s",
> > + pubname, dbname, PQerrorMessage(conn));
> > +
> > + exists = (PQntuples(res) == 1);
> > +
> > + PQclear(res);
> > + pg_free(query);
> > + return exists;
> > +}
> >
> > The PQescapeLiteral() function through malloc to allocate memmory，and should be free by PQfreemem。
> >
> > I suggest making the following modifications：
> >
> > + char *pub = PQescapeLiteral(conn, pubname, strlen(pubname);
> > + query = psprintf("SELECT 1 FROM pg_publication WHERE pubname = %s", pub);
> > ......
> > + PQfreemem(pub);
> >
>
> Fixed in v22.

I felt that instead of adding a new test case, let's try to combine
with the existing test case, something like below:
diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
index e2a78f28c72..981da668507 100644
--- a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
+++ b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
@@ -443,10 +443,14 @@ is(scalar(() = $stderr =~ /would create the
replication slot/g),
 is(scalar(() = $stderr =~ /would create subscription/g),
        3, "verify subscriptions are created for all databases");

+# Create user-defined publications.
+$node_p->safe_psql($db2,
+       "CREATE PUBLICATION test_pub_existing FOR TABLE tbl2");
+
 # Run pg_createsubscriber on node S.  --verbose is used twice
 # to show more information.
-# In passing, also test the --enable-two-phase option and
-# --clean option
+# In passing, also test the --enable-two-phase option,
+# --clean option and specifying an existing publication test_pub_existing.
 command_ok(
        [
                'pg_createsubscriber',
@@ -457,7 +461,7 @@ command_ok(
                '--socketdir' => $node_s->host,
                '--subscriber-port' => $node_s->port,
                '--publication' => 'pub1',
-               '--publication' => 'pub2',
+               '--publication' => 'test_pub_existing',
                '--replication-slot' => 'replslot1',
                '--replication-slot' => 'replslot2',
                '--database' => $db1,
@@ -502,6 +506,17 @@ $result = $node_s->safe_psql(
 ));
 is($result, qq(0), 'pre-existing subscription was dropped');

+# Get subscription names and publications
+$result = $node_s->safe_psql(
+       'postgres', qq(
+    SELECT subname, subpublications FROM pg_subscription WHERE
subname ~ '^pg_createsubscriber_'
+));
+like(
+       $result,
+       qr/^pg_createsubscriber_\d+_[0-9a-f]+ \|\{pub1\}\n
+        pg_createsubscriber_\d+_[0-9a-f]+ \|\{test_pub_existing\}$/x,
+       "Subscription and publication name are ok");
+
 # Get subscription names
 $result = $node_s->safe_psql(
        'postgres', qq(

Thoughts?

Regards,
Vignesh