pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema
5+ messages / 4 participants
[nested] [flat]
* BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema
@ 2025-05-14 12:37 PG Bug reporting form <noreply@postgresql.org>
2025-05-14 15:06 ` Re: BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema Matheus Alcantara <matheusssilv97@gmail.com>
0 siblings, 1 reply; 5+ messages in thread
From: PG Bug reporting form @ 2025-05-14 12:37 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: jackie.space@gmail.com
The following bug has been logged on the website:
Bug reference: 18928
Logged by: Jackie Li
Email address: jackie.space@gmail.com
PostgreSQL version: 17.5
Operating system: Linux, MacOS
Description:
I filed an issue to pgbouncer, but I realised the issue is more related to
postgres_fdw extension: https://github.com/pgbouncer/pgbouncer/issues/1313
To reproduce this issue, you can use a minimal code sample at
https://github.com/jackielii/pgbouncer-fdw-issue/tree/main
Steps to reproduce:
1. set up 2 databases: db1 and db2
2. setup pgbouncer that connects to both in transaction pooling mode. See
[pgbouncer.ini](https://github.com/jackielii/pgbouncer-fdw-issue/blob/main/pgbouncer.ini)
3. connect to db2 via pgbouncer and creates foreign schema connects to db1
using postgres_fdw
4. Now open a connection to db1 via pgbouncer and run select query without
schema
5. observe an error occurs because wrong default schema pg_catalog is used:
relation "test1" does not exist
The issue seems to be related to a connection sets the search_path gets
reused and search_path is not reverted: [related code in
postgres_fdw](https://github.com/postgres/postgres/blob/master/contrib/postgres_fdw/postgres_fdw.c#L3935-L3938)
I know this feels like working as intended as postgres_fdw does close and
reset it at
https://github.com/postgres/postgres/blob/master/contrib/postgres_fdw/postgres_fdw.c#L3946,
but I think it should revert the `set_config_option` effect, especially for
search_path
Kind Regards,
Jackie
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema
2025-05-14 12:37 BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema PG Bug reporting form <noreply@postgresql.org>
@ 2025-05-14 15:06 ` Matheus Alcantara <matheusssilv97@gmail.com>
2025-05-14 15:30 ` Re: BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema Tom Lane <tgl@sss.pgh.pa.us>
2025-05-14 16:30 ` Re: BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema Jackie Li <jackie.space@gmail.com>
0 siblings, 2 replies; 5+ messages in thread
From: Matheus Alcantara @ 2025-05-14 15:06 UTC (permalink / raw)
To: jackie.space@gmail.com; pgsql-bugs@lists.postgresql.org
Hi,
On 14/05/25 09:37, PG Bug reporting form wrote:
> I filed an issue to pgbouncer, but I realised the issue is more related to
> postgres_fdw extension: https://github.com/pgbouncer/pgbouncer/issues/1313
> To reproduce this issue, you can use a minimal code sample at
> https://github.com/jackielii/pgbouncer-fdw-issue/tree/main
> Steps to reproduce:
> 1. set up 2 databases: db1 and db2
> 2. setup pgbouncer that connects to both in transaction pooling mode. See
> [pgbouncer.ini](https://github.com/jackielii/pgbouncer-fdw-issue/blob/main/pgbouncer.ini)
> 3. connect to db2 via pgbouncer and creates foreign schema connects to db1
> using postgres_fdw
> 4. Now open a connection to db1 via pgbouncer and run select query without
> schema
> 5. observe an error occurs because wrong default schema pg_catalog is used:
> relation "test1" does not exist
> The issue seems to be related to a connection sets the search_path gets
> reused and search_path is not reverted: [related code in
> postgres_fdw](https://github.com/postgres/postgres/blob/master/contrib/postgres_fdw/postgres_fdw.c#L3935-L3938)
> I know this feels like working as intended as postgres_fdw does close and
> reset it at
> https://github.com/postgres/postgres/blob/master/contrib/postgres_fdw/postgres_fdw.c#L3946,
> but I think it should revert the `set_config_option` effect, especially for
> search_path
>
I've managed to reproduce the issue.
I don't think that the problem it's on set_transmission_modes() that
you've shared, it seems to me that the issue it's on
configure_remote_session() that is called for every new foreign
connection created by postgres_fdw and it currently sets the seach_path
and other GUCs(timezone, datestyle, and others).
This issue seems a bit tricky to fix because Postgres does not know that
the connection can be reused by PgBouncer. One possible solution for
this is that on postgresEndForeignScan() we could reset all the GUCs set
by the configure_remote_session() and before reusing the connection from
the ConnCacheEntry we configure the remote session using
configure_remote_session(). See attached diff as a proof of concept
(please keep in mind that it's POC and I'm not considering possible
other scenarios and I'm not sure if the reset_remote_session() should be
called at ReleaseConnection()).
One problem with this idea is that it can cause some overhead when
reusing a cached connection and also when ending the foreign scan, so I
think that we could also add a foreign server option to enable this
behaviour. Thoughts?
--
Matheus Alcantara
Attachments:
[application/octet-stream] postgres-fdw-reset-gucs.diff (2.2K, ../../CAFY6G8deU8mh=THZXVCDzan_9D6b8K9YUw4bBPBrYS4z37BZ9Q@mail.gmail.com/2-postgres-fdw-reset-gucs.diff)
download | inline diff:
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 304f3c20f83..c47f9efa378 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -138,6 +138,7 @@ static PGconn *connect_pg_server(ForeignServer *server, UserMapping *user);
static void disconnect_pg_server(ConnCacheEntry *entry);
static void check_conn_params(const char **keywords, const char **values, UserMapping *user);
static void configure_remote_session(PGconn *conn);
+static void reset_remote_session(PGconn *conn);
static void do_sql_command_begin(PGconn *conn, const char *sql);
static void do_sql_command_end(PGconn *conn, const char *sql,
bool consume_input);
@@ -204,6 +205,7 @@ GetConnection(UserMapping *user, bool will_prep_stmt, PgFdwConnState **state)
{
bool found;
bool retry = false;
+ bool new_connection = false;
ConnCacheEntry *entry;
ConnCacheKey key;
MemoryContext ccxt = CurrentMemoryContext;
@@ -274,7 +276,10 @@ GetConnection(UserMapping *user, bool will_prep_stmt, PgFdwConnState **state)
* will remain in a valid empty state, ie conn == NULL.)
*/
if (entry->conn == NULL)
+ {
make_new_connection(entry, user);
+ new_connection = true;
+ }
/*
* We check the health of the cached connection here when using it. In
@@ -283,6 +288,10 @@ GetConnection(UserMapping *user, bool will_prep_stmt, PgFdwConnState **state)
*/
PG_TRY();
{
+ /* Prepare the cached connection for use */
+ if (!new_connection)
+ configure_remote_session(entry->conn);
+
/* Process a pending asynchronous request if any. */
if (entry->state.pendingAreq)
process_pending_request(entry->state.pendingAreq);
@@ -798,6 +807,12 @@ configure_remote_session(PGconn *conn)
do_sql_command(conn, "SET extra_float_digits = 2");
}
+static void
+reset_remote_session(PGconn *conn)
+{
+ do_sql_command(conn, "RESET search_path");
+}
+
/*
* Convenience subroutine to issue a non-data-returning SQL command to remote
*/
@@ -895,6 +910,8 @@ ReleaseConnection(PGconn *conn)
* cleanup is managed on a transaction or subtransaction basis instead. So
* there's nothing to do here.
*/
+
+ reset_remote_session(conn);
}
/*
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema
2025-05-14 12:37 BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema PG Bug reporting form <noreply@postgresql.org>
2025-05-14 15:06 ` Re: BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema Matheus Alcantara <matheusssilv97@gmail.com>
@ 2025-05-14 15:30 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-05-14 16:38 ` Re: BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema Jackie Li <jackie.space@gmail.com>
1 sibling, 1 reply; 5+ messages in thread
From: Tom Lane @ 2025-05-14 15:30 UTC (permalink / raw)
To: Matheus Alcantara <matheusssilv97@gmail.com>; +Cc: jackie.space@gmail.com; pgsql-bugs@lists.postgresql.org
ISTM the problem described here is a failure on PgBouncer's part: it is
failing to maintain the illusion of a separate connection per user.
I don't know that much about PgBouncer, but perhaps you just have
it configured wrong?
regards, tom lane
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema
2025-05-14 12:37 BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema PG Bug reporting form <noreply@postgresql.org>
2025-05-14 15:06 ` Re: BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema Matheus Alcantara <matheusssilv97@gmail.com>
2025-05-14 15:30 ` Re: BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema Tom Lane <tgl@sss.pgh.pa.us>
@ 2025-05-14 16:38 ` Jackie Li <jackie.space@gmail.com>
0 siblings, 0 replies; 5+ messages in thread
From: Jackie Li @ 2025-05-14 16:38 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Matheus Alcantara <matheusssilv97@gmail.com>; pgsql-bugs@lists.postgresql.org
Thanks for the response. You can checkout the pgbouncer's config at my
minimal reproducing example:
https://github.com/jackielii/pgbouncer-fdw-issue/blob/separate-server/pgbouncer.ini
I can't say the config is wrong or not, but our devs require the pool_mode
to be "transaction" for their respective db.
At this point I feel we shouldn't fix this: postgres shouldn't care how the
connection is reused. If anything pgbouncer should make sure the connection
is reset for reuse. And they kind of do: use a separate pool.
In conclusion, my workaround at
https://github.com/jackielii/pgbouncer-fdw-issue/tree/separate-server which
creates another pool for db1 is the right solution.
Thanks & Kind regards
On Wed, 14 May 2025 at 16:30, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> ISTM the problem described here is a failure on PgBouncer's part: it is
> failing to maintain the illusion of a separate connection per user.
> I don't know that much about PgBouncer, but perhaps you just have
> it configured wrong?
>
> regards, tom lane
>
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema
2025-05-14 12:37 BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema PG Bug reporting form <noreply@postgresql.org>
2025-05-14 15:06 ` Re: BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema Matheus Alcantara <matheusssilv97@gmail.com>
@ 2025-05-14 16:30 ` Jackie Li <jackie.space@gmail.com>
1 sibling, 0 replies; 5+ messages in thread
From: Jackie Li @ 2025-05-14 16:30 UTC (permalink / raw)
To: Matheus Alcantara <matheusssilv97@gmail.com>; +Cc: pgsql-bugs@lists.postgresql.org
I see, thanks a lot for the insight.
It sounds to me that my best option is to have a separate connection pool
for the foreign connections. I.e. create a different pool in pgbouncer (
https://github.com/jackielii/pgbouncer-fdw-issue/blob/separate-server/pgbouncer.ini#L4)
that can be used by the create server statement right?
On Wed, 14 May 2025 at 16:07, Matheus Alcantara <matheusssilv97@gmail.com>
wrote:
> Hi,
>
> On 14/05/25 09:37, PG Bug reporting form wrote:
> > I filed an issue to pgbouncer, but I realised the issue is more related
> to
> > postgres_fdw extension:
> https://github.com/pgbouncer/pgbouncer/issues/1313
> > To reproduce this issue, you can use a minimal code sample at
> > https://github.com/jackielii/pgbouncer-fdw-issue/tree/main
> > Steps to reproduce:
> > 1. set up 2 databases: db1 and db2
> > 2. setup pgbouncer that connects to both in transaction pooling mode. See
> > [pgbouncer.ini](
> https://github.com/jackielii/pgbouncer-fdw-issue/blob/main/pgbouncer.ini)
> > 3. connect to db2 via pgbouncer and creates foreign schema connects to
> db1
> > using postgres_fdw
> > 4. Now open a connection to db1 via pgbouncer and run select query
> without
> > schema
> > 5. observe an error occurs because wrong default schema pg_catalog is
> used:
> > relation "test1" does not exist
> > The issue seems to be related to a connection sets the search_path gets
> > reused and search_path is not reverted: [related code in
> > postgres_fdw](
> https://github.com/postgres/postgres/blob/master/contrib/postgres_fdw/postgres_fdw.c#L3935-L3938
> )
> > I know this feels like working as intended as postgres_fdw does close and
> > reset it at
> >
> https://github.com/postgres/postgres/blob/master/contrib/postgres_fdw/postgres_fdw.c#L3946
> ,
> > but I think it should revert the `set_config_option` effect, especially
> for
> > search_path
> >
>
> I've managed to reproduce the issue.
>
> I don't think that the problem it's on set_transmission_modes() that
> you've shared, it seems to me that the issue it's on
> configure_remote_session() that is called for every new foreign
> connection created by postgres_fdw and it currently sets the seach_path
> and other GUCs(timezone, datestyle, and others).
>
> This issue seems a bit tricky to fix because Postgres does not know that
> the connection can be reused by PgBouncer. One possible solution for
> this is that on postgresEndForeignScan() we could reset all the GUCs set
> by the configure_remote_session() and before reusing the connection from
> the ConnCacheEntry we configure the remote session using
> configure_remote_session(). See attached diff as a proof of concept
> (please keep in mind that it's POC and I'm not considering possible
> other scenarios and I'm not sure if the reset_remote_session() should be
> called at ReleaseConnection()).
>
> One problem with this idea is that it can cause some overhead when
> reusing a cached connection and also when ending the foreign scan, so I
> think that we could also add a foreign server option to enable this
> behaviour. Thoughts?
>
> --
> Matheus Alcantara
>
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2025-05-14 16:38 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-05-14 12:37 BUG #18928: postgres_fdw search_path not reset causing pgbouncer pool_mode=transaction using the wrong schema PG Bug reporting form <noreply@postgresql.org>
2025-05-14 15:06 ` Matheus Alcantara <matheusssilv97@gmail.com>
2025-05-14 15:30 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-05-14 16:38 ` Jackie Li <jackie.space@gmail.com>
2025-05-14 16:30 ` Jackie Li <jackie.space@gmail.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