pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: CREATE SUBSCRIPTION ... SERVER.
9+ messages / 4 participants
[nested] [flat]
Commits from this thread:
2026-03-24 f16f5d608c GetSubscription(): use per-object memory context.
2026-03-11 547c15f9f8 Fix use of volatile.
* pgsql: CREATE SUBSCRIPTION ... SERVER.
@ 2026-03-06 16:44 Jeff Davis <jdavis@postgresql.org>
0 siblings, 1 reply; 9+ messages in thread
From: Jeff Davis @ 2026-03-06 16:44 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
CREATE SUBSCRIPTION ... SERVER.
Allow CREATE SUBSCRIPTION to accept a foreign server using the SERVER
clause instead of a raw connection string using the CONNECTION clause.
* Enables a user with sufficient privileges to create a subscription
using a foreign server by name without specifying the connection
details.
* Integrates with user mappings (and other FDW infrastructure) using
the subscription owner.
* Provides a layer of indirection to manage multiple subscriptions
to the same remote server more easily.
Also add CREATE FOREIGN DATA WRAPPER ... CONNECTION clause to specify
a connection_function. To be eligible for a subscription, the foreign
server's foreign data wrapper must specify a connection_function.
Add connection_function support to postgres_fdw, and bump postgres_fdw
version to 1.3.
Bump catversion.
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Shlok Kyal <shlok.kyal.oss@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/61831790a0a937038f78ce09f8dd4cef7de7456a.camel@j-davis.com
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/8185bb53476378443240d57f7d844347d5fae1bf
Modified Files
--------------
contrib/postgres_fdw/Makefile | 2 +-
contrib/postgres_fdw/connection.c | 299 +++++++++++++---------
contrib/postgres_fdw/expected/postgres_fdw.out | 8 +
contrib/postgres_fdw/meson.build | 2 +
contrib/postgres_fdw/postgres_fdw--1.2--1.3.sql | 12 +
contrib/postgres_fdw/postgres_fdw.control | 2 +-
contrib/postgres_fdw/sql/postgres_fdw.sql | 7 +
contrib/postgres_fdw/t/010_subscription.pl | 71 +++++
doc/src/sgml/logical-replication.sgml | 4 +-
doc/src/sgml/postgres-fdw.sgml | 26 ++
doc/src/sgml/ref/alter_foreign_data_wrapper.sgml | 20 ++
doc/src/sgml/ref/alter_subscription.sgml | 18 +-
doc/src/sgml/ref/create_foreign_data_wrapper.sgml | 20 ++
doc/src/sgml/ref/create_server.sgml | 7 +
doc/src/sgml/ref/create_subscription.sgml | 16 +-
src/backend/catalog/dependency.c | 11 +
src/backend/catalog/pg_subscription.c | 38 ++-
src/backend/catalog/system_views.sql | 2 +-
src/backend/commands/foreigncmds.c | 58 ++++-
src/backend/commands/subscriptioncmds.c | 203 +++++++++++++--
src/backend/foreign/foreign.c | 86 +++++++
src/backend/parser/gram.y | 22 ++
src/backend/replication/logical/worker.c | 24 +-
src/bin/pg_dump/pg_dump.c | 39 ++-
src/bin/pg_dump/pg_dump.h | 1 +
src/bin/psql/describe.c | 6 +-
src/bin/psql/tab-complete.in.c | 11 +-
src/include/catalog/catversion.h | 2 +-
src/include/catalog/pg_foreign_data_wrapper.h | 3 +
src/include/catalog/pg_subscription.h | 8 +-
src/include/foreign/foreign.h | 3 +
src/include/nodes/parsenodes.h | 3 +
src/test/regress/expected/oidjoins.out | 2 +
src/test/regress/expected/subscription.out | 222 +++++++++-------
src/test/regress/regress.c | 7 +
src/test/regress/sql/subscription.sql | 55 ++++
36 files changed, 1070 insertions(+), 250 deletions(-)
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: pgsql: CREATE SUBSCRIPTION ... SERVER.
@ 2026-03-11 07:37 Peter Eisentraut <peter@eisentraut.org>
parent: Jeff Davis <jdavis@postgresql.org>
0 siblings, 1 reply; 9+ messages in thread
From: Peter Eisentraut @ 2026-03-11 07:37 UTC (permalink / raw)
To: Jeff Davis <jdavis@postgresql.org>; pgsql-committers@lists.postgresql.org
On 06.03.26 17:44, Jeff Davis wrote:
> CREATE SUBSCRIPTION ... SERVER.
In src/backend/foreign/foreign.c, this
volatile text *connection_text = NULL;
should probably be
text *volatile connection_text = NULL;
similar to commit 6307b096e25.
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: pgsql: CREATE SUBSCRIPTION ... SERVER.
@ 2026-03-11 21:49 Andres Freund <andres@anarazel.de>
parent: Peter Eisentraut <peter@eisentraut.org>
0 siblings, 1 reply; 9+ messages in thread
From: Andres Freund @ 2026-03-11 21:49 UTC (permalink / raw)
To: Peter Eisentraut <peter@eisentraut.org>; +Cc: Jeff Davis <jdavis@postgresql.org>; pgsql-committers@lists.postgresql.org
Hi,
On 2026-03-11 08:37:13 +0100, Peter Eisentraut wrote:
> On 06.03.26 17:44, Jeff Davis wrote:
> > CREATE SUBSCRIPTION ... SERVER.
>
> In src/backend/foreign/foreign.c, this
>
> volatile text *connection_text = NULL;
>
> should probably be
>
> text *volatile connection_text = NULL;
>
> similar to commit 6307b096e25.
Seems like the issue is a bit bigger to me. Isn't the whole way the function
uses PG_TRY / PG_FINALLY just plain odd?
The only reason connection_text needs to be volatile is because it's modified
in PG_TRY and then accessed in PG_FINALLY. But what's the point of the
latter? If an error was thrown, why would we want to construct the 'result'
string, as the error isn't caught, there's no PG_CATCH. So now the function
builds the result string in case of an error, just to throw it away.
Am I missing something?
I'm also rather unconvinced by ForeignServerConnectionString() creating a
temporary memory context. When would you ever use the function in a long lived
memory context?
Greetings,
Andres
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: pgsql: CREATE SUBSCRIPTION ... SERVER.
@ 2026-03-12 00:59 Jeff Davis <pgsql@j-davis.com>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 2 replies; 9+ messages in thread
From: Jeff Davis @ 2026-03-12 00:59 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; Peter Eisentraut <peter@eisentraut.org>; +Cc: Jeff Davis <jdavis@postgresql.org>; pgsql-committers@lists.postgresql.org
On Wed, 2026-03-11 at 17:49 -0400, Andres Freund wrote:
> The only reason connection_text needs to be volatile is because it's
> modified
> in PG_TRY and then accessed in PG_FINALLY. But what's the point of
> the
> latter? If an error was thrown, why would we want to construct the
> 'result'
> string, as the error isn't caught, there's no PG_CATCH. So now the
> function
> builds the result string in case of an error, just to throw it away.
I believe it would be correct if the variable was not volatile at all,
because it's only set in the last statement of PG_TRY(), so the
PG_FINALLY() will never need to read the modified variable on the error
path. I added the volatile to follow the expected pattern, in case the
code is adjusted later.
That leaves some awkwardness, but I don't immediately see a cleaner way
to do it. Suggestions welcome.
> I'm also rather unconvinced by ForeignServerConnectionString()
> creating a
> temporary memory context. When would you ever use the function in a
> long lived
> memory context?
It's called by GetSubscription(), and the context is ApplyContext.
Regards,
Jeff Davis
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: pgsql: CREATE SUBSCRIPTION ... SERVER.
@ 2026-03-13 01:15 Jeff Davis <pgsql@j-davis.com>
parent: Jeff Davis <pgsql@j-davis.com>
1 sibling, 0 replies; 9+ messages in thread
From: Jeff Davis @ 2026-03-13 01:15 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; Peter Eisentraut <peter@eisentraut.org>; +Cc: Jeff Davis <jdavis@postgresql.org>; pgsql-committers@lists.postgresql.org
On Wed, 2026-03-11 at 17:59 -0700, Jeff Davis wrote:
> That leaves some awkwardness, but I don't immediately see a cleaner
> way
> to do it. Suggestions welcome.
I did see a way to avoid the volatile entirely, it just needs a
redundant MemoryContextSwtichTo(). I think this is cleaner overall,
attached.
Regards,
Jeff Davis
Attachments:
v21-0001-Clean-up-PG_TRY-PG_FINALLY.patch (text/x-patch, 1.7K) download
From ae3beb8493888e1ff8c23813bb56e4abc4e8b723 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Thu, 12 Mar 2026 18:04:35 -0700
Subject: [PATCH v21] Clean up PG_TRY()/PG_FINALLY().
In ForeignServerConnectionString(), refactor the PG_TRY()/PG_FINALLY()
blocks to avoid the need for volatile at all. The only downside is a
redundant MemoryContextSwitchTo() in the success case.
Reported-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/xvdjrdqnpap3uq7owbaox3r7p5gf7sv62aaqf2ju3vb6yglatr@kvvwhoudrlxq
---
src/backend/foreign/foreign.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index 160cf6f51c9..cf10a8c00f9 100644
--- a/src/backend/foreign/foreign.c
+++ b/src/backend/foreign/foreign.c
@@ -225,7 +225,6 @@ ForeignServerConnectionString(Oid userid, Oid serverid)
{
MemoryContext tempContext;
MemoryContext oldcxt;
- text *volatile connection_text = NULL;
char *result = NULL;
/*
@@ -243,6 +242,7 @@ ForeignServerConnectionString(Oid userid, Oid serverid)
{
ForeignServer *server;
ForeignDataWrapper *fdw;
+ text *connection_text = NULL;
Datum connection_datum;
server = GetForeignServer(serverid);
@@ -262,14 +262,15 @@ ForeignServerConnectionString(Oid userid, Oid serverid)
PointerGetDatum(NULL));
connection_text = DatumGetTextPP(connection_datum);
+
+ MemoryContextSwitchTo(oldcxt);
+ result = text_to_cstring((text *) connection_text);
}
PG_FINALLY();
{
+ /* no-op on success path */
MemoryContextSwitchTo(oldcxt);
- if (connection_text)
- result = text_to_cstring((text *) connection_text);
-
MemoryContextDelete(tempContext);
}
PG_END_TRY();
--
2.43.0
* Re: pgsql: CREATE SUBSCRIPTION ... SERVER.
@ 2026-03-13 13:54 Andres Freund <andres@anarazel.de>
parent: Jeff Davis <pgsql@j-davis.com>
1 sibling, 1 reply; 9+ messages in thread
From: Andres Freund @ 2026-03-13 13:54 UTC (permalink / raw)
To: Jeff Davis <pgsql@j-davis.com>; +Cc: Peter Eisentraut <peter@eisentraut.org>; Jeff Davis <jdavis@postgresql.org>; pgsql-committers@lists.postgresql.org
Hi,
On 2026-03-11 17:59:40 -0700, Jeff Davis wrote:
> On Wed, 2026-03-11 at 17:49 -0400, Andres Freund wrote:
> > The only reason connection_text needs to be volatile is because it's
> > modified
> > in PG_TRY and then accessed in PG_FINALLY. But what's the point of
> > the
> > latter? If an error was thrown, why would we want to construct the
> > 'result'
> > string, as the error isn't caught, there's no PG_CATCH. So now the
> > function
> > builds the result string in case of an error, just to throw it away.
>
> I believe it would be correct if the variable was not volatile at all,
> because it's only set in the last statement of PG_TRY(), so the
> PG_FINALLY() will never need to read the modified variable on the error
> path. I added the volatile to follow the expected pattern, in case the
> code is adjusted later.
>
> That leaves some awkwardness, but I don't immediately see a cleaner way
> to do it. Suggestions welcome.
>
> > I'm also rather unconvinced by ForeignServerConnectionString()
> > creating a
> > temporary memory context. When would you ever use the function in a
> > long lived
> > memory context?
>
> It's called by GetSubscription(), and the context is ApplyContext.
GetSubscription() does a dozen allocations or so, so I'm not sure it matters
that ForeignServerConnectionString() would do another few. There is
FreeSubscription(), but it only seems to free a subset of the allocations, and
none of the temporary allocations that are surely made below
GetSubscription().
But regardless of that, even if you want to make
ForeignServerConnectionString() not leak, I don't think that means you need to
use PG_TRY/FINALLY. If there's an error, afaict the callers will just throw
away the apply context, no? Otherwise there'd likely be way more problems. So
just create the context without a PG_TRY and delete it in the success case.
In the failure case it'll be cleaned up by error handling.
And if you do need the PG_TRY for some reason, why not do the
text_to_cstring() call inside the PG_TRY, since it never can have a value if
an error was thrown?
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: pgsql: CREATE SUBSCRIPTION ... SERVER.
@ 2026-03-13 20:37 Jeff Davis <pgsql@j-davis.com>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 9+ messages in thread
From: Jeff Davis @ 2026-03-13 20:37 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Peter Eisentraut <peter@eisentraut.org>; Jeff Davis <jdavis@postgresql.org>; pgsql-committers@lists.postgresql.org
On Fri, 2026-03-13 at 09:54 -0400, Andres Freund wrote:
> GetSubscription() does a dozen allocations or so, so I'm not sure it
> matters
> that ForeignServerConnectionString() would do another few. There is
> FreeSubscription(), but it only seems to free a subset of the
> allocations, and
> none of the temporary allocations that are surely made below
> GetSubscription().
That by itself seems like a problem. If FreeSubscription() is needed,
then it should do its job; and if it's not needed, then it should be
eliminated.
To me it looks like it's needed. Otherwise there will be leaks for
every invalidation.
> But regardless of that, even if you want to make
> ForeignServerConnectionString() not leak, I don't think that means
> you need to
> use PG_TRY/FINALLY. If there's an error, afaict the callers will just
> throw
> away the apply context, no? Otherwise there'd likely be way more
> problems.
I believe that's correct. While there's no delete or reset, it looks
like an error will cause the worker to exit/restart.
> So
> just create the context without a PG_TRY and delete it in the success
> case.
> In the failure case it'll be cleaned up by error handling.
Thank you, patch attached.
> And if you do need the PG_TRY for some reason, why not do the
> text_to_cstring() call inside the PG_TRY, since it never can have a
> value if
> an error was thrown?
That's what I did in the patch I posted here:
https://www.postgresql.org/message-id/f48610c90e69de4b30841361c568c3765e8f3dfe.camel%40j-davis.com
but I think you are right that we don't need the try/catch at all.
Regards,
Jeff Davis
Attachments:
v22-0001-Eliminate-PG_TRY-PG_CATCH-in-ForeignServerConnec.patch (text/x-patch, 3.0K) download
From bf09932dbc0c3ab73bfd5566f5f69a32d709278a Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Fri, 13 Mar 2026 13:24:18 -0700
Subject: [PATCH v22] Eliminate PG_TRY()/PG_CATCH() in
ForeignServerConnectionString().
The error path is cleaned up for all callers, so only worry about the
success path. In the case of a logical worker, it will just exit.
Suggested-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/xvdjrdqnpap3uq7owbaox3r7p5gf7sv62aaqf2ju3vb6yglatr%40kvvwhoudrlxq
---
src/backend/foreign/foreign.c | 53 ++++++++++++++---------------------
1 file changed, 21 insertions(+), 32 deletions(-)
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index cf10a8c00f9..cd2d85df811 100644
--- a/src/backend/foreign/foreign.c
+++ b/src/backend/foreign/foreign.c
@@ -225,7 +225,11 @@ ForeignServerConnectionString(Oid userid, Oid serverid)
{
MemoryContext tempContext;
MemoryContext oldcxt;
- char *result = NULL;
+ ForeignServer *server;
+ ForeignDataWrapper *fdw;
+ text *connection_text;
+ Datum connection_datum;
+ char *result;
/*
* GetForeignServer, GetForeignDataWrapper, and the connection function
@@ -238,42 +242,27 @@ ForeignServerConnectionString(Oid userid, Oid serverid)
oldcxt = MemoryContextSwitchTo(tempContext);
- PG_TRY();
- {
- ForeignServer *server;
- ForeignDataWrapper *fdw;
- text *connection_text = NULL;
- Datum connection_datum;
-
- server = GetForeignServer(serverid);
- fdw = GetForeignDataWrapper(server->fdwid);
-
- if (!OidIsValid(fdw->fdwconnection))
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("foreign data wrapper \"%s\" does not support subscription connections",
- fdw->fdwname),
- errdetail("Foreign data wrapper must be defined with CONNECTION specified.")));
+ server = GetForeignServer(serverid);
+ fdw = GetForeignDataWrapper(server->fdwid);
+ if (!OidIsValid(fdw->fdwconnection))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("foreign data wrapper \"%s\" does not support subscription connections",
+ fdw->fdwname),
+ errdetail("Foreign data wrapper must be defined with CONNECTION specified.")));
- connection_datum = OidFunctionCall3(fdw->fdwconnection,
- ObjectIdGetDatum(userid),
- ObjectIdGetDatum(serverid),
- PointerGetDatum(NULL));
+ connection_datum = OidFunctionCall3(fdw->fdwconnection,
+ ObjectIdGetDatum(userid),
+ ObjectIdGetDatum(serverid),
+ PointerGetDatum(NULL));
- connection_text = DatumGetTextPP(connection_datum);
+ connection_text = DatumGetTextPP(connection_datum);
- MemoryContextSwitchTo(oldcxt);
- result = text_to_cstring((text *) connection_text);
- }
- PG_FINALLY();
- {
- /* no-op on success path */
- MemoryContextSwitchTo(oldcxt);
+ MemoryContextSwitchTo(oldcxt);
+ result = text_to_cstring(connection_text);
- MemoryContextDelete(tempContext);
- }
- PG_END_TRY();
+ MemoryContextDelete(tempContext);
return result;
}
--
2.43.0
* Re: pgsql: CREATE SUBSCRIPTION ... SERVER.
@ 2026-03-13 20:48 Andres Freund <andres@anarazel.de>
parent: Jeff Davis <pgsql@j-davis.com>
0 siblings, 1 reply; 9+ messages in thread
From: Andres Freund @ 2026-03-13 20:48 UTC (permalink / raw)
To: Jeff Davis <pgsql@j-davis.com>; +Cc: Peter Eisentraut <peter@eisentraut.org>; Jeff Davis <jdavis@postgresql.org>; pgsql-committers@lists.postgresql.org
Hi,
On 2026-03-13 13:37:29 -0700, Jeff Davis wrote:
> On Fri, 2026-03-13 at 09:54 -0400, Andres Freund wrote:
> > GetSubscription() does a dozen allocations or so, so I'm not sure it
> > matters
> > that ForeignServerConnectionString() would do another few. There is
> > FreeSubscription(), but it only seems to free a subset of the
> > allocations, and
> > none of the temporary allocations that are surely made below
> > GetSubscription().
>
> That by itself seems like a problem. If FreeSubscription() is needed,
> then it should do its job; and if it's not needed, then it should be
> eliminated.
No argument there.
> To me it looks like it's needed. Otherwise there will be leaks for
> every invalidation.
It does indeed look like it'd leak.
I suspect the more scalable approach would be to create a dedicated memory
context for each GetSubscription() call that's then torn down during
invalidation.
> > So
> > just create the context without a PG_TRY and delete it in the success
> > case.
> > In the failure case it'll be cleaned up by error handling.
>
> Thank you, patch attached.
LGTM on a quick scan.
> > And if you do need the PG_TRY for some reason, why not do the
> > text_to_cstring() call inside the PG_TRY, since it never can have a
> > value if
> > an error was thrown?
>
> That's what I did in the patch I posted here:
>
> https://www.postgresql.org/message-id/f48610c90e69de4b30841361c568c3765e8f3dfe.camel%40j-davis.com
>
> but I think you are right that we don't need the try/catch at all.
I was behind on my email, so I hadn't yet seen that :)
Greetings,
Andres
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: pgsql: CREATE SUBSCRIPTION ... SERVER.
@ 2026-03-13 22:19 Jeff Davis <pgsql@j-davis.com>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 9+ messages in thread
From: Jeff Davis @ 2026-03-13 22:19 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Peter Eisentraut <peter@eisentraut.org>; Jeff Davis <jdavis@postgresql.org>; pgsql-committers@lists.postgresql.org
On Fri, 2026-03-13 at 16:48 -0400, Andres Freund wrote:
> I suspect the more scalable approach would be to create a dedicated
> memory
> context for each GetSubscription() call that's then torn down during
> invalidation.
Attached. This is posted as an alternative to the previous patch.
Regards,
Jeff Davis
Attachments:
v22-0001-Temp-context-for-maybe_reread_subscription.patch (text/x-patch, 6.9K) download
From 98705f06a8dfb757c5db69f3b4f9ea2d73f21deb Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Fri, 13 Mar 2026 13:24:18 -0700
Subject: [PATCH v22] Temp context for maybe_reread_subscription().
Move temp context from ForeignServerConnectionString() to
maybe_reread_subscription(), so that it prevents more
invalidation-related leaks.
Suggested-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/xvdjrdqnpap3uq7owbaox3r7p5gf7sv62aaqf2ju3vb6yglatr%40kvvwhoudrlxq
---
src/backend/catalog/pg_subscription.c | 14 -----
src/backend/foreign/foreign.c | 67 +++++++-----------------
src/backend/replication/logical/worker.c | 30 +++++++++--
src/include/catalog/pg_subscription.h | 1 -
4 files changed, 43 insertions(+), 69 deletions(-)
diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index 3673d4f0bc1..ca053c152cf 100644
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -216,20 +216,6 @@ CountDBSubscriptions(Oid dbid)
return nsubs;
}
-/*
- * Free memory allocated by subscription struct.
- */
-void
-FreeSubscription(Subscription *sub)
-{
- pfree(sub->name);
- pfree(sub->conninfo);
- if (sub->slotname)
- pfree(sub->slotname);
- list_free_deep(sub->publications);
- pfree(sub);
-}
-
/*
* Disable the given subscription.
*/
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index cf10a8c00f9..f437b447282 100644
--- a/src/backend/foreign/foreign.c
+++ b/src/backend/foreign/foreign.c
@@ -219,63 +219,32 @@ GetForeignServerByName(const char *srvname, bool missing_ok)
/*
* Retrieve connection string from server's FDW.
+ *
+ * NB: leaks into CurrentMemoryContext.
*/
char *
ForeignServerConnectionString(Oid userid, Oid serverid)
{
- MemoryContext tempContext;
- MemoryContext oldcxt;
- char *result = NULL;
-
- /*
- * GetForeignServer, GetForeignDataWrapper, and the connection function
- * itself all leak memory into CurrentMemoryContext. Switch to a temporary
- * context for easy cleanup.
- */
- tempContext = AllocSetContextCreate(CurrentMemoryContext,
- "FDWConnectionContext",
- ALLOCSET_SMALL_SIZES);
-
- oldcxt = MemoryContextSwitchTo(tempContext);
-
- PG_TRY();
- {
- ForeignServer *server;
- ForeignDataWrapper *fdw;
- text *connection_text = NULL;
- Datum connection_datum;
-
- server = GetForeignServer(serverid);
- fdw = GetForeignDataWrapper(server->fdwid);
-
- if (!OidIsValid(fdw->fdwconnection))
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("foreign data wrapper \"%s\" does not support subscription connections",
- fdw->fdwname),
- errdetail("Foreign data wrapper must be defined with CONNECTION specified.")));
-
-
- connection_datum = OidFunctionCall3(fdw->fdwconnection,
- ObjectIdGetDatum(userid),
- ObjectIdGetDatum(serverid),
- PointerGetDatum(NULL));
+ ForeignServer *server;
+ ForeignDataWrapper *fdw;
+ Datum connection_datum;
- connection_text = DatumGetTextPP(connection_datum);
+ server = GetForeignServer(serverid);
+ fdw = GetForeignDataWrapper(server->fdwid);
- MemoryContextSwitchTo(oldcxt);
- result = text_to_cstring((text *) connection_text);
- }
- PG_FINALLY();
- {
- /* no-op on success path */
- MemoryContextSwitchTo(oldcxt);
+ if (!OidIsValid(fdw->fdwconnection))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("foreign data wrapper \"%s\" does not support subscription connections",
+ fdw->fdwname),
+ errdetail("Foreign data wrapper must be defined with CONNECTION specified.")));
- MemoryContextDelete(tempContext);
- }
- PG_END_TRY();
+ connection_datum = OidFunctionCall3(fdw->fdwconnection,
+ ObjectIdGetDatum(userid),
+ ObjectIdGetDatum(serverid),
+ PointerGetDatum(NULL));
- return result;
+ return text_to_cstring(DatumGetTextPP(connection_datum));
}
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 033858752d9..e00da48ae39 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -479,6 +479,7 @@ static MemoryContext LogicalStreamingContext = NULL;
WalReceiverConn *LogRepWorkerWalRcvConn = NULL;
Subscription *MySubscription = NULL;
+MemoryContext MySubscriptionCtx = NULL;
static bool MySubscriptionValid = false;
static List *on_commit_wakeup_workers_subids = NIL;
@@ -5042,6 +5043,7 @@ void
maybe_reread_subscription(void)
{
MemoryContext oldctx;
+ MemoryContext newctx;
Subscription *newsub;
bool started_tx = false;
@@ -5056,8 +5058,15 @@ maybe_reread_subscription(void)
started_tx = true;
}
- /* Ensure allocations in permanent context. */
- oldctx = MemoryContextSwitchTo(ApplyContext);
+ newctx = AllocSetContextCreate(ApplyContext,
+ "Subscription Context",
+ ALLOCSET_SMALL_SIZES);
+
+ /*
+ * GetSubscription() leaks a number of small allocations, so use a
+ * subcontext for each call.
+ */
+ oldctx = MemoryContextSwitchTo(newctx);
newsub = GetSubscription(MyLogicalRepWorker->subid, true, true);
@@ -5149,7 +5158,8 @@ maybe_reread_subscription(void)
}
/* Clean old subscription info and switch to new one. */
- FreeSubscription(MySubscription);
+ MemoryContextDelete(MySubscriptionCtx);
+ MySubscriptionCtx = newctx;
MySubscription = newsub;
MemoryContextSwitchTo(oldctx);
@@ -5794,12 +5804,19 @@ InitializeLogRepWorker(void)
*/
SetConfigOption("search_path", "", PGC_SUSET, PGC_S_OVERRIDE);
- /* Load the subscription into persistent memory context. */
ApplyContext = AllocSetContextCreate(TopMemoryContext,
"ApplyContext",
ALLOCSET_DEFAULT_SIZES);
+
+ /*
+ * GetSubscription() leaks a number of small allocations, so use a
+ * subcontext for each call.
+ */
+ MySubscriptionCtx = AllocSetContextCreate(ApplyContext,
+ "Subscription Context",
+ ALLOCSET_SMALL_SIZES);
+
StartTransactionCommand();
- oldctx = MemoryContextSwitchTo(ApplyContext);
/*
* Lock the subscription to prevent it from being concurrently dropped,
@@ -5808,7 +5825,10 @@ InitializeLogRepWorker(void)
*/
LockSharedObject(SubscriptionRelationId, MyLogicalRepWorker->subid, 0,
AccessShareLock);
+
+ oldctx = MemoryContextSwitchTo(MySubscriptionCtx);
MySubscription = GetSubscription(MyLogicalRepWorker->subid, true, true);
+
if (!MySubscription)
{
ereport(LOG,
diff --git a/src/include/catalog/pg_subscription.h b/src/include/catalog/pg_subscription.h
index 0058d9387d7..2f6f7b57698 100644
--- a/src/include/catalog/pg_subscription.h
+++ b/src/include/catalog/pg_subscription.h
@@ -212,7 +212,6 @@ typedef struct Subscription
extern Subscription *GetSubscription(Oid subid, bool missing_ok,
bool aclcheck);
-extern void FreeSubscription(Subscription *sub);
extern void DisableSubscription(Oid subid);
extern int CountDBSubscriptions(Oid dbid);
--
2.43.0
end of thread, other threads:[~2026-03-13 22:19 UTC | newest]
Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-03-06 16:44 pgsql: CREATE SUBSCRIPTION ... SERVER. Jeff Davis <jdavis@postgresql.org>
2026-03-11 07:37 ` Peter Eisentraut <peter@eisentraut.org>
2026-03-11 21:49 ` Andres Freund <andres@anarazel.de>
2026-03-12 00:59 ` Jeff Davis <pgsql@j-davis.com>
2026-03-13 01:15 ` Jeff Davis <pgsql@j-davis.com>
2026-03-13 13:54 ` Andres Freund <andres@anarazel.de>
2026-03-13 20:37 ` Jeff Davis <pgsql@j-davis.com>
2026-03-13 20:48 ` Andres Freund <andres@anarazel.de>
2026-03-13 22:19 ` Jeff Davis <pgsql@j-davis.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox