On 2024/08/08 11:38, Hayato Kuroda (Fujitsu) wrote:
Dear Fujii-san, Thanks for reviewing! PSA new version.
Thanks for updating the patch! LGTM. I made a couple of small adjustments and attached the updated version. If that's ok, I'll go ahead and commit it. + Name of the local user mapped to the foreign server of this + connection, or "public" if a public mapping is used. If the user I enclosed "public" with <literal> tag, i.e., <literal>public</literal>.
I did not done that be cause either of server_name or user_name is NULL and it might be strange. But yes, the example should have more information. Based on that, I added a tuple so that the example has below. Thought? loopback1 - user is "postgres", valid loopback2 - user is "public", valid loopback3 - user is NULL, invalid
LGTM. Also I added the following to the example for clarity: postgres=# SELECT * FROM postgres_fdw_get_connections(true); Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
From 1b50979b84d2f8718a3af41bd407a76a0d048ea6 Mon Sep 17 00:00:00 2001 From: Hayato Kuroda <kuroda.hay...@fujitsu.com> Date: Thu, 18 Jul 2024 10:11:03 +0000 Subject: [PATCH v4] postgres_fdw: Extend postgres_fdw_get_connections to return user name. This commit adds a "user_name" output column to the postgres_fdw_get_connections function, returning the name of the local user mapped to the foreign server for each connection. If a public mapping is used, it returns "public." This helps identify postgres_fdw connections more easily, such as determining which connections are invalid, closed, or used within the current transaction. No extension version bump is needed, as commit c297a47c5f already handled it for v18~. Author: Hayato Kuroda Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/b492a935-6c7e-8c08-e485-3c1d64d7d...@oss.nttdata.com --- contrib/postgres_fdw/connection.c | 57 +++++++++++++++---- .../postgres_fdw/expected/postgres_fdw.out | 16 +++--- .../postgres_fdw/postgres_fdw--1.1--1.2.sql | 3 +- contrib/postgres_fdw/sql/postgres_fdw.sql | 8 ++- doc/src/sgml/postgres-fdw.sgml | 23 ++++++-- 5 files changed, 81 insertions(+), 26 deletions(-) diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c index 12d1fec0e8..2e5303eac1 100644 --- a/contrib/postgres_fdw/connection.c +++ b/contrib/postgres_fdw/connection.c @@ -1997,8 +1997,8 @@ pgfdw_finish_abort_cleanup(List *pending_entries, List *cancel_requested, /* Number of output arguments (columns) for various API versions */ #define POSTGRES_FDW_GET_CONNECTIONS_COLS_V1_1 2 -#define POSTGRES_FDW_GET_CONNECTIONS_COLS_V1_2 4 -#define POSTGRES_FDW_GET_CONNECTIONS_COLS 4 /* maximum of above */ +#define POSTGRES_FDW_GET_CONNECTIONS_COLS_V1_2 5 +#define POSTGRES_FDW_GET_CONNECTIONS_COLS 5 /* maximum of above */ /* * Internal function used by postgres_fdw_get_connections variants. @@ -2014,10 +2014,13 @@ pgfdw_finish_abort_cleanup(List *pending_entries, List *cancel_requested, * * For API version 1.2 and later, this function takes an input parameter * to check a connection status and returns the following - * additional values along with the two values from version 1.1: + * additional values along with the three values from version 1.1: * + * - user_name - the local user name of the active connection. In case the + * user mapping is dropped but the connection is still active, then the + * user name will be NULL in the output. * - used_in_xact - true if the connection is used in the current transaction. - * - closed: true if the connection is closed. + * - closed - true if the connection is closed. * * No records are returned when there are no cached connections at all. */ @@ -2056,6 +2059,7 @@ postgres_fdw_get_connections_internal(FunctionCallInfo fcinfo, ForeignServer *server; Datum values[POSTGRES_FDW_GET_CONNECTIONS_COLS] = {0}; bool nulls[POSTGRES_FDW_GET_CONNECTIONS_COLS] = {0}; + int i = 0; /* We only look for open remote connections */ if (!entry->conn) @@ -2100,28 +2104,61 @@ postgres_fdw_get_connections_internal(FunctionCallInfo fcinfo, Assert(entry->conn && entry->xact_depth > 0 && entry->invalidated); /* Show null, if no server name was found */ - nulls[0] = true; + nulls[i++] = true; } else - values[0] = CStringGetTextDatum(server->servername); + values[i++] = CStringGetTextDatum(server->servername); - values[1] = BoolGetDatum(!entry->invalidated); + if (api_version >= PGFDW_V1_2) + { + HeapTuple tp; + + /* Use the system cache to obtain the user mapping */ + tp = SearchSysCache1(USERMAPPINGOID, ObjectIdGetDatum(entry->key)); + + /* + * Just like in the foreign server case, user mappings can also be + * dropped in the current explicit transaction. Therefore, the + * similar check as in the server case is required. + */ + if (!HeapTupleIsValid(tp)) + { + /* + * If we reach here, this entry must have been invalidated in + * pgfdw_inval_callback, same as in the server case. + */ + Assert(entry->conn && entry->xact_depth > 0 && + entry->invalidated); + + nulls[i++] = true; + } + else + { + Oid userid; + + userid = ((Form_pg_user_mapping) GETSTRUCT(tp))->umuser; + values[i++] = CStringGetTextDatum(MappingUserName(userid)); + ReleaseSysCache(tp); + } + } + + values[i++] = BoolGetDatum(!entry->invalidated); if (api_version >= PGFDW_V1_2) { bool check_conn = PG_GETARG_BOOL(0); /* Is this connection used in the current transaction? */ - values[2] = BoolGetDatum(entry->xact_depth > 0); + values[i++] = BoolGetDatum(entry->xact_depth > 0); /* * If a connection status check is requested and supported, return * whether the connection is closed. Otherwise, return NULL. */ if (check_conn && pgfdw_conn_checkable()) - values[3] = BoolGetDatum(pgfdw_conn_check(entry->conn) != 0); + values[i++] = BoolGetDatum(pgfdw_conn_check(entry->conn) != 0); else - nulls[3] = true; + nulls[i++] = true; } tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, values, nulls); diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index f3eb055e2c..f2bcd6aa98 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -10472,13 +10472,15 @@ NOTICE: drop cascades to 2 other objects DETAIL: drop cascades to user mapping for public on server loopback3 drop cascades to foreign table ft7 -- List all the existing cached connections. loopback and loopback3 --- should be output as invalid connections. Also the server name for --- loopback3 should be NULL because the server was dropped. -SELECT * FROM postgres_fdw_get_connections() ORDER BY 1; - server_name | valid | used_in_xact | closed --------------+-------+--------------+-------- - loopback | f | t | - | f | t | +-- should be output as invalid connections. Also the server name and user name +-- for loopback3 should be NULL because both server and user mapping were +-- dropped. +SELECT server_name, user_name = CURRENT_USER as "user_name = CURRENT_USER", valid, used_in_xact, closed +FROM postgres_fdw_get_connections() ORDER BY 1; + server_name | user_name = CURRENT_USER | valid | used_in_xact | closed +-------------+--------------------------+-------+--------------+-------- + loopback | t | f | t | + | | f | t | (2 rows) -- The invalid connections get closed in pgfdw_xact_callback during commit. diff --git a/contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql b/contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql index 0d406c6028..81aad4fcda 100644 --- a/contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql +++ b/contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql @@ -11,7 +11,8 @@ DROP FUNCTION postgres_fdw_get_connections (); CREATE FUNCTION postgres_fdw_get_connections ( IN check_conn boolean DEFAULT false, OUT server_name text, - OUT valid boolean, OUT used_in_xact boolean, OUT closed boolean) + OUT user_name text, OUT valid boolean, OUT used_in_xact boolean, + OUT closed boolean) RETURNS SETOF record AS 'MODULE_PATHNAME', 'postgres_fdw_get_connections_1_2' LANGUAGE C STRICT PARALLEL RESTRICTED; diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 0734716ad9..372fe6dad1 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -3382,9 +3382,11 @@ SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1; ALTER SERVER loopback OPTIONS (ADD use_remote_estimate 'off'); DROP SERVER loopback3 CASCADE; -- List all the existing cached connections. loopback and loopback3 --- should be output as invalid connections. Also the server name for --- loopback3 should be NULL because the server was dropped. -SELECT * FROM postgres_fdw_get_connections() ORDER BY 1; +-- should be output as invalid connections. Also the server name and user name +-- for loopback3 should be NULL because both server and user mapping were +-- dropped. +SELECT server_name, user_name = CURRENT_USER as "user_name = CURRENT_USER", valid, used_in_xact, closed +FROM postgres_fdw_get_connections() ORDER BY 1; -- The invalid connections get closed in pgfdw_xact_callback during commit. COMMIT; -- All cached connections were closed while committing above xact, so no diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml index 468724e94e..627bb5ab5c 100644 --- a/doc/src/sgml/postgres-fdw.sgml +++ b/doc/src/sgml/postgres-fdw.sgml @@ -779,7 +779,8 @@ OPTIONS (ADD password_required 'false'); <varlistentry> <term><function>postgres_fdw_get_connections( IN check_conn boolean DEFAULT false, OUT server_name text, - OUT valid boolean, OUT used_in_xact boolean, OUT closed boolean) + OUT user_name text, OUT valid boolean, OUT used_in_xact boolean, + OUT closed boolean) returns setof record</function></term> <listitem> <para> @@ -806,10 +807,12 @@ OPTIONS (ADD password_required 'false'); <para> Example usage of the function: <screen> - server_name | valid | used_in_xact | closed --------------+-------+--------------+-------- - loopback1 | t | t | - loopback2 | f | t | +postgres=# SELECT * FROM postgres_fdw_get_connections(true); + server_name | user_name | valid | used_in_xact | closed +-------------+-----------+-------+--------------+-------- + loopback1 | postgres | t | t | f + loopback2 | public | t | t | f + loopback3 | | f | t | f </screen> The output columns are described in <xref linkend="postgres-fdw-get-connections-columns"/>. @@ -836,6 +839,16 @@ OPTIONS (ADD password_required 'false'); invalid), this will be <literal>NULL</literal>. </entry> </row> + <row> + <entry><structfield>user_name</structfield></entry> + <entry><type>text</type></entry> + <entry> + Name of the local user mapped to the foreign server of this + connection, or <literal>public</literal> if a public mapping is used. + If the user mapping is dropped but the connection remains open + (i.e., marked as invalid), this will be <literal>NULL</literal>. + </entry> + </row> <row> <entry><structfield>valid</structfield></entry> <entry><type>boolean</type></entry> -- 2.45.2
