On Thu, 4 Dec 2025 at 07:47, Peter Smith <[email protected]> wrote:
>
> On Wed, Dec 3, 2025 at 4:47 PM tianbing <[email protected]> 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
