pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedAllow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
18+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2022-01-07 353aa01687 postgres_fdw: Add regression test for postgres_fdw.application_name GUC.
2021-12-24 6e0cb3dec1 postgres_fdw: Allow postgres_fdw.application_name to include escape sequences.
2021-12-16 58e2e6eb67 doc: Add note about postgres_fdw.application_name.
2021-09-08 98dbef90eb postgres_fdw: Revert unstable tests for postgres_fdw.application_name.
2021-09-07 449ab63505 postgres_fdw: Allow application_name of remote connection to be set via GUC.
Related threads (commitfest entry Allow escape sequence in application_name/3277):
[postgres_fdw] add local pid to fallback_application_name
* Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
@ 2021-08-05 03:18 kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 1 reply; 18+ messages in thread
From: kuroda.hayato@fujitsu.com @ 2021-08-05 03:18 UTC (permalink / raw)
To: kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>
Dear Hackers, Tom,
(I changed subject because this is no longer postgres_fdw's patch)
> > What would be better to think about is how to let users specify this
> > kind of behavior for themselves. I think it's possible to set
> > application_name as part of a foreign server's connection options,
> > but at present the result would only be a constant string. Somebody
> > who wished the PID to be in there would like to have some sort of
> > formatting escape, say "%p" for PID. Extrapolating wildly, maybe we
> > could make all the %-codes known to log_line_prefix available here.
>
> I think your argument is better than mine. I will try to implement this approach.
I made a patch based on your advice. I add parse and rewrite function in connectOptions2().
I implemented in libpq layer, hence any other application can be used.
Here is an example:
```
$ env PGAPPNAME=000%p%utest000 psql -d postgres -c "show application_name"
application_name
-----------------------
00025632hayatotest000
(1 row)
```
> > I don't think this is a great idea as-is. People who need to do this
> > sort of thing will all have their own ideas of what they need to track
> > --- most obviously, it might be appropriate to include the originating
> > server's name, else you don't know what machine the PID is for.
>
> I thought this is not big problem because hostname (or IP address) can be
> added to log_line_prefix. I added only local-pid because this is the only thing
> that cannot be set in the parameter.
Currently network hostname (or IP address) cannot be set because of the above reason.
Some backends' code must be modified if we want to get and embed such information.
Of cause we can get system hostname by gethostname(), but I cannot judge whether
it is meaningful.
How do you think?
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
allow_escape_in_allication_name.patch (application/octet-stream, 6.9K) download
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index a3e1c59a82..ee5a8ede06 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -82,7 +82,7 @@
#include "utils/guc.h"
#include "utils/memutils.h"
#include "utils/ps_status.h"
-
+#include "common/string.h"
/* In this module, access gettext() via err_gettext() */
#undef _
@@ -177,7 +177,6 @@ static void set_errdata_field(MemoryContextData *cxt, char **ptr, const char *st
static void write_console(const char *line, int len);
static void setup_formatted_log_time(void);
static void setup_formatted_start_time(void);
-static const char *process_log_prefix_padding(const char *p, int *padding);
static void log_line_prefix(StringInfo buf, ErrorData *edata);
static void write_csvlog(ErrorData *edata);
static void send_message_to_server_log(ErrorData *edata);
@@ -2338,41 +2337,6 @@ setup_formatted_start_time(void)
pg_localtime(&stamp_time, log_timezone));
}
-/*
- * process_log_prefix_padding --- helper function for processing the format
- * string in log_line_prefix
- *
- * Note: This function returns NULL if it finds something which
- * it deems invalid in the format string.
- */
-static const char *
-process_log_prefix_padding(const char *p, int *ppadding)
-{
- int paddingsign = 1;
- int padding = 0;
-
- if (*p == '-')
- {
- p++;
-
- if (*p == '\0') /* Did the buf end in %- ? */
- return NULL;
- paddingsign = -1;
- }
-
- /* generate an int version of the numerical string */
- while (*p >= '0' && *p <= '9')
- padding = padding * 10 + (*p++ - '0');
-
- /* format is invalid if it ends with the padding number */
- if (*p == '\0')
- return NULL;
-
- padding *= paddingsign;
- *ppadding = padding;
- return p;
-}
-
/*
* Format tag info for log lines; append to the provided buffer.
*/
@@ -2427,7 +2391,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
/*
* Process any formatting which may exist after the '%'. Note that
- * process_log_prefix_padding moves p past the padding number if it
+ * process_padding moves p past the padding number if it
* exists.
*
* Note: Since only '-', '0' to '9' are valid formatting characters we
@@ -2441,7 +2405,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
*/
if (*p > '9')
padding = 0;
- else if ((p = process_log_prefix_padding(p, &padding)) == NULL)
+ else if ((p = process_padding(p, &padding)) == NULL)
break;
/* process the option */
diff --git a/src/common/string.c b/src/common/string.c
index 3aa378c051..9f8b5c3468 100644
--- a/src/common/string.c
+++ b/src/common/string.c
@@ -128,3 +128,38 @@ pg_strip_crlf(char *str)
return len;
}
+
+/*
+ * process_padding --- helper function for processing the format
+ * string
+ *
+ * Note: This function returns NULL if it finds something which
+ * it deems invalid in the format string.
+ */
+const char *
+process_padding(const char *p, int *ppadding)
+{
+ int paddingsign = 1;
+ int padding = 0;
+
+ if (*p == '-')
+ {
+ p++;
+
+ if (*p == '\0') /* Did the buf end in %- ? */
+ return NULL;
+ paddingsign = -1;
+ }
+
+ /* generate an int version of the numerical string */
+ while (*p >= '0' && *p <= '9')
+ padding = padding * 10 + (*p++ - '0');
+
+ /* format is invalid if it ends with the padding number */
+ if (*p == '\0')
+ return NULL;
+
+ padding *= paddingsign;
+ *ppadding = padding;
+ return p;
+}
diff --git a/src/include/common/string.h b/src/include/common/string.h
index 686c158efe..1f14e3124f 100644
--- a/src/include/common/string.h
+++ b/src/include/common/string.h
@@ -19,6 +19,7 @@ extern int strtoint(const char *pg_restrict str, char **pg_restrict endptr,
extern void pg_clean_ascii(char *str);
extern int pg_strip_crlf(char *str);
extern bool pg_is_ascii(const char *str);
+extern const char* process_padding(const char *p, int *ppadding);
/* functions in src/common/pg_get_line.c */
extern char *pg_get_line(FILE *stream);
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 49eec3e835..9e7d3d35e6 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -427,6 +427,8 @@ static void default_threadlock(int acquire);
static bool sslVerifyProtocolVersion(const char *version);
static bool sslVerifyProtocolRange(const char *min, const char *max);
+static void parse_and_rewrite_appnames(PGconn *conn);
+static void parse_and_rewrite_appname_internal(PGconn *conn, char **name);
/* global variable because fe-auth.c needs to access it */
pgthreadlock_t pg_g_threadlock = default_threadlock;
@@ -1436,6 +1438,13 @@ connectOptions2(PGconn *conn)
goto oom_error;
}
+ /*
+ * if we have application_name or fallback_application_name,
+ * parse it and rewrite escape characters
+ */
+ if (conn->appname || conn->fbappname)
+ parse_and_rewrite_appnames(conn);
+
/*
* Only if we get this far is it appropriate to try to connect. (We need a
* state flag, rather than just the boolean result of this function, in
@@ -3834,6 +3843,96 @@ error_return:
return PGRES_POLLING_FAILED;
}
+static void
+parse_and_rewrite_appnames(PGconn *conn)
+{
+ if (conn->appname && conn->appname[0] != '\0')
+ parse_and_rewrite_appname_internal(conn, &conn->appname);
+ else if (conn->fbappname && conn->fbappname[0] != '\0')
+ parse_and_rewrite_appname_internal(conn, &conn->fbappname);
+}
+
+/*
+ * Almost same as log_line_prefix
+ */
+static void
+parse_and_rewrite_appname_internal(PGconn *conn, char **name)
+{
+ PQExpBuffer buf = createPQExpBuffer();
+ const char *p;
+ int padding;
+
+ for (p = *name; *p != '\0'; p++)
+ {
+ if (*p != '%')
+ {
+ /* literal char, just copy */
+ appendPQExpBufferChar(buf, *p);
+ continue;
+ }
+
+ /* must be a '%', so skip to the next char */
+ p++;
+ if (*p == '\0')
+ break; /* format error - ignore it */
+ else if (*p == '%')
+ {
+ /* string contains %% */
+ appendPQExpBufferChar(buf, '%');
+ continue;
+ }
+
+ /* calculate padding, same as log_line_prefix */
+ if (*p > '9')
+ padding = 0;
+ else if ((p = process_padding(p, &padding)) == NULL)
+ break;
+
+ /* process the option */
+ switch (*p)
+ {
+ case 'd':
+ {
+ const char *dbname = conn->dbName;
+
+ if (padding != 0)
+ appendPQExpBuffer(buf, "%*s", padding, dbname);
+ else
+ appendPQExpBufferStr(buf, dbname);
+ }
+ break;
+ case 'u':
+ {
+ const char *username = conn->pguser;
+
+ if (padding != 0)
+ appendPQExpBuffer(buf, "%*s", padding, username);
+ else
+ appendPQExpBufferStr(buf, username);
+ }
+ break;
+ case 'p':
+ {
+ int pid = (int) getpid();
+
+ if (padding != 0)
+ appendPQExpBuffer(buf, "%*d", padding, pid);
+ else
+ appendPQExpBuffer(buf, "%d", pid);
+ }
+ break;
+ default:
+ /* format error - ignore it */
+ break;
+ }
+ }
+
+ /* Finnaly replace the string */
+ free(*name);
+ *name = strdup(buf->data);
+
+ destroyPQExpBuffer(buf);
+}
/*
* internal_ping
* Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
@ 2021-08-19 11:08 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-08-26 06:00 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
0 siblings, 1 reply; 18+ messages in thread
From: Fujii Masao @ 2021-08-19 11:08 UTC (permalink / raw)
To: kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>
On 2021/08/05 12:18, kuroda.hayato@fujitsu.com wrote:
> Dear Hackers, Tom,
>
> (I changed subject because this is no longer postgres_fdw's patch)
>
>>> What would be better to think about is how to let users specify this
>>> kind of behavior for themselves. I think it's possible to set
>>> application_name as part of a foreign server's connection options,
>>> but at present the result would only be a constant string. Somebody
>>> who wished the PID to be in there would like to have some sort of
>>> formatting escape, say "%p" for PID. Extrapolating wildly, maybe we
>>> could make all the %-codes known to log_line_prefix available here.
>>
>> I think your argument is better than mine. I will try to implement this approach.
>
> I made a patch based on your advice. I add parse and rewrite function in connectOptions2().
> I implemented in libpq layer, hence any other application can be used.
> Here is an example:
>
> ```
> $ env PGAPPNAME=000%p%utest000 psql -d postgres -c "show application_name"
> application_name
> -----------------------
> 00025632hayatotest000
> (1 row)
> ```
Why did you make even %u and %d available in application_name?
With the patch, they are replaced with the user name to connect as
and the database name to connect to, respectively. Unlike %p
(i.e., PID in *client* side), those information can be easily viewed via
log_line_prefix and pg_stat_activity, etc in the server side.
So I'm not sure how much helpful to expose them also in application_name.
>
>>> I don't think this is a great idea as-is. People who need to do this
>>> sort of thing will all have their own ideas of what they need to track
>>> --- most obviously, it might be appropriate to include the originating
>>> server's name, else you don't know what machine the PID is for.
So some people may want to specify their own ID in application_name
when connecting to the foreign server. For now they can do that by
setting application_name per foreign server object. But this means that
every session connecting to the same foreign server basically should
use the same application_name. If they want to change the setting,
they need to execute "ALTER SERAVER ... OPTIONS ...". Isn't this inconvenient?
If yes, what about allowing us to specify foreign server's application_name
per session? For example, we can add postgres_fdw.application_name GUC,
and then if it's set postgres_fdw always uses it instead of the server-level
option when connecting to the foreign server. Thought?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 18+ messages in thread
* RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2021-08-26 06:00 ` kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-31 07:11 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
0 siblings, 1 reply; 18+ messages in thread
From: kuroda.hayato@fujitsu.com @ 2021-08-26 06:00 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
Dear Fujii-san,
Thank you for replying! I attached new version.
> Why did you make even %u and %d available in application_name?
Actually no particular reason. I added them because they can easily add...
And I agree what you say, so removed.
> So some people may want to specify their own ID in application_name
> when connecting to the foreign server. For now they can do that by
> setting application_name per foreign server object. But this means that
> every session connecting to the same foreign server basically should
> use the same application_name. If they want to change the setting,
> they need to execute "ALTER SERAVER ... OPTIONS ...". Isn't this inconvenient?
Yeah, currently such users must execute ALTER command for each time.
> If yes, what about allowing us to specify foreign server's application_name
> per session? For example, we can add postgres_fdw.application_name GUC,
> and then if it's set postgres_fdw always uses it instead of the server-level
> option when connecting to the foreign server. Thought?
OK, I added the GUC parameter. My patch Defines new GUC at _PG_init().
The value is used when backends started to connect to remote server.
Normal users can modify the value by SET commands but
that change does not propagate to the established connections.
I'm not sure about the versioning policy about contrib.
Do we have to make new version? Any other objects are not changed.
Finally, I and Fujii-san were now discussing locally whether
this replacement should be in libpq or postgres_fdw layer.
I started to think that it should be postgres_fdw, because:
* previously aplications could distinguish by using current applicaiton_name,
* libpq is used almost all uses so some modifications might affect someone, and
* libpq might be just a tool, and should not be intelligent
I will make and attach another version that new codes move to postgres_fdw.
So could you vote which version is better?
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments: download all (2 files, .tar.gz)
v02_0002_add_guc.patch (application/octet-stream, 3.9K) download
commit 8783a78011e3aaa84c2fd32152cc42490eed316b
Author: kuroda.hayato%40jp.fujitsu.com <kuroda.hayato@jp.fujitsu.com>
Date: Thu Aug 26 05:11:37 2021 +0000
add application_name to postgres_fdw
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 82aa14a65d..0dc13f4f58 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -29,6 +29,7 @@
#include "utils/inval.h"
#include "utils/memutils.h"
#include "utils/syscache.h"
+#include "utils/guc.h"
/*
* Connection cache hash table entry
@@ -79,6 +80,10 @@ static unsigned int prep_stmt_number = 0;
/* tracks whether any work is needed in callback functions */
static bool xact_got_connection = false;
+/* GUC parameters */
+static char* pgfdw_application_name = NULL;
+void _PG_init(void);
+
/*
* SQL functions
*/
@@ -354,7 +359,8 @@ 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.
+ * for application_name or fallback_application_name, client_encoding,
+ * end marker.
*/
n = list_length(server->options) + list_length(user->options) + 3;
keywords = (const char **) palloc(n * sizeof(char *));
@@ -366,10 +372,20 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
n += ExtractConnectionOptions(user->options,
keywords + n, values + n);
- /* Use "postgres_fdw" as fallback_application_name. */
- keywords[n] = "fallback_application_name";
- values[n] = "postgres_fdw";
- n++;
+ if (pgfdw_application_name && *pgfdw_application_name != '\0')
+ {
+ /* Use GUC paramter if set */
+ keywords[n] = "application_name";
+ values[n] = pgfdw_application_name;
+ n++;
+ }
+ else
+ {
+ /* Use "postgres_fdw" as fallback_application_name */
+ keywords[n] = "fallback_application_name";
+ values[n] = "postgres_fdw";
+ n++;
+ }
/* Set client_encoding so that libpq can convert encoding properly. */
keywords[n] = "client_encoding";
@@ -1645,3 +1661,21 @@ disconnect_cached_connections(Oid serverid)
return result;
}
+
+/*
+ * Define GUC parameters.
+ */
+void
+_PG_init(void)
+{
+ DefineCustomStringVariable("postgres_fdw.application_name",
+ "Sets the application name. This is used when connects to the remote server.",
+ NULL,
+ &pgfdw_application_name,
+ NULL,
+ PGC_USERSET,
+ GUC_IS_NAME,
+ NULL,
+ NULL,
+ NULL);
+}
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 9d443baf02..a4720f9e0a 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -318,6 +318,8 @@ typedef struct
*/
PG_FUNCTION_INFO_V1(postgres_fdw_handler);
+void _PG_init(void);
+
/*
* FDW callback routines
*/
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 0075bc3dbb..b6af6e118d 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -105,6 +105,26 @@
of columns to the remote table is by name, not position.
</para>
+ <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. This value
+ is used only when a backend process starts to establish the remote connection.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </sect2>
+
<sect2>
<title>FDW Options of postgres_fdw</title>
v02_0001_allow_escape_in_application_name.patch (application/octet-stream, 6.7K) download
commit 8783a78011e3aaa84c2fd32152cc42490eed316b
Author: kuroda.hayato%40jp.fujitsu.com <kuroda.hayato@jp.fujitsu.com>
Date: Thu Aug 26 05:11:37 2021 +0000
add application_name to postgres_fdw
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index a3e1c59a82..ee5a8ede06 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -82,7 +82,7 @@
#include "utils/guc.h"
#include "utils/memutils.h"
#include "utils/ps_status.h"
-
+#include "common/string.h"
/* In this module, access gettext() via err_gettext() */
#undef _
@@ -177,7 +177,6 @@ static void set_errdata_field(MemoryContextData *cxt, char **ptr, const char *st
static void write_console(const char *line, int len);
static void setup_formatted_log_time(void);
static void setup_formatted_start_time(void);
-static const char *process_log_prefix_padding(const char *p, int *padding);
static void log_line_prefix(StringInfo buf, ErrorData *edata);
static void write_csvlog(ErrorData *edata);
static void send_message_to_server_log(ErrorData *edata);
@@ -2338,41 +2337,6 @@ setup_formatted_start_time(void)
pg_localtime(&stamp_time, log_timezone));
}
-/*
- * process_log_prefix_padding --- helper function for processing the format
- * string in log_line_prefix
- *
- * Note: This function returns NULL if it finds something which
- * it deems invalid in the format string.
- */
-static const char *
-process_log_prefix_padding(const char *p, int *ppadding)
-{
- int paddingsign = 1;
- int padding = 0;
-
- if (*p == '-')
- {
- p++;
-
- if (*p == '\0') /* Did the buf end in %- ? */
- return NULL;
- paddingsign = -1;
- }
-
- /* generate an int version of the numerical string */
- while (*p >= '0' && *p <= '9')
- padding = padding * 10 + (*p++ - '0');
-
- /* format is invalid if it ends with the padding number */
- if (*p == '\0')
- return NULL;
-
- padding *= paddingsign;
- *ppadding = padding;
- return p;
-}
-
/*
* Format tag info for log lines; append to the provided buffer.
*/
@@ -2427,7 +2391,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
/*
* Process any formatting which may exist after the '%'. Note that
- * process_log_prefix_padding moves p past the padding number if it
+ * process_padding moves p past the padding number if it
* exists.
*
* Note: Since only '-', '0' to '9' are valid formatting characters we
@@ -2441,7 +2405,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
*/
if (*p > '9')
padding = 0;
- else if ((p = process_log_prefix_padding(p, &padding)) == NULL)
+ else if ((p = process_padding(p, &padding)) == NULL)
break;
/* process the option */
diff --git a/src/common/string.c b/src/common/string.c
index 3aa378c051..9f8b5c3468 100644
--- a/src/common/string.c
+++ b/src/common/string.c
@@ -128,3 +128,38 @@ pg_strip_crlf(char *str)
return len;
}
+
+/*
+ * process_padding --- helper function for processing the format
+ * string
+ *
+ * Note: This function returns NULL if it finds something which
+ * it deems invalid in the format string.
+ */
+const char *
+process_padding(const char *p, int *ppadding)
+{
+ int paddingsign = 1;
+ int padding = 0;
+
+ if (*p == '-')
+ {
+ p++;
+
+ if (*p == '\0') /* Did the buf end in %- ? */
+ return NULL;
+ paddingsign = -1;
+ }
+
+ /* generate an int version of the numerical string */
+ while (*p >= '0' && *p <= '9')
+ padding = padding * 10 + (*p++ - '0');
+
+ /* format is invalid if it ends with the padding number */
+ if (*p == '\0')
+ return NULL;
+
+ padding *= paddingsign;
+ *ppadding = padding;
+ return p;
+}
diff --git a/src/include/common/string.h b/src/include/common/string.h
index 686c158efe..1f14e3124f 100644
--- a/src/include/common/string.h
+++ b/src/include/common/string.h
@@ -19,6 +19,7 @@ extern int strtoint(const char *pg_restrict str, char **pg_restrict endptr,
extern void pg_clean_ascii(char *str);
extern int pg_strip_crlf(char *str);
extern bool pg_is_ascii(const char *str);
+extern const char* process_padding(const char *p, int *ppadding);
/* functions in src/common/pg_get_line.c */
extern char *pg_get_line(FILE *stream);
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 49eec3e835..55fbad3087 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -427,6 +427,8 @@ static void default_threadlock(int acquire);
static bool sslVerifyProtocolVersion(const char *version);
static bool sslVerifyProtocolRange(const char *min, const char *max);
+static void parse_and_rewrite_appnames(PGconn *conn);
+static void parse_and_rewrite_appname_internal(PGconn *conn, char **name);
/* global variable because fe-auth.c needs to access it */
pgthreadlock_t pg_g_threadlock = default_threadlock;
@@ -1436,6 +1438,13 @@ connectOptions2(PGconn *conn)
goto oom_error;
}
+ /*
+ * if we have application_name or fallback_application_name,
+ * parse it and rewrite escape characters
+ */
+ if (conn->appname || conn->fbappname)
+ parse_and_rewrite_appnames(conn);
+
/*
* Only if we get this far is it appropriate to try to connect. (We need a
* state flag, rather than just the boolean result of this function, in
@@ -3834,6 +3843,76 @@ error_return:
return PGRES_POLLING_FAILED;
}
+static void
+parse_and_rewrite_appnames(PGconn *conn)
+{
+ if (conn->appname && conn->appname[0] != '\0')
+ parse_and_rewrite_appname_internal(conn, &conn->appname);
+ else if (conn->fbappname && conn->fbappname[0] != '\0')
+ parse_and_rewrite_appname_internal(conn, &conn->fbappname);
+}
+
+/*
+ * Almost same as log_line_prefix
+ */
+static void
+parse_and_rewrite_appname_internal(PGconn *conn, char **name)
+{
+ PQExpBuffer buf = createPQExpBuffer();
+ const char *p;
+ int padding;
+
+ for (p = *name; *p != '\0'; p++)
+ {
+ if (*p != '%')
+ {
+ /* literal char, just copy */
+ appendPQExpBufferChar(buf, *p);
+ continue;
+ }
+
+ /* must be a '%', so skip to the next char */
+ p++;
+ if (*p == '\0')
+ break; /* format error - ignore it */
+ else if (*p == '%')
+ {
+ /* string contains %% */
+ appendPQExpBufferChar(buf, '%');
+ continue;
+ }
+
+ /* calculate padding, same as log_line_prefix */
+ if (*p > '9')
+ padding = 0;
+ else if ((p = process_padding(p, &padding)) == NULL)
+ break;
+
+ /* process the option */
+ switch (*p)
+ {
+ case 'p':
+ {
+ int pid = (int) getpid();
+
+ if (padding != 0)
+ appendPQExpBuffer(buf, "%*d", padding, pid);
+ else
+ appendPQExpBuffer(buf, "%d", pid);
+ }
+ break;
+ default:
+ /* format error - ignore it */
+ break;
+ }
+ }
+
+ /* Finnaly replace the string */
+ free(*name);
+ *name = strdup(buf->data);
+
+ destroyPQExpBuffer(buf);
+}
/*
* internal_ping
* RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-08-26 06:00 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
@ 2021-08-31 07:11 ` kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 05:13 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 1 reply; 18+ messages in thread
From: kuroda.hayato@fujitsu.com @ 2021-08-31 07:11 UTC (permalink / raw)
To: kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>; Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
Dear Fujii-san,
I attached new version, that almost all codes
moved from libpq to postgres_fdw.
Now we can accept four types of escapes.
All escapes will be rewritten to connection souce's information:
* application_name,
* user name,
* database name, and
* backend's pid.
These are cannot be set by log_line_prefix, so I think it is useful.
We can use escape sequences in two ways:
* Sets postgres_fdw.application_name GUC paramter, or
* Sets application_name option in CREATE SERVER command.
Which version do you like?
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments: download all (2 files, .tar.gz)
v04_0002_add_guc.patch (application/octet-stream, 8.7K) download
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 82aa14a65d..da62b0f604 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -349,16 +349,20 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
const char **keywords;
const char **values;
int n;
+ int i;
+ StringInfoData buf;
/*
* 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 *));
+ initStringInfo(&buf);
n = 0;
n += ExtractConnectionOptions(server->options,
@@ -366,7 +370,26 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
n += ExtractConnectionOptions(user->options,
keywords + n, values + n);
- /* Use "postgres_fdw" as fallback_application_name. */
+ /* Use GUC paramter if set */
+ if (pgfdw_application_name && *pgfdw_application_name != '\0')
+ {
+ keywords[n] = "application_name";
+ values[n] = pgfdw_application_name;
+ n++;
+ }
+
+ /* Search application_name and replace it */
+ for (i = n - 1; i >= 0; i--)
+ {
+ if (strcmp(keywords[i], "application_name") == 0)
+ {
+ parse_application_name(&buf, values[i]);
+ values[i] = buf.data;
+ break;
+ }
+ }
+
+ /* Use "postgres_fdw" as fallback_application_name */
keywords[n] = "fallback_application_name";
values[n] = "postgres_fdw";
n++;
@@ -437,6 +460,7 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
pfree(keywords);
pfree(values);
+ pfree(buf.data);
}
PG_CATCH();
{
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index c574ca2cf3..b9dfedf6c7 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
*
@@ -18,6 +18,9 @@
#include "catalog/pg_user_mapping.h"
#include "commands/defrem.h"
#include "commands/extension.h"
+#include "common/string.h"
+#include "lib/stringinfo.h"
+#include "libpq/libpq-be.h"
#include "postgres_fdw.h"
#include "utils/builtins.h"
#include "utils/guc.h"
@@ -52,6 +55,18 @@ static void InitPgFdwOptions(void);
static bool is_valid_option(const char *keyword, Oid context);
static bool is_libpq_option(const char *keyword);
+/*
+ * GUC parameters
+ */
+char* pgfdw_application_name = NULL;
+
+/*
+ * _PG_init() and check hook
+ */
+
+static bool check_pgfdw_application_name(char **newval, void **extra, GucSource source);
+void _PG_init(void);
+
#include "miscadmin.h"
/*
@@ -435,3 +450,118 @@ ExtractExtensionList(const char *extensionsString, bool warnOnMissing)
list_free(extlist);
return extensionOids;
}
+
+/*
+ * parse application_name and set escaped string.
+ * This function is almost same as log_line_prefix(), but
+ * accepted escape sequence is different.
+ *
+ * buf must be initialized.
+ */
+void
+parse_application_name(StringInfo buf, const char *name)
+{
+ int padding;
+ const char *p;
+
+ for(p = name; *p != '\0'; p++)
+ {
+ if (*p != '%')
+ {
+ /* literal char, just copy */
+ appendStringInfoChar(buf, *p);
+ continue;
+ }
+
+ /* must be a '%', so skip to the next char */
+ p++;
+ if (*p == '\0')
+ break; /* format error - ignore it */
+ else if (*p == '%')
+ {
+ /* string contains %% */
+ appendStringInfoChar(buf, '%');
+ continue;
+ }
+ if (*p > '9')
+ padding = 0;
+ else if ((p = process_padding(p, &padding)) == NULL)
+ break;
+
+ /* process the option */
+ switch (*p)
+ {
+ case 'a':
+ {
+ const char *appname = application_name;
+
+ if (*appname == '\0')
+ appname = "[unknown]";
+ if (padding != 0)
+ appendStringInfo(buf, "%*s", padding, appname);
+ else
+ appendStringInfoString(buf, appname);
+ }
+ break;
+ case 'u':
+ {
+ const char *username = MyProcPort->user_name;
+
+ if (padding != 0)
+ appendStringInfo(buf, "%*s", padding, username);
+ else
+ appendStringInfoString(buf, username);
+ }
+ break;
+ case 'd':
+ {
+ const char *dbname = MyProcPort->database_name;
+
+ if (padding != 0)
+ appendStringInfo(buf, "%*s", padding, dbname);
+ else
+ appendStringInfoString(buf, dbname);
+ }
+ break;
+ case 'p':
+ if (padding != 0)
+ appendStringInfo(buf, "%*d", padding, MyProcPid);
+ else
+ appendStringInfo(buf, "%d", MyProcPid);
+ break;
+ default:
+ /* format error - ignore it */
+ break;
+ }
+ }
+}
+
+/*
+ * Completely same as server-side.
+ */
+static bool
+check_pgfdw_application_name(char **newval, void **extra, GucSource source)
+{
+ /* Only allow clean ASCII chars in the application name */
+ if (*newval)
+ pg_clean_ascii(*newval);
+ return true;
+}
+
+/*
+ * Define GUC parameters.
+ */
+void
+_PG_init(void)
+{
+ DefineCustomStringVariable("postgres_fdw.application_name",
+ "Sets the application name. This is used when connects to the remote server.",
+ NULL,
+ &pgfdw_application_name,
+ NULL,
+ PGC_USERSET,
+ GUC_IS_NAME,
+ check_pgfdw_application_name,
+ NULL,
+ NULL);
+}
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 9d443baf02..a4720f9e0a 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -318,6 +318,8 @@ typedef struct
*/
PG_FUNCTION_INFO_V1(postgres_fdw_handler);
+void _PG_init(void);
+
/*
* FDW callback routines
*/
diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index ca83306af9..1168ad4b52 100644
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -158,6 +158,8 @@ extern int ExtractConnectionOptions(List *defelems,
const char **values);
extern List *ExtractExtensionList(const char *extensionsString,
bool warnOnMissing);
+extern void parse_application_name(StringInfo buf, const char *name);
+extern char *pgfdw_application_name;
/* in deparse.c */
extern void classifyConditions(PlannerInfo *root,
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 0075bc3dbb..437aed647a 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -105,6 +105,66 @@
of columns to the remote table is by name, not position.
</para>
+ <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. This value is used only when a backend process
+ starts to establish the remote connection.
+ </para>
+
+ <para>
+ Same as <xref linkend="guc-log-line-prefix"/>, this is a
+ <function>printf</function>-style string. Accepted escapes are
+ bit different from <xref linkend="guc-log-line-prefix"/>,
+ but padding can be used like as it.
+ </para>
+
+ <informaltable>
+ <tgroup cols="2">
+ <thead>
+ <row>
+ <entry>Escape</entry>
+ <entry>Effect</entry>
+ </row>
+ </thead>
+ <tbody>
+ <row>
+ <entry><literal>%a</literal></entry>
+ <entry>Application name</entry>
+ </row>
+ <row>
+ <entry><literal>%u</literal></entry>
+ <entry>Local user name</entry>
+ </row>
+ <row>
+ <entry><literal>%d</literal></entry>
+ <entry>Local database name</entry>
+ </row>
+ <row>
+ <entry><literal>%p</literal></entry>
+ <entry>Local backend process ID</entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </informaltable>
+
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </sect2>
+
<sect2>
<title>FDW Options of postgres_fdw</title>
v04_0001_move_process_padding.patch (application/octet-stream, 4.0K) download
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index a3e1c59a82..d4b3350912 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -82,6 +82,7 @@
#include "utils/guc.h"
#include "utils/memutils.h"
#include "utils/ps_status.h"
+#include "common/string.h"
/* In this module, access gettext() via err_gettext() */
@@ -177,7 +178,6 @@ static void set_errdata_field(MemoryContextData *cxt, char **ptr, const char *st
static void write_console(const char *line, int len);
static void setup_formatted_log_time(void);
static void setup_formatted_start_time(void);
-static const char *process_log_prefix_padding(const char *p, int *padding);
static void log_line_prefix(StringInfo buf, ErrorData *edata);
static void write_csvlog(ErrorData *edata);
static void send_message_to_server_log(ErrorData *edata);
@@ -2338,41 +2338,6 @@ setup_formatted_start_time(void)
pg_localtime(&stamp_time, log_timezone));
}
-/*
- * process_log_prefix_padding --- helper function for processing the format
- * string in log_line_prefix
- *
- * Note: This function returns NULL if it finds something which
- * it deems invalid in the format string.
- */
-static const char *
-process_log_prefix_padding(const char *p, int *ppadding)
-{
- int paddingsign = 1;
- int padding = 0;
-
- if (*p == '-')
- {
- p++;
-
- if (*p == '\0') /* Did the buf end in %- ? */
- return NULL;
- paddingsign = -1;
- }
-
- /* generate an int version of the numerical string */
- while (*p >= '0' && *p <= '9')
- padding = padding * 10 + (*p++ - '0');
-
- /* format is invalid if it ends with the padding number */
- if (*p == '\0')
- return NULL;
-
- padding *= paddingsign;
- *ppadding = padding;
- return p;
-}
-
/*
* Format tag info for log lines; append to the provided buffer.
*/
@@ -2427,7 +2392,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
/*
* Process any formatting which may exist after the '%'. Note that
- * process_log_prefix_padding moves p past the padding number if it
+ * process_padding moves p past the padding number if it
* exists.
*
* Note: Since only '-', '0' to '9' are valid formatting characters we
@@ -2441,7 +2406,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
*/
if (*p > '9')
padding = 0;
- else if ((p = process_log_prefix_padding(p, &padding)) == NULL)
+ else if ((p = process_padding(p, &padding)) == NULL)
break;
/* process the option */
diff --git a/src/common/string.c b/src/common/string.c
index 3aa378c051..ea5d4e87bc 100644
--- a/src/common/string.c
+++ b/src/common/string.c
@@ -107,6 +107,40 @@ pg_is_ascii(const char *str)
return true;
}
+/*
+ * process_padding --- helper function for processing the format
+ * string
+ *
+ * Note: This function returns NULL if it finds something which
+ * it deems invalid in the format string.
+ */
+const char *
+process_padding(const char *p, int *ppadding)
+{
+ int paddingsign = 1;
+ int padding = 0;
+
+ if (*p == '-')
+ {
+ p++;
+
+ if (*p == '\0') /* Did the buf end in %- ? */
+ return NULL;
+ paddingsign = -1;
+ }
+
+ /* generate an int version of the numerical string */
+ while (*p >= '0' && *p <= '9')
+ padding = padding * 10 + (*p++ - '0');
+
+ /* format is invalid if it ends with the padding number */
+ if (*p == '\0')
+ return NULL;
+
+ padding *= paddingsign;
+ *ppadding = padding;
+ return p;
+}
/*
* pg_strip_crlf -- Remove any trailing newline and carriage return
diff --git a/src/include/common/string.h b/src/include/common/string.h
index 686c158efe..1f14e3124f 100644
--- a/src/include/common/string.h
+++ b/src/include/common/string.h
@@ -19,6 +19,7 @@ extern int strtoint(const char *pg_restrict str, char **pg_restrict endptr,
extern void pg_clean_ascii(char *str);
extern int pg_strip_crlf(char *str);
extern bool pg_is_ascii(const char *str);
+extern const char* process_padding(const char *p, int *ppadding);
/* functions in src/common/pg_get_line.c */
extern char *pg_get_line(FILE *stream);
* Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-08-26 06:00 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-31 07:11 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
@ 2021-09-01 05:13 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-01 10:04 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
0 siblings, 1 reply; 18+ messages in thread
From: Fujii Masao @ 2021-09-01 05:13 UTC (permalink / raw)
To: kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>; +Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
On 2021/08/31 16:11, kuroda.hayato@fujitsu.com wrote:
> Dear Fujii-san,
>
> I attached new version, that almost all codes
> moved from libpq to postgres_fdw.
Thanks for updating the patch!
Can we split the patch into two as follows? If so, we can review
and commit them one by one.
#1. Add application_name GUC into postgres_fdw
#2. Allow to specify special escape characters in application_name that postgres_fdw uses.
>
> Now we can accept four types of escapes.
> All escapes will be rewritten to connection souce's information:
>
> * application_name,
> * user name,
> * database name, and
> * backend's pid.
+1
>
> These are cannot be set by log_line_prefix, so I think it is useful.
>
>
> We can use escape sequences in two ways:
> * Sets postgres_fdw.application_name GUC paramter, or
> * Sets application_name option in CREATE SERVER command.
OK.
>
> Which version do you like?
For now I've not found real use case that implementing the feature
in libpq would introduce. So IMO postgres_fdw version is better.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 18+ messages in thread
* RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-08-26 06:00 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-31 07:11 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 05:13 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2021-09-01 10:04 ` kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 15:40 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 1 reply; 18+ messages in thread
From: kuroda.hayato@fujitsu.com @ 2021-09-01 10:04 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
Dear Fujii-san,
> Can we split the patch into two as follows? If so, we can review
> and commit them one by one.
>
> #1. Add application_name GUC into postgres_fdw
> #2. Allow to specify special escape characters in application_name that
> postgres_fdw uses.
OK, I split and attached like that. 0001 adds new GUC, and
0002 allows to accept escapes.
> For now I've not found real use case that implementing the feature
> in libpq would introduce. So IMO postgres_fdw version is better.
+1, so I'll focus on the this version.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments: download all (2 files, .tar.gz)
v05_0002_allow_to_escape.patch (application/octet-stream, 9.9K) download
commit 3dbcec16979d57d433c5ae7fdf11c27cec609afd
Author: kuroda.hayato%40jp.fujitsu.com <kuroda.hayato@jp.fujitsu.com>
Date: Wed Sep 1 09:48:30 2021 +0000
Allow to specify special escape characters
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index af9ae8bc7d..3c0fa4413f 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -349,6 +349,8 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
const char **keywords;
const char **values;
int n;
+ int i;
+ StringInfoData buf;
/*
* Construct connection params from generic options of ForeignServer
@@ -360,6 +362,7 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
n = list_length(server->options) + list_length(user->options) + 4;
keywords = (const char **) palloc(n * sizeof(char *));
values = (const char **) palloc(n * sizeof(char *));
+ initStringInfo(&buf);
n = 0;
n += ExtractConnectionOptions(server->options,
@@ -374,6 +377,17 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
values[n] = pgfdw_application_name;
n++;
}
+ /* Search application_name and replace it */
+ for (i = n - 1; i >= 0; i--)
+ {
+ if (strcmp(keywords[i], "application_name") == 0)
+ {
+ parse_application_name(&buf, values[i]);
+ values[i] = buf.data;
+ break;
+ }
+ }
+
/* Use "postgres_fdw" as fallback_application_name */
keywords[n] = "fallback_application_name";
values[n] = "postgres_fdw";
@@ -445,6 +459,7 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
pfree(keywords);
pfree(values);
+ pfree(buf.data);
}
PG_CATCH();
{
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index da693cb74c..efd7053d01 100644
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -19,6 +19,8 @@
#include "commands/defrem.h"
#include "commands/extension.h"
#include "common/string.h"
+#include "lib/stringinfo.h"
+#include "libpq/libpq-be.h"
#include "postgres_fdw.h"
#include "utils/builtins.h"
#include "utils/guc.h"
@@ -449,6 +451,91 @@ ExtractExtensionList(const char *extensionsString, bool warnOnMissing)
return extensionOids;
}
+/*
+ * parse application_name and set escaped string.
+ * This function is almost same as log_line_prefix(), but
+ * accepted escape sequence is different.
+ *
+ * buf must be initialized.
+ */
+void
+parse_application_name(StringInfo buf, const char *name)
+{
+ int padding;
+ const char *p;
+
+ for(p = name; *p != '\0'; p++)
+ {
+ if (*p != '%')
+ {
+ /* literal char, just copy */
+ appendStringInfoChar(buf, *p);
+ continue;
+ }
+
+ /* must be a '%', so skip to the next char */
+ p++;
+ if (*p == '\0')
+ break; /* format error - ignore it */
+ else if (*p == '%')
+ {
+ /* string contains %% */
+ appendStringInfoChar(buf, '%');
+ continue;
+ }
+ if (*p > '9')
+ padding = 0;
+ else if ((p = process_padding(p, &padding)) == NULL)
+ break;
+
+ /* process the option */
+ switch (*p)
+ {
+ case 'a':
+ {
+ const char *appname = application_name;
+
+ if (*appname == '\0')
+ appname = "[unknown]";
+ if (padding != 0)
+ appendStringInfo(buf, "%*s", padding, appname);
+ else
+ appendStringInfoString(buf, appname);
+ }
+ break;
+ case 'u':
+ {
+ const char *username = MyProcPort->user_name;
+
+ if (padding != 0)
+ appendStringInfo(buf, "%*s", padding, username);
+ else
+ appendStringInfoString(buf, username);
+ }
+ break;
+ case 'd':
+ {
+ const char *dbname = MyProcPort->database_name;
+
+ if (padding != 0)
+ appendStringInfo(buf, "%*s", padding, dbname);
+ else
+ appendStringInfoString(buf, dbname);
+ }
+ break;
+ case 'p':
+ if (padding != 0)
+ appendStringInfo(buf, "%*d", padding, MyProcPid);
+ else
+ appendStringInfo(buf, "%d", MyProcPid);
+ break;
+ default:
+ /* format error - ignore it */
+ break;
+ }
+ }
+}
+
/*
* Completely same as server-side.
*/
diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index 90b72e9ec5..1168ad4b52 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 void parse_application_name(StringInfo buf, const char *name);
extern char *pgfdw_application_name;
/* in deparse.c */
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index caa10519e0..437aed647a 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -123,6 +123,43 @@
configuration parameter. This value is used only when a backend process
starts to establish the remote connection.
</para>
+
+ <para>
+ Same as <xref linkend="guc-log-line-prefix"/>, this is a
+ <function>printf</function>-style string. Accepted escapes are
+ bit different from <xref linkend="guc-log-line-prefix"/>,
+ but padding can be used like as it.
+ </para>
+
+ <informaltable>
+ <tgroup cols="2">
+ <thead>
+ <row>
+ <entry>Escape</entry>
+ <entry>Effect</entry>
+ </row>
+ </thead>
+ <tbody>
+ <row>
+ <entry><literal>%a</literal></entry>
+ <entry>Application name</entry>
+ </row>
+ <row>
+ <entry><literal>%u</literal></entry>
+ <entry>Local user name</entry>
+ </row>
+ <row>
+ <entry><literal>%d</literal></entry>
+ <entry>Local database name</entry>
+ </row>
+ <row>
+ <entry><literal>%p</literal></entry>
+ <entry>Local backend process ID</entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </informaltable>
+
</listitem>
</varlistentry>
</variablelist>
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index a3e1c59a82..d4b3350912 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -82,6 +82,7 @@
#include "utils/guc.h"
#include "utils/memutils.h"
#include "utils/ps_status.h"
+#include "common/string.h"
/* In this module, access gettext() via err_gettext() */
@@ -177,7 +178,6 @@ static void set_errdata_field(MemoryContextData *cxt, char **ptr, const char *st
static void write_console(const char *line, int len);
static void setup_formatted_log_time(void);
static void setup_formatted_start_time(void);
-static const char *process_log_prefix_padding(const char *p, int *padding);
static void log_line_prefix(StringInfo buf, ErrorData *edata);
static void write_csvlog(ErrorData *edata);
static void send_message_to_server_log(ErrorData *edata);
@@ -2338,41 +2338,6 @@ setup_formatted_start_time(void)
pg_localtime(&stamp_time, log_timezone));
}
-/*
- * process_log_prefix_padding --- helper function for processing the format
- * string in log_line_prefix
- *
- * Note: This function returns NULL if it finds something which
- * it deems invalid in the format string.
- */
-static const char *
-process_log_prefix_padding(const char *p, int *ppadding)
-{
- int paddingsign = 1;
- int padding = 0;
-
- if (*p == '-')
- {
- p++;
-
- if (*p == '\0') /* Did the buf end in %- ? */
- return NULL;
- paddingsign = -1;
- }
-
- /* generate an int version of the numerical string */
- while (*p >= '0' && *p <= '9')
- padding = padding * 10 + (*p++ - '0');
-
- /* format is invalid if it ends with the padding number */
- if (*p == '\0')
- return NULL;
-
- padding *= paddingsign;
- *ppadding = padding;
- return p;
-}
-
/*
* Format tag info for log lines; append to the provided buffer.
*/
@@ -2427,7 +2392,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
/*
* Process any formatting which may exist after the '%'. Note that
- * process_log_prefix_padding moves p past the padding number if it
+ * process_padding moves p past the padding number if it
* exists.
*
* Note: Since only '-', '0' to '9' are valid formatting characters we
@@ -2441,7 +2406,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
*/
if (*p > '9')
padding = 0;
- else if ((p = process_log_prefix_padding(p, &padding)) == NULL)
+ else if ((p = process_padding(p, &padding)) == NULL)
break;
/* process the option */
diff --git a/src/common/string.c b/src/common/string.c
index 3aa378c051..ea5d4e87bc 100644
--- a/src/common/string.c
+++ b/src/common/string.c
@@ -107,6 +107,40 @@ pg_is_ascii(const char *str)
return true;
}
+/*
+ * process_padding --- helper function for processing the format
+ * string
+ *
+ * Note: This function returns NULL if it finds something which
+ * it deems invalid in the format string.
+ */
+const char *
+process_padding(const char *p, int *ppadding)
+{
+ int paddingsign = 1;
+ int padding = 0;
+
+ if (*p == '-')
+ {
+ p++;
+
+ if (*p == '\0') /* Did the buf end in %- ? */
+ return NULL;
+ paddingsign = -1;
+ }
+
+ /* generate an int version of the numerical string */
+ while (*p >= '0' && *p <= '9')
+ padding = padding * 10 + (*p++ - '0');
+
+ /* format is invalid if it ends with the padding number */
+ if (*p == '\0')
+ return NULL;
+
+ padding *= paddingsign;
+ *ppadding = padding;
+ return p;
+}
/*
* pg_strip_crlf -- Remove any trailing newline and carriage return
diff --git a/src/include/common/string.h b/src/include/common/string.h
index 686c158efe..1f14e3124f 100644
--- a/src/include/common/string.h
+++ b/src/include/common/string.h
@@ -19,6 +19,7 @@ extern int strtoint(const char *pg_restrict str, char **pg_restrict endptr,
extern void pg_clean_ascii(char *str);
extern int pg_strip_crlf(char *str);
extern bool pg_is_ascii(const char *str);
+extern const char* process_padding(const char *p, int *ppadding);
/* functions in src/common/pg_get_line.c */
extern char *pg_get_line(FILE *stream);
v05_0001_add_application_name_GUC.patch (application/octet-stream, 5.1K) download
commit 5865a275eca08ca525b140ad3b61ea9b88276295
Author: kuroda.hayato%40jp.fujitsu.com <kuroda.hayato@jp.fujitsu.com>
Date: Wed Sep 1 09:22:42 2021 +0000
postgres_fdw: Add application_name GUC
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 82aa14a65d..af9ae8bc7d 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,14 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
n += ExtractConnectionOptions(user->options,
keywords + n, values + n);
- /* Use "postgres_fdw" as fallback_application_name. */
+ /* Use GUC paramter if set */
+ 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/option.c b/contrib/postgres_fdw/option.c
index c574ca2cf3..da693cb74c 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
*
@@ -18,6 +18,7 @@
#include "catalog/pg_user_mapping.h"
#include "commands/defrem.h"
#include "commands/extension.h"
+#include "common/string.h"
#include "postgres_fdw.h"
#include "utils/builtins.h"
#include "utils/guc.h"
@@ -52,6 +53,18 @@ static void InitPgFdwOptions(void);
static bool is_valid_option(const char *keyword, Oid context);
static bool is_libpq_option(const char *keyword);
+/*
+ * GUC parameters
+ */
+char* pgfdw_application_name = NULL;
+
+/*
+ * _PG_init() and check hook
+ */
+
+static bool check_pgfdw_application_name(char **newval, void **extra, GucSource source);
+void _PG_init(void);
+
#include "miscadmin.h"
/*
@@ -435,3 +448,33 @@ ExtractExtensionList(const char *extensionsString, bool warnOnMissing)
list_free(extlist);
return extensionOids;
}
+
+/*
+ * Completely same as server-side.
+ */
+static bool
+check_pgfdw_application_name(char **newval, void **extra, GucSource source)
+{
+ /* Only allow clean ASCII chars in the application name */
+ if (*newval)
+ pg_clean_ascii(*newval);
+ return true;
+}
+
+/*
+ * Define GUC parameters.
+ */
+void
+_PG_init(void)
+{
+ DefineCustomStringVariable("postgres_fdw.application_name",
+ "Sets the application name. This is used when connects to the remote server.",
+ NULL,
+ &pgfdw_application_name,
+ NULL,
+ PGC_USERSET,
+ GUC_IS_NAME,
+ check_pgfdw_application_name,
+ 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/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 0075bc3dbb..caa10519e0 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -105,6 +105,29 @@
of columns to the remote table is by name, not position.
</para>
+ <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. This value is used only when a backend process
+ starts to establish the remote connection.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </sect2>
+
<sect2>
<title>FDW Options of postgres_fdw</title>
* Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-08-26 06:00 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-31 07:11 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 05:13 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-01 10:04 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
@ 2021-09-01 15:40 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-02 09:27 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
0 siblings, 1 reply; 18+ messages in thread
From: Fujii Masao @ 2021-09-01 15:40 UTC (permalink / raw)
To: kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>; +Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
On 2021/09/01 19:04, kuroda.hayato@fujitsu.com wrote:
> OK, I split and attached like that. 0001 adds new GUC, and
> 0002 allows to accept escapes.
Thanks for splitting and updating the patches!
Here are the comments for 0001 patch.
- /* Use "postgres_fdw" as fallback_application_name. */
+ /* Use GUC paramter if set */
+ if (pgfdw_application_name && *pgfdw_application_name != '\0')
This GUC parameter should be set after the options of foreign server
are set so that its setting can override the server-level ones.
Isn't it better to comment this?
+static bool
+check_pgfdw_application_name(char **newval, void **extra, GucSource source)
+{
+ /* Only allow clean ASCII chars in the application name */
+ if (*newval)
+ pg_clean_ascii(*newval);
+ return true;
Do we really need this check hook? Even without that, any non-ASCII characters
in application_name would be replaced with "?" in the foreign PostgreSQL server
when it's passed to that.
On the other hand, if it's really necessary, application_name set in foreign
server object also needs to be processed in the same way.
+ NULL,
+ PGC_USERSET,
+ GUC_IS_NAME,
Why is GUC_IS_NAME flag necessary?
postgres_fdw.application_name overrides application_name set in foreign server object.
Shouldn't this information be documented?
Isn't it better to have the regression test for this feature?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 18+ messages in thread
* RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-08-26 06:00 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-31 07:11 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 05:13 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-01 10:04 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 15:40 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2021-09-02 09:27 ` kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-02 12:36 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 1 reply; 18+ messages in thread
From: kuroda.hayato@fujitsu.com @ 2021-09-02 09:27 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
Dear Fujii-san,
Thank you for reviewing!
> This GUC parameter should be set after the options of foreign server
> are set so that its setting can override the server-level ones.
> Isn't it better to comment this?
I added the following comments:
```diff
- /* Use "postgres_fdw" as fallback_application_name. */
+ /*
+ * Use pgfdw_application_name as application_name.
+ *
+ * Note that this check must be behind constructing generic options
+ * because pgfdw_application_name has higher priority.
+ */
```
> Do we really need this check hook? Even without that, any non-ASCII characters
> in application_name would be replaced with "?" in the foreign PostgreSQL server
> when it's passed to that.
>
> On the other hand, if it's really necessary, application_name set in foreign
> server object also needs to be processed in the same way.
I added check_hook because I want to make sure that
input for parse function has only ascii characters.
But in this phase we don't have such a function, so removed.
(Actually I'm not sure it is really needed, so I will check until next phase)
> Why is GUC_IS_NAME flag necessary?
I thought again and I noticed even if extremely long string is specified
it will be truncated at server-side. This check is duplicated so removed.
Maybe such a check is a user-responsibility.
> postgres_fdw.application_name overrides application_name set in foreign server
> object.
> Shouldn't this information be documented?
I added descriptions to postgres_fdw.sgml.
> Isn't it better to have the regression test for this feature?
+1, added. In that test SET the parameter and check pg_stat_activity.
Attached is the fixed version. 0002 will be rebased later.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
v06_0001_add_application_name_GUC.patch (application/octet-stream, 8.3K) download
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 82aa14a65d..9c15bc3957 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,19 @@ 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.
+ *
+ * Note that this check must be behind constructing generic options
+ * because pgfdw_application_name has higher priority.
+ */
+ 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..c3dc3c0565 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -10761,3 +10761,50 @@ 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
+-- ===================================================================
+-- Disconnect once because the value is used only when establishing connections
+DO $$
+ BEGIN
+ PERFORM postgres_fdw_disconnect_all();
+ END
+$$;
+-- Set postgres_fdw.application_name
+SET postgres_fdw.application_name TO 'guc_application_name';
+-- Add application_name server option only to loopback2
+ALTER SERVER loopback2 OPTIONS (ADD application_name 'loopback2');
+-- Start transaction because make sure remote connections are alive
+BEGIN;
+-- Make new connection and check pg_stat_activity
+SELECT FROM ft1 LIMIT 1;
+--
+(1 row)
+
+SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE 'guc_application_name';
+ count
+-------
+ 1
+(1 row)
+
+-- application_name will be overwritten by postgres_fdw.application_name
+SELECT FROM ft6 LIMIT 1;
+--
+(1 row)
+
+SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE 'loopback2';
+ count
+-------
+ 0
+(1 row)
+
+SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE 'guc_application_name';
+ count
+-------
+ 1
+(1 row)
+
+COMMIT;
+--Clean up
+ALTER SERVER loopback2 OPTIONS (DROP application_name);
+RESET postgres_fdw.application_name;
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index c574ca2cf3..3bf03c4643 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
*
@@ -18,6 +18,7 @@
#include "catalog/pg_user_mapping.h"
#include "commands/defrem.h"
#include "commands/extension.h"
+#include "common/string.h"
#include "postgres_fdw.h"
#include "utils/builtins.h"
#include "utils/guc.h"
@@ -52,6 +53,16 @@ static void InitPgFdwOptions(void);
static bool is_valid_option(const char *keyword, Oid context);
static bool is_libpq_option(const char *keyword);
+/*
+ * GUC parameters
+ */
+char* pgfdw_application_name = NULL;
+
+/*
+ * _PG_init()
+ */
+void _PG_init(void);
+
#include "miscadmin.h"
/*
@@ -435,3 +446,21 @@ ExtractExtensionList(const char *extensionsString, bool warnOnMissing)
list_free(extlist);
return extensionOids;
}
+
+/*
+ * Define GUC parameters.
+ */
+void
+_PG_init(void)
+{
+ DefineCustomStringVariable("postgres_fdw.application_name",
+ "Sets the application name. This is used when connects to 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..3bc19236a2 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -3422,3 +3422,33 @@ 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
+-- ===================================================================
+-- Disconnect once because the value is used only when establishing connections
+DO $$
+ BEGIN
+ PERFORM postgres_fdw_disconnect_all();
+ END
+$$;
+-- Set postgres_fdw.application_name
+SET postgres_fdw.application_name TO 'guc_application_name';
+-- Add application_name server option only to loopback2
+ALTER SERVER loopback2 OPTIONS (ADD application_name 'loopback2');
+
+-- Start transaction because make sure remote connections are alive
+BEGIN;
+-- Make new connection and check pg_stat_activity
+SELECT FROM ft1 LIMIT 1;
+SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE 'guc_application_name';
+
+-- application_name will be overwritten by postgres_fdw.application_name
+SELECT FROM ft6 LIMIT 1;
+SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE 'loopback2';
+SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE 'guc_application_name';
+COMMIT;
+
+--Clean up
+ALTER SERVER loopback2 OPTIONS (DROP application_name);
+RESET postgres_fdw.application_name;
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 0075bc3dbb..9afb080786 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -105,6 +105,34 @@
of columns to the remote table is by name, not position.
</para>
+ <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. This value is used only when a backend process
+ starts to establish the remote connection.
+ </para>
+ <para>
+ Note that this parameter has higher priority than sever-level options.
+ If <varname>application_name</varname> is specified as the
+ option of the server object, and set this parameter at the same time,
+ <varname>postgres_fdw.application_name</varname> will be used.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </sect2>
<sect2>
<title>FDW Options of postgres_fdw</title>
* Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-08-26 06:00 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-31 07:11 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 05:13 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-01 10:04 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 15:40 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-02 09:27 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
@ 2021-09-02 12:36 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-03 05:56 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
0 siblings, 1 reply; 18+ messages in thread
From: Fujii Masao @ 2021-09-02 12:36 UTC (permalink / raw)
To: kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>; +Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
On 2021/09/02 18:27, kuroda.hayato@fujitsu.com wrote:
> I added the following comments:
>
> ```diff
> - /* Use "postgres_fdw" as fallback_application_name. */
> + /*
> + * Use pgfdw_application_name as application_name.
> + *
> + * Note that this check must be behind constructing generic options
> + * because pgfdw_application_name has higher priority.
> + */
> ```
Thanks! What about updating the comments furthermore as follows?
---------------------------------
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.
---------------------------------
> Attached is the fixed version. 0002 will be rebased later.
Thanks for updating the patch!
+ }
+ /* Use "postgres_fdw" as fallback_application_name */
It's better to add new empty line between these two lines.
+-- Disconnect once because the value is used only when establishing connections
+DO $$
+ BEGIN
+ PERFORM postgres_fdw_disconnect_all();
+ END
+$$;
Why does DO command need to be used here to execute
postgres_fdw_disconnect_all()? Instead, we can just execute
"SELECT 1 FROM postgres_fdw_disconnect_all();"?
For test coverage, it's better to test at least the following three cases?
(1) appname is set in neither GUC nor foreign server
-> "postgres_fdw" set in fallback_application_name is used
(2) appname is set in foreign server, but not in GUC
-> appname in foreign server is used
(3) appname is set both in GUC and foreign server
-> appname in GUC is used
+SELECT FROM ft1 LIMIT 1;
"1" should be added just after SELECT in the above statement?
Because postgres_fdw regression test basically uses "SELECT 1 FROM ..."
in other places.
+ DefineCustomStringVariable("postgres_fdw.application_name",
+ "Sets the application name. This is used when connects to the remote server.",
What about simplifying this description as follows?
---------------
Sets the application name to be used on the remote server.
---------------
+ <title> Configuration Parameters </title>
+ <variablelist>
The empty characters just after <title> and before </title> should be removed?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 18+ messages in thread
* RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-08-26 06:00 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-31 07:11 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 05:13 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-01 10:04 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 15:40 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-02 09:27 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-02 12:36 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2021-09-03 05:56 ` kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-03 16:54 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 1 reply; 18+ messages in thread
From: kuroda.hayato@fujitsu.com @ 2021-09-03 05:56 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
Dear Fujii-san,
Thank you for your great works. Attached is the latest version.
> Thanks! What about updating the comments furthermore as follows?
>
> ---------------------------------
> 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.
> ---------------------------------
It's more friendly than mine because it mentions
about specification about PQconnectdbParams().
Fixed like yours.
> + }
> + /* Use "postgres_fdw" as fallback_application_name */
>
> It's better to add new empty line between these two lines.
Fixed.
> +-- Disconnect once because the value is used only when establishing
> connections
> +DO $$
> + BEGIN
> + PERFORM postgres_fdw_disconnect_all();
> + END
> +$$;
>
> Why does DO command need to be used here to execute
> postgres_fdw_disconnect_all()? Instead, we can just execute
> "SELECT 1 FROM postgres_fdw_disconnect_all();"?
DO command was used because I want to
ignore the returning value of postgres_fdw_disconnect_all().
Currently this function retruns false, but if other tests are modified,
some connection may remain and the function may become to return true.
I seeked sql file and I found that this function was called by your way.
Hence I fixed.
> For test coverage, it's better to test at least the following three cases?
>
> (1) appname is set in neither GUC nor foreign server
> -> "postgres_fdw" set in fallback_application_name is used
> (2) appname is set in foreign server, but not in GUC
> -> appname in foreign server is used
> (3) appname is set both in GUC and foreign server
> -> appname in GUC is used
I set four testcases:
(1) Sets neither GUC nor server option
(2) Sets server option, but not GUC
(3) Sets GUC but not server option
(4) Sets both GUC and server option
I confirmed it almost works fine, but I found that
fallback_application_name will be never used in our test enviroment.
It is caused because our test runner pg_regress sets PGAPPNAME to "pg_regress" and
libpq prefer the environment variable to fallback_appname.
(I tried to control it by \setenv, but failed...)
> +SELECT FROM ft1 LIMIT 1;
>
> "1" should be added just after SELECT in the above statement?
> Because postgres_fdw regression test basically uses "SELECT 1 FROM ..."
> in other places.
Fixed.
> + DefineCustomStringVariable("postgres_fdw.application_name",
> + "Sets the application name. This is used when connects to the remote server.",
>
> What about simplifying this description as follows?
>
> ---------------
> Sets the application name to be used on the remote server.
> ---------------
+1.
> + <title> Configuration Parameters </title>
> + <variablelist>
>
> The empty characters just after <title> and before </title> should be removed?
I checked other sgml file and agreed. Fixed.
And I found that including string.h is no more needed. Hence it is removed.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
v07_0001_add_application_name_GUC.patch (application/octet-stream, 10.0K) download
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 82aa14a65d..8ce9aea024 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..739b686aca 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -10761,3 +10761,104 @@ 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
+-- ===================================================================
+-- Disconnect once because the value is used only when establishing connections
+SELECT 1 FROM postgres_fdw_disconnect_all();
+ ?column?
+----------
+ 1
+(1 row)
+
+-- Remove application_name from server option
+ALTER SERVER loopback OPTIONS (DROP application_name);
+-- Start transaction because make sure remote connections are alive
+-- If appname is not set in neither GUC nor server option,
+-- the default value is used. In our environment, it is "pg_regress"
+BEGIN;
+SELECT 1 FROM ft1 LIMIT 1;
+ ?column?
+----------
+ 1
+(1 row)
+
+SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE 'pg_regress';
+ count
+-------
+ 1
+(1 row)
+
+COMMIT;
+-- Add application_name server option only to loopback2
+ALTER SERVER loopback2 OPTIONS (ADD application_name 'loopback2');
+BEGIN;
+-- If appname is set in server option but not in GUC,
+-- appname in server option is used
+SELECT 1 FROM ft6 LIMIT 1;
+ ?column?
+----------
+ 1
+(1 row)
+
+SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE 'loopback2';
+ count
+-------
+ 1
+(1 row)
+
+COMMIT;
+SELECT 1 FROM postgres_fdw_disconnect_all();
+ ?column?
+----------
+ 1
+(1 row)
+
+SET postgres_fdw.application_name TO 'guc_application_name';
+BEGIN;
+-- If appname is set in GUC but not foreign server,
+-- appname in GUC is used
+SELECT 1 FROM ft1 LIMIT 1;
+ ?column?
+----------
+ 1
+(1 row)
+
+SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE 'guc_application_name';
+ count
+-------
+ 1
+(1 row)
+
+COMMIT;
+SELECT 1 FROM postgres_fdw_disconnect('loopback');
+ ?column?
+----------
+ 1
+(1 row)
+
+BEGIN;
+-- If appname is set both in GUC and foreign server,
+-- appname in GUC is used
+SELECT 1 FROM ft6 LIMIT 1;
+ ?column?
+----------
+ 1
+(1 row)
+
+SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE 'loopback2';
+ count
+-------
+ 0
+(1 row)
+
+SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE 'guc_application_name';
+ count
+-------
+ 1
+(1 row)
+
+COMMIT;
+--Clean up
+ALTER SERVER loopback2 OPTIONS (DROP application_name);
+RESET postgres_fdw.application_name;
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index c574ca2cf3..e5de766a72 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
*
@@ -52,6 +52,16 @@ static void InitPgFdwOptions(void);
static bool is_valid_option(const char *keyword, Oid context);
static bool is_libpq_option(const char *keyword);
+/*
+ * GUC parameters
+ */
+char* pgfdw_application_name = NULL;
+
+/*
+ * _PG_init()
+ */
+void _PG_init(void);
+
#include "miscadmin.h"
/*
@@ -435,3 +445,21 @@ ExtractExtensionList(const char *extensionsString, bool warnOnMissing)
list_free(extlist);
return extensionOids;
}
+
+/*
+ * Define GUC parameters.
+ */
+void
+_PG_init(void)
+{
+ 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..81bddacbb5 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -3422,3 +3422,53 @@ 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
+-- ===================================================================
+-- Disconnect once because the value is used only when establishing connections
+SELECT 1 FROM postgres_fdw_disconnect_all();
+-- Remove application_name from server option
+ALTER SERVER loopback OPTIONS (DROP application_name);
+
+-- Start transaction because make sure remote connections are alive
+-- If appname is not set in neither GUC nor server option,
+-- the default value is used. In our environment, it is "pg_regress"
+BEGIN;
+SELECT 1 FROM ft1 LIMIT 1;
+SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE 'pg_regress';
+COMMIT;
+
+-- Add application_name server option only to loopback2
+ALTER SERVER loopback2 OPTIONS (ADD application_name 'loopback2');
+
+BEGIN;
+-- If appname is set in server option but not in GUC,
+-- appname in server option is used
+SELECT 1 FROM ft6 LIMIT 1;
+SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE 'loopback2';
+COMMIT;
+
+SELECT 1 FROM postgres_fdw_disconnect_all();
+SET postgres_fdw.application_name TO 'guc_application_name';
+
+BEGIN;
+-- If appname is set in GUC but not foreign server,
+-- appname in GUC is used
+SELECT 1 FROM ft1 LIMIT 1;
+SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE 'guc_application_name';
+COMMIT;
+
+SELECT 1 FROM postgres_fdw_disconnect('loopback');
+
+BEGIN;
+-- If appname is set both in GUC and foreign server,
+-- appname in GUC is used
+SELECT 1 FROM ft6 LIMIT 1;
+SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE 'loopback2';
+SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE 'guc_application_name';
+COMMIT;
+
+--Clean up
+ALTER SERVER loopback2 OPTIONS (DROP application_name);
+RESET postgres_fdw.application_name;
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 0075bc3dbb..e7861b02dd 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -105,6 +105,34 @@
of columns to the remote table is by name, not position.
</para>
+ <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. This value is used only when a backend process
+ starts to establish the remote connection.
+ </para>
+ <para>
+ Note that this parameter has higher priority than sever-level options.
+ If <varname>application_name</varname> is specified as the
+ option of the server object, and set this parameter at the same time,
+ <varname>postgres_fdw.application_name</varname> will be used.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </sect2>
<sect2>
<title>FDW Options of postgres_fdw</title>
* Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-08-26 06:00 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-31 07:11 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 05:13 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-01 10:04 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 15:40 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-02 09:27 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-02 12:36 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-03 05:56 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
@ 2021-09-03 16:54 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-06 01:32 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
0 siblings, 1 reply; 18+ messages in thread
From: Fujii Masao @ 2021-09-03 16:54 UTC (permalink / raw)
To: kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>; +Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
On 2021/09/03 14:56, kuroda.hayato@fujitsu.com wrote:
> Dear Fujii-san,
>
> Thank you for your great works. Attached is the latest version.
Thanks a lot!
> I set four testcases:
>
> (1) Sets neither GUC nor server option
> (2) Sets server option, but not GUC
> (3) Sets GUC but not server option
> (4) Sets both GUC and server option
OK.
> I confirmed it almost works fine, but I found that
> fallback_application_name will be never used in our test enviroment.
> It is caused because our test runner pg_regress sets PGAPPNAME to "pg_regress" and
> libpq prefer the environment variable to fallback_appname.
> (I tried to control it by \setenv, but failed...)
make check uses "pg_regress", but I guess that make installcheck uses
"postgres_fdw". So the patch would cause make installcheck to fail.
I think that the case (1) is not so important, so can be removed. Thought?
Attached is the updated version of the patch. I removed the test
for case (1). And I arranged the regression tests so that they are based
on debug_discard_caches, to simplify them. Also I added and updated
some comments and docs. Could you review this version?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Attachments:
v08_0001_add_application_name_GUC.patch (text/plain, 10.0K) download
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..6a7d83c81f 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 ft1 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..2fad091a5e 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 ft1 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>
* RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-08-26 06:00 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-31 07:11 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 05:13 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-01 10:04 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 15:40 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-02 09:27 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-02 12:36 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-03 05:56 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-03 16:54 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2021-09-06 01:32 ` kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-06 11:52 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 1 reply; 18+ messages in thread
From: kuroda.hayato@fujitsu.com @ 2021-09-06 01:32 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
Dear Fujii-san,
Thank you for updating! Your modification is very interesting and
I learn something new.
> Attached is the updated version of the patch. I removed the test
> for case (1). And I arranged the regression tests so that they are based
> on debug_discard_caches, to simplify them. Also I added and updated
> some comments and docs. Could you review this version?
I agree removing (1) because it is obvious case.
```diff
+-- 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 ft1 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)
```
I think we should SELECT ft6 because foreign server 'loopback'
doesn't have application_name server option.
I have no comments anymore.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-08-26 06:00 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-31 07:11 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 05:13 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-01 10:04 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 15:40 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-02 09:27 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-02 12:36 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-03 05:56 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-03 16:54 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-06 01:32 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
@ 2021-09-06 11:52 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-07 01:32 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
0 siblings, 1 reply; 18+ messages in thread
From: Fujii Masao @ 2021-09-06 11:52 UTC (permalink / raw)
To: kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>; +Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
On 2021/09/06 10:32, kuroda.hayato@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
Attachments:
v09_0001_add_application_name_GUC.patch (text/plain, 10.0K) download
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>
* RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-08-26 06:00 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-31 07:11 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 05:13 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-01 10:04 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 15:40 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-02 09:27 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-02 12:36 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-03 05:56 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-03 16:54 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-06 01:32 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-06 11:52 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2021-09-07 01:32 ` kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-07 03:30 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 1 reply; 18+ messages in thread
From: kuroda.hayato@fujitsu.com @ 2021-09-07 01:32 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; Masahiro Ikeda <ikedamsh@oss.nttdata.com>; +Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
Dear Fujii-san,
I confirmed it and I think it's OK.
Other comments should be included in 0002.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-08-26 06:00 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-31 07:11 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 05:13 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-01 10:04 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 15:40 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-02 09:27 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-02 12:36 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-03 05:56 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-03 16:54 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-06 01:32 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-06 11:52 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-07 01:32 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
@ 2021-09-07 03:30 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-07 11:30 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-07 22:46 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 18+ messages in thread
From: Fujii Masao @ 2021-09-07 03:30 UTC (permalink / raw)
To: kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>; Masahiro Ikeda <ikedamsh@oss.nttdata.com>; +Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
On 2021/09/07 10:32, kuroda.hayato@fujitsu.com wrote:
> Dear Fujii-san,
>
> I confirmed it and I think it's OK.
> Other comments should be included in 0002.
Pushed 0001 patch. Thanks!
Could you rebase 0002 patch?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 18+ messages in thread
* RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-08-26 06:00 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-31 07:11 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 05:13 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-01 10:04 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 15:40 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-02 09:27 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-02 12:36 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-03 05:56 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-03 16:54 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-06 01:32 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-06 11:52 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-07 01:32 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-07 03:30 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2021-09-07 11:30 ` kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
1 sibling, 0 replies; 18+ messages in thread
From: kuroda.hayato@fujitsu.com @ 2021-09-07 11:30 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; Masahiro Ikeda <ikedamsh@oss.nttdata.com>; +Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
Dear Fujii-san, Ikeda-san,
> Pushed 0001 patch. Thanks!
I confirmed your commit. Thanks!
I attached the rebased version. Tests and descriptions were added.
In my understanding Ikeda-san's indication is included.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
v10_0002_allow_escapes.patch (application/octet-stream, 14.2K) download
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 705f60a3ae..f51355d84d 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -17,6 +17,7 @@
#include "catalog/pg_user_mapping.h"
#include "commands/defrem.h"
#include "funcapi.h"
+#include "lib/stringinfo.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
#include "pgstat.h"
@@ -349,6 +350,8 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
const char **keywords;
const char **values;
int n;
+ int i;
+ StringInfoData buf;
/*
* Construct connection params from generic options of ForeignServer
@@ -360,6 +363,7 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
n = list_length(server->options) + list_length(user->options) + 4;
keywords = (const char **) palloc(n * sizeof(char *));
values = (const char **) palloc(n * sizeof(char *));
+ initStringInfo(&buf);
n = 0;
n += ExtractConnectionOptions(server->options,
@@ -383,6 +387,22 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
n++;
}
+ /*
+ * Search application_name and replace it if found.
+ *
+ * We search paramters from the end because the later
+ * one have higher priority. See also the previous comment.
+ */
+ for (i = n -1; i >= 0; i--)
+ {
+ if (strcmp(keywords[i], "application_name") == 0)
+ {
+ parse_application_name(&buf, values[i]);
+ values[i] = buf.data;
+ break;
+ }
+ }
+
/* Use "postgres_fdw" as fallback_application_name */
keywords[n] = "fallback_application_name";
values[n] = "postgres_fdw";
@@ -454,6 +474,7 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
pfree(keywords);
pfree(values);
+ pfree(buf.data);
}
PG_CATCH();
{
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 39befa394a..b65bc942f1 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -10836,7 +10836,48 @@ SELECT application_name FROM pg_stat_activity
fdw_guc_appname
(1 row)
+-- Some escapes can be used for this GUC.
+SET postgres_fdw.application_name TO '%a%u%d%p%%';
+-- All escape candidates depend on the runtime environment
+-- and it causes some fails for this tests.
+-- Hence we wrapped the check and hid some values.
+-- It returns true if works well.
+CREATE FUNCTION for_escapes() RETURNS bool AS $$
+ DECLARE
+ appname text;
+ c bool;
+ BEGIN
+ SHOW application_name INTO appname;
+ EXECUTE 'SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE '''
+ || appname
+ || current_user
+ || current_database()
+ || pg_backend_pid()
+ || '%'''
+ into c;
+ RETURN c;
+ END
+$$ LANGUAGE plpgsql;
+SELECT 1 FROM postgres_fdw_disconnect_all();
+ ?column?
+----------
+ 1
+(1 row)
+
+SELECT 1 FROM ft6 LIMIT 1;
+ ?column?
+----------
+ 1
+(1 row)
+
+SELECT for_escapes();
+ for_escapes
+-------------
+ t
+(1 row)
+
--Clean up
ALTER SERVER loopback2 OPTIONS (DROP application_name);
+DROP FUNCTION for_escapes;
RESET postgres_fdw.application_name;
RESET debug_discard_caches;
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index 5bb1af4084..e3cac07e6d 100644
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -18,11 +18,13 @@
#include "catalog/pg_user_mapping.h"
#include "commands/defrem.h"
#include "commands/extension.h"
+#include "common/string.h"
+#include "lib/stringinfo.h"
+#include "libpq/libpq-be.h"
#include "postgres_fdw.h"
#include "utils/builtins.h"
#include "utils/guc.h"
#include "utils/varlena.h"
-
/*
* Describes the valid options for objects that this wrapper uses.
*/
@@ -443,6 +445,99 @@ ExtractExtensionList(const char *extensionsString, bool warnOnMissing)
return extensionOids;
}
+/*
+ * parse application_name and set escaped string.
+ * This function is almost same as log_line_prefix(), but
+ * accepted escape sequence is different.
+ *
+ * Note that argument buf must be initialized.
+ */
+void
+parse_application_name(StringInfo buf, const char *name)
+{
+ int padding;
+ const char *p;
+
+ for(p = name; *p != '\0'; p++)
+ {
+ if (*p != '%')
+ {
+ /* literal char, just copy */
+ appendStringInfoChar(buf, *p);
+ continue;
+ }
+
+ /* must be a '%', so skip to the next char */
+ p++;
+ if (*p == '\0')
+ break; /* format error - ignore it */
+ else if (*p == '%')
+ {
+ /* string contains %% */
+ appendStringInfoChar(buf, '%');
+ continue;
+ }
+
+ /*
+ * Process any formatting which may exist after the '%'. Note that
+ * process_padding moves p past the padding number if it
+ * exists.
+ *
+ * For some note please see log_line_prefix().
+ */
+ if (*p > '9')
+ padding = 0;
+ else if ((p = process_padding(p, &padding)) == NULL)
+ break;
+
+ /* process the option */
+ switch (*p)
+ {
+ case 'a':
+ {
+ const char *appname = application_name;
+
+ if (*appname == '\0')
+ appname = "[unknown]";
+ if (padding != 0)
+ appendStringInfo(buf, "%*s", padding, appname);
+ else
+ appendStringInfoString(buf, appname);
+ }
+ break;
+ case 'u':
+ {
+ const char *username = MyProcPort->user_name;
+
+ if (padding != 0)
+ appendStringInfo(buf, "%*s", padding, username);
+ else
+ appendStringInfoString(buf, username);
+ }
+ break;
+ case 'd':
+ {
+ const char *dbname = MyProcPort->database_name;
+
+ if (padding != 0)
+ appendStringInfo(buf, "%*s", padding, dbname);
+ else
+ appendStringInfoString(buf, dbname);
+ }
+ break;
+ case 'p':
+ if (padding != 0)
+ appendStringInfo(buf, "%*d", padding, MyProcPid);
+ else
+ appendStringInfo(buf, "%d", MyProcPid);
+ break;
+ default:
+ /* format error - ignore it */
+ break;
+ }
+ }
+}
+
/*
* Module load callback
*/
diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index 90b72e9ec5..ed4f7b41e8 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 void parse_application_name(StringInfo buf, const char *appname);
extern char *pgfdw_application_name;
/* in deparse.c */
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 20749868d3..165630c4b0 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -3457,7 +3457,34 @@ SELECT 1 FROM ft6 LIMIT 1;
SELECT application_name FROM pg_stat_activity
WHERE application_name IN ('loopback2', 'fdw_guc_appname');
+-- Some escapes can be used for this GUC.
+SET postgres_fdw.application_name TO '%a%u%d%p%%';
+-- All escape candidates depend on the runtime environment
+-- and it causes some fails for this tests.
+-- Hence we wrapped the check and hid some values.
+-- It returns true if works well.
+CREATE FUNCTION for_escapes() RETURNS bool AS $$
+ DECLARE
+ appname text;
+ c bool;
+ BEGIN
+ SHOW application_name INTO appname;
+ EXECUTE 'SELECT COUNT(*) FROM pg_stat_activity WHERE application_name LIKE '''
+ || appname
+ || current_user
+ || current_database()
+ || pg_backend_pid()
+ || '%'''
+ into c;
+ RETURN c;
+ END
+$$ LANGUAGE plpgsql;
+SELECT 1 FROM postgres_fdw_disconnect_all();
+SELECT 1 FROM ft6 LIMIT 1;
+SELECT for_escapes();
+
--Clean up
ALTER SERVER loopback2 OPTIONS (DROP application_name);
+DROP FUNCTION for_escapes;
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 bf95da9721..5d67b85329 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -180,6 +180,11 @@ OPTIONS (ADD password_required 'false');
relationship granted by authentication modes like <literal>peer</literal>
or <literal>ident</literal> authentication.
</para>
+ <para>
+ You can use escape sequences for <literal>application_name</literal> even if
+ it is set as a connection or a user mapping option.
+ Please refer the later section.
+ </para>
</sect3>
<sect3>
@@ -909,7 +914,7 @@ postgres=# SELECT postgres_fdw_disconnect_all();
<title>Configuration Parameters</title>
<variablelist>
- <varlistentry>
+ <varlistentry id="guc-pgfdw-application-name" xreflabel="pgfdw_application_name">
<term>
<varname>postgres_fdw.application_name</varname> (<type>string</type>)
<indexterm>
@@ -925,6 +930,53 @@ postgres=# SELECT postgres_fdw_disconnect_all();
Note that change of this parameter doesn't affect any existing
connections until they are re-established.
</para>
+
+ <para>
+ Same as <xref linkend="guc-log-line-prefix"/>, this is a
+ <function>printf</function>-style string. Accepted escapes are
+ bit different from <xref linkend="guc-log-line-prefix"/>,
+ but padding can be used like as it.
+ </para>
+
+ <informaltable>
+ <tgroup cols="2">
+ <thead>
+ <row>
+ <entry>Escape</entry>
+ <entry>Effect</entry>
+ </row>
+ </thead>
+ <tbody>
+ <row>
+ <entry><literal>%a</literal></entry>
+ <entry>Application name</entry>
+ </row>
+ <row>
+ <entry><literal>%u</literal></entry>
+ <entry>Local user name</entry>
+ </row>
+ <row>
+ <entry><literal>%d</literal></entry>
+ <entry>Local database name</entry>
+ </row>
+ <row>
+ <entry><literal>%p</literal></entry>
+ <entry>Local backend process ID</entry>
+ </row>
+ <row>
+ <entry><literal>%%</literal></entry>
+ <entry>Liteal %</entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </informaltable>
+
+ <para>
+ Note that if embedded strings have Non-ASCII,
+ these characters will be replaced with the question marks (<literal>?</literal>).
+ This limitation is caused by <literal>application_name</literal>.
+ </para>
+
</listitem>
</varlistentry>
</variablelist>
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 816b071afa..33fd515f93 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -68,6 +68,7 @@
#include "access/transam.h"
#include "access/xact.h"
+#include "common/string.h"
#include "libpq/libpq.h"
#include "libpq/pqformat.h"
#include "mb/pg_wchar.h"
@@ -177,7 +178,6 @@ static void set_errdata_field(MemoryContextData *cxt, char **ptr, const char *st
static void write_console(const char *line, int len);
static void setup_formatted_log_time(void);
static void setup_formatted_start_time(void);
-static const char *process_log_prefix_padding(const char *p, int *padding);
static void log_line_prefix(StringInfo buf, ErrorData *edata);
static void write_csvlog(ErrorData *edata);
static void send_message_to_server_log(ErrorData *edata);
@@ -2338,41 +2338,6 @@ setup_formatted_start_time(void)
pg_localtime(&stamp_time, log_timezone));
}
-/*
- * process_log_prefix_padding --- helper function for processing the format
- * string in log_line_prefix
- *
- * Note: This function returns NULL if it finds something which
- * it deems invalid in the format string.
- */
-static const char *
-process_log_prefix_padding(const char *p, int *ppadding)
-{
- int paddingsign = 1;
- int padding = 0;
-
- if (*p == '-')
- {
- p++;
-
- if (*p == '\0') /* Did the buf end in %- ? */
- return NULL;
- paddingsign = -1;
- }
-
- /* generate an int version of the numerical string */
- while (*p >= '0' && *p <= '9')
- padding = padding * 10 + (*p++ - '0');
-
- /* format is invalid if it ends with the padding number */
- if (*p == '\0')
- return NULL;
-
- padding *= paddingsign;
- *ppadding = padding;
- return p;
-}
-
/*
* Format tag info for log lines; append to the provided buffer.
*/
@@ -2427,7 +2392,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
/*
* Process any formatting which may exist after the '%'. Note that
- * process_log_prefix_padding moves p past the padding number if it
+ * process_padding moves p past the padding number if it
* exists.
*
* Note: Since only '-', '0' to '9' are valid formatting characters we
@@ -2441,7 +2406,7 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
*/
if (*p > '9')
padding = 0;
- else if ((p = process_log_prefix_padding(p, &padding)) == NULL)
+ else if ((p = process_padding(p, &padding)) == NULL)
break;
/* process the option */
diff --git a/src/common/string.c b/src/common/string.c
index 3aa378c051..c5044ae15f 100644
--- a/src/common/string.c
+++ b/src/common/string.c
@@ -128,3 +128,38 @@ pg_strip_crlf(char *str)
return len;
}
+
+/*
+ * process_padding --- helper function for processing the format
+ * string in log_line_prefix
+ *
+ * Note: This function returns NULL if it finds something which
+ * it deems invalid in the format string.
+ */
+const char *
+process_padding(const char *p, int *ppadding)
+{
+ int paddingsign = 1;
+ int padding = 0;
+
+ if (*p == '-')
+ {
+ p++;
+
+ if (*p == '\0') /* Did the buf end in %- ? */
+ return NULL;
+ paddingsign = -1;
+ }
+
+ /* generate an int version of the numerical string */
+ while (*p >= '0' && *p <= '9')
+ padding = padding * 10 + (*p++ - '0');
+
+ /* format is invalid if it ends with the padding number */
+ if (*p == '\0')
+ return NULL;
+
+ padding *= paddingsign;
+ *ppadding = padding;
+ return p;
+}
diff --git a/src/include/common/string.h b/src/include/common/string.h
index 686c158efe..672dbce62f 100644
--- a/src/include/common/string.h
+++ b/src/include/common/string.h
@@ -19,6 +19,7 @@ extern int strtoint(const char *pg_restrict str, char **pg_restrict endptr,
extern void pg_clean_ascii(char *str);
extern int pg_strip_crlf(char *str);
extern bool pg_is_ascii(const char *str);
+extern const char *process_padding(const char *p, int *ppadding);
/* functions in src/common/pg_get_line.c */
extern char *pg_get_line(FILE *stream);
* Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-08-26 06:00 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-31 07:11 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 05:13 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-01 10:04 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 15:40 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-02 09:27 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-02 12:36 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-03 05:56 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-03 16:54 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-06 01:32 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-06 11:52 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-07 01:32 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-07 03:30 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2021-09-07 22:46 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-09-08 00:34 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
1 sibling, 1 reply; 18+ messages in thread
From: Tom Lane @ 2021-09-07 22:46 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>; 'Masahiro Ikeda' <ikedamsh@oss.nttdata.com>; 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>
Fujii Masao <masao.fujii@oss.nttdata.com> writes:
> Pushed 0001 patch. Thanks!
The buildfarm shows that this test case isn't terribly reliable.
TBH, I think you should just remove the test case altogether.
It does not look useful enough to justify a permanent expenditure of
testing cycles, never mind the amount of effort that would be needed to
stabilize it.
regards, tom lane
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-08-26 06:00 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-31 07:11 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 05:13 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-01 10:04 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 15:40 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-02 09:27 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-02 12:36 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-03 05:56 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-03 16:54 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-06 01:32 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-06 11:52 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-07 01:32 ` RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-07 03:30 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-07 22:46 ` Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-09-08 00:34 ` Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 0 replies; 18+ messages in thread
From: Fujii Masao @ 2021-09-08 00:34 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>; Masahiro Ikeda <ikedamsh@oss.nttdata.com>; 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>
On 2021/09/08 7:46, Tom Lane wrote:
> Fujii Masao <masao.fujii@oss.nttdata.com> writes:
>> Pushed 0001 patch. Thanks!
>
> The buildfarm shows that this test case isn't terribly reliable.
Yes... Thanks for reporting this!
> TBH, I think you should just remove the test case altogether.
> It does not look useful enough to justify a permanent expenditure of
> testing cycles, never mind the amount of effort that would be needed to
> stabilize it.
Ok, I will remove the tests.
Kuroda-san is proposing the additional feature which allows
postgres_fdw.application_name to include escape characters.
If we commit the feature, it's worth adding the similar tests
checking that the escape characters there are replaced with
expected values. So it's better to investigate what made
the tests fail, not to cause the same tests failure later.
The tests use postgres_fdw_disconnect_all() to close the existing
remote connections before establishing new remote connection
with new application_name setting. Then they check that
the expected application_name appears in pg_stat_activity.
The cause of the issue seems to be that it can take a bit time for
the existing remote connection (i.e., its corresponding backend)
to exit after postgres_fdw_disconect_all() is called.
So application_name of the existing remote connection also could
appear in pg_stat_activity when it's checked next time.
If this analysis is right, the tests that the upcoming patch adds
should handle the case where the existing remote connection
can appear in pg_stat_activity after postgres_fdw_disconect_all().
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 18+ messages in thread
end of thread, other threads:[~2021-09-08 00:34 UTC | newest]
Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-08-05 03:18 Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-19 11:08 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-08-26 06:00 ` kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-08-31 07:11 ` kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 05:13 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-01 10:04 ` kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-01 15:40 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-02 09:27 ` kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-02 12:36 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-03 05:56 ` kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-03 16:54 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-06 01:32 ` kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-06 11:52 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-07 01:32 ` kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-07 03:30 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-09-07 11:30 ` kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
2021-09-07 22:46 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-09-08 00:34 ` Fujii Masao <masao.fujii@oss.nttdata.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