On 2021/09/06 10:32, kuroda.hay...@fujitsu.com wrote:
I think we should SELECT ft6 because foreign server 'loopback' doesn't have application_name server option.
Yes. I forgot to update that... Thanks for the review! Attached is the updated version of the patch. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c index 82aa14a65d..705f60a3ae 100644 --- a/contrib/postgres_fdw/connection.c +++ b/contrib/postgres_fdw/connection.c @@ -353,10 +353,11 @@ connect_pg_server(ForeignServer *server, UserMapping *user) /* * Construct connection params from generic options of ForeignServer * and UserMapping. (Some of them might not be libpq options, in - * which case we'll just waste a few array slots.) Add 3 extra slots - * for fallback_application_name, client_encoding, end marker. + * which case we'll just waste a few array slots.) Add 4 extra slots + * for application_name, fallback_application_name, client_encoding, + * end marker. */ - n = list_length(server->options) + list_length(user->options) + 3; + n = list_length(server->options) + list_length(user->options) + 4; keywords = (const char **) palloc(n * sizeof(char *)); values = (const char **) palloc(n * sizeof(char *)); @@ -366,7 +367,23 @@ connect_pg_server(ForeignServer *server, UserMapping *user) n += ExtractConnectionOptions(user->options, keywords + n, values + n); - /* Use "postgres_fdw" as fallback_application_name. */ + /* + * Use pgfdw_application_name as application_name if set. + * + * PQconnectdbParams() processes the parameter arrays from start to + * end. If any key word is repeated, the last value is used. Therefore + * note that pgfdw_application_name must be added to the arrays after + * options of ForeignServer are, so that it can override + * application_name set in ForeignServer. + */ + if (pgfdw_application_name && *pgfdw_application_name != '\0') + { + keywords[n] = "application_name"; + values[n] = pgfdw_application_name; + n++; + } + + /* Use "postgres_fdw" as fallback_application_name */ keywords[n] = "fallback_application_name"; values[n] = "postgres_fdw"; n++; diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index e3ee30f1aa..39befa394a 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -10761,3 +10761,82 @@ ERROR: invalid value for integer option "fetch_size": 100$%$#$# CREATE FOREIGN TABLE inv_bsz (c1 int ) SERVER loopback OPTIONS (batch_size '100$%$#$#'); ERROR: invalid value for integer option "batch_size": 100$%$#$# +-- =================================================================== +-- test postgres_fdw.application_name GUC +-- =================================================================== +-- Turn debug_discard_caches off for this test to make that +-- the remote connection is alive when checking its application_name. +-- For each test, close all the existing cached connections manually and +-- establish connection with new setting of application_name. +SET debug_discard_caches = 0; +-- If appname is set as GUC but not as options of server object, +-- the GUC setting is used as application_name of remote connection. +SET postgres_fdw.application_name TO 'fdw_guc_appname'; +SELECT 1 FROM postgres_fdw_disconnect_all(); + ?column? +---------- + 1 +(1 row) + +SELECT 1 FROM ft6 LIMIT 1; + ?column? +---------- + 1 +(1 row) + +SELECT application_name FROM pg_stat_activity + WHERE application_name IN ('loopback2', 'fdw_guc_appname'); + application_name +------------------ + fdw_guc_appname +(1 row) + +-- If appname is set as options of server object but not as GUC, +-- appname of server object is used. +RESET postgres_fdw.application_name; +ALTER SERVER loopback2 OPTIONS (ADD application_name 'loopback2'); +SELECT 1 FROM postgres_fdw_disconnect_all(); + ?column? +---------- + 1 +(1 row) + +SELECT 1 FROM ft6 LIMIT 1; + ?column? +---------- + 1 +(1 row) + +SELECT application_name FROM pg_stat_activity + WHERE application_name IN ('loopback2', 'fdw_guc_appname'); + application_name +------------------ + loopback2 +(1 row) + +-- If appname is set both as GUC and as options of server object, +-- the GUC setting overrides appname of server object and is used. +SET postgres_fdw.application_name TO 'fdw_guc_appname'; +SELECT 1 FROM postgres_fdw_disconnect_all(); + ?column? +---------- + 1 +(1 row) + +SELECT 1 FROM ft6 LIMIT 1; + ?column? +---------- + 1 +(1 row) + +SELECT application_name FROM pg_stat_activity + WHERE application_name IN ('loopback2', 'fdw_guc_appname'); + application_name +------------------ + fdw_guc_appname +(1 row) + +--Clean up +ALTER SERVER loopback2 OPTIONS (DROP application_name); +RESET postgres_fdw.application_name; +RESET debug_discard_caches; diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c index c574ca2cf3..5bb1af4084 100644 --- a/contrib/postgres_fdw/option.c +++ b/contrib/postgres_fdw/option.c @@ -1,7 +1,7 @@ /*------------------------------------------------------------------------- * * option.c - * FDW option handling for postgres_fdw + * FDW and GUC option handling for postgres_fdw * * Portions Copyright (c) 2012-2021, PostgreSQL Global Development Group * @@ -45,6 +45,13 @@ static PgFdwOption *postgres_fdw_options; */ static PQconninfoOption *libpq_options; +/* + * GUC parameters + */ +char *pgfdw_application_name = NULL; + +void _PG_init(void); + /* * Helper functions */ @@ -435,3 +442,29 @@ ExtractExtensionList(const char *extensionsString, bool warnOnMissing) list_free(extlist); return extensionOids; } + +/* + * Module load callback + */ +void +_PG_init(void) +{ + /* + * Unlike application_name GUC, don't set GUC_IS_NAME flag nor check_hook + * to allow postgres_fdw.application_name to be any string more than + * NAMEDATALEN characters and to include non-ASCII characters. Instead, + * remote server truncates application_name of remote connection to less + * than NAMEDATALEN and replaces any non-ASCII characters in it with a '?' + * character. + */ + DefineCustomStringVariable("postgres_fdw.application_name", + "Sets the application name to be used on the remote server.", + NULL, + &pgfdw_application_name, + NULL, + PGC_USERSET, + 0, + NULL, + NULL, + NULL); +} diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h index ca83306af9..90b72e9ec5 100644 --- a/contrib/postgres_fdw/postgres_fdw.h +++ b/contrib/postgres_fdw/postgres_fdw.h @@ -158,6 +158,7 @@ extern int ExtractConnectionOptions(List *defelems, const char **values); extern List *ExtractExtensionList(const char *extensionsString, bool warnOnMissing); +extern char *pgfdw_application_name; /* in deparse.c */ extern void classifyConditions(PlannerInfo *root, diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 30b5175da5..20749868d3 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -3422,3 +3422,42 @@ CREATE FOREIGN TABLE inv_fsz (c1 int ) -- Invalid batch_size option CREATE FOREIGN TABLE inv_bsz (c1 int ) SERVER loopback OPTIONS (batch_size '100$%$#$#'); + +-- =================================================================== +-- test postgres_fdw.application_name GUC +-- =================================================================== +-- Turn debug_discard_caches off for this test to make that +-- the remote connection is alive when checking its application_name. +-- For each test, close all the existing cached connections manually and +-- establish connection with new setting of application_name. +SET debug_discard_caches = 0; + +-- If appname is set as GUC but not as options of server object, +-- the GUC setting is used as application_name of remote connection. +SET postgres_fdw.application_name TO 'fdw_guc_appname'; +SELECT 1 FROM postgres_fdw_disconnect_all(); +SELECT 1 FROM ft6 LIMIT 1; +SELECT application_name FROM pg_stat_activity + WHERE application_name IN ('loopback2', 'fdw_guc_appname'); + +-- If appname is set as options of server object but not as GUC, +-- appname of server object is used. +RESET postgres_fdw.application_name; +ALTER SERVER loopback2 OPTIONS (ADD application_name 'loopback2'); +SELECT 1 FROM postgres_fdw_disconnect_all(); +SELECT 1 FROM ft6 LIMIT 1; +SELECT application_name FROM pg_stat_activity + WHERE application_name IN ('loopback2', 'fdw_guc_appname'); + +-- If appname is set both as GUC and as options of server object, +-- the GUC setting overrides appname of server object and is used. +SET postgres_fdw.application_name TO 'fdw_guc_appname'; +SELECT 1 FROM postgres_fdw_disconnect_all(); +SELECT 1 FROM ft6 LIMIT 1; +SELECT application_name FROM pg_stat_activity + WHERE application_name IN ('loopback2', 'fdw_guc_appname'); + +--Clean up +ALTER SERVER loopback2 OPTIONS (DROP application_name); +RESET postgres_fdw.application_name; +RESET debug_discard_caches; diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml index 0075bc3dbb..bf95da9721 100644 --- a/doc/src/sgml/postgres-fdw.sgml +++ b/doc/src/sgml/postgres-fdw.sgml @@ -905,6 +905,31 @@ postgres=# SELECT postgres_fdw_disconnect_all(); </para> </sect2> + <sect2> + <title>Configuration Parameters</title> + + <variablelist> + <varlistentry> + <term> + <varname>postgres_fdw.application_name</varname> (<type>string</type>) + <indexterm> + <primary><varname>postgres_fdw.application_name</varname> configuration parameter</primary> + </indexterm> + </term> + <listitem> + <para> + Specifies a value for <xref linkend="guc-application-name"/> + configuration parameter used when <filename>postgres_fdw</filename> + establishes a connection to a foreign server. This overrides + <varname>application_name</varname> option of the server object. + Note that change of this parameter doesn't affect any existing + connections until they are re-established. + </para> + </listitem> + </varlistentry> + </variablelist> + </sect2> + <sect2> <title>Examples</title>