Proposal to add GUC_REPORT to lc_monetary, lc_numeric and search_path
Hello,
The first 2 lc_monetary and lc_numeric are useful if the client for some
reason executes set lc_*. We don't get a report and in many cases can't
continue to parse numerics or money.
Now it it possible to get these at startup by issuing show or querying the
catalog, but it seems much cleaner to just send them.
The latter is important for similar reasons. JDBC caches prepared
statements internally and if the user changes the search path without using
setSchema or uses a function to change it then internally it would be
necessary to invalidate the cache. Currently if this occurs these
statements fail.
This seems like a rather innocuous change as the protocol is not changed,
rather the amount of information returned on startup is increased
marginally.
I've included the authors of the npgsql and the node drivers in the email
for their input.
Dave Cramer
The latter is important for similar reasons. JDBC caches prepared
statements internally and if the user changes the search path without using
setSchema or uses a function to change it then internally it would be
necessary to invalidate the cache. Currently if this occurs these
statements fail.
While Npgsql specifically doesn't care about any locale/formatting (being a
binary-only driver), knowing about search_path changes would benefit Npgsql
in the same way as it would JDBC.
This seems like a rather innocuous change as the protocol is not changed,
rather the amount of information returned on startup is increased
marginally.
Although adding these specific parameters are easy to add, we could also
think of a more generic way for clients to subscribe to parameter updates
(am not sure if this was previously discussed - I cannot see anything
obvious in the wiki TODO page). At its simplest, this could be a new
parameter containing a comma-separated list of parameters for which
asynchronous updates should be sent. This new parameter would default to
the current hard-coded list (as documented in
https://www.postgresql.org/docs/current/protocol-flow.html#PROTOCOL-ASYNC).
Unless I'm mistaken, one issue (as in general with new parameters) is that
drivers wouldn't be able to send this new parameter in the startup package
because they don't yet know whether they're talking to a PostgreSQL version
which supports it.
See attached patch.
I think adding GUC_REPORT to search_path is probably the most important as
this is potentially a security issue. See joe conway's blog on security and
search path here
https://info.crunchydata.com/blog/postgresql-defaults-and-impact-on-security-part-2
I also see there was a proposal to make reportable GUC's configurable here
/messages/by-id/CA+TgmobSXsy0KFR_vDQQOXJxQAFNESFXF_-dArNE+QHhqCwrAA@mail.gmail.com
I don't really care which one gets implemented, although I think the latter
makes more sense.
Dave Cramer
On Fri, 5 Jul 2019 at 08:05, Shay Rojansky <roji@roji.org> wrote:
Show quoted text
The latter is important for similar reasons. JDBC caches prepared
statements internally and if the user changes the search path without using
setSchema or uses a function to change it then internally it would be
necessary to invalidate the cache. Currently if this occurs these
statements fail.While Npgsql specifically doesn't care about any locale/formatting (being
a binary-only driver), knowing about search_path changes would benefit
Npgsql in the same way as it would JDBC.This seems like a rather innocuous change as the protocol is not
changed, rather the amount of information returned on startup is increased
marginally.Although adding these specific parameters are easy to add, we could also
think of a more generic way for clients to subscribe to parameter updates
(am not sure if this was previously discussed - I cannot see anything
obvious in the wiki TODO page). At its simplest, this could be a new
parameter containing a comma-separated list of parameters for which
asynchronous updates should be sent. This new parameter would default to
the current hard-coded list (as documented in
https://www.postgresql.org/docs/current/protocol-flow.html#PROTOCOL-ASYNC).
Unless I'm mistaken, one issue (as in general with new parameters) is that
drivers wouldn't be able to send this new parameter in the startup package
because they don't yet know whether they're talking to a PostgreSQL version
which supports it.
Attachments:
0001-make-lc_monetary-lc_numeric-and-search_path-GUC_REPO.patchapplication/octet-stream; name=0001-make-lc_monetary-lc_numeric-and-search_path-GUC_REPO.patchDownload
From 3c3137efc9ea8a5302630dd22dd4a8d5186284fe Mon Sep 17 00:00:00 2001
From: Dave Cramer <davecramer@gmail.com>
Date: Tue, 9 Jul 2019 14:03:14 -0400
Subject: [PATCH] make lc_monetary, lc_numeric and search_path GUC_REPORT so
that we get these in the startup messages and whenever someone changes them.
---
src/backend/utils/misc/guc.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index fc463601ff..01a86d0bd8 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3739,7 +3739,8 @@ static struct config_string ConfigureNamesString[] =
{
{"lc_monetary", PGC_USERSET, CLIENT_CONN_LOCALE,
gettext_noop("Sets the locale for formatting monetary amounts."),
- NULL
+ NULL,
+ GUC_REPORT
},
&locale_monetary,
"C",
@@ -3749,7 +3750,8 @@ static struct config_string ConfigureNamesString[] =
{
{"lc_numeric", PGC_USERSET, CLIENT_CONN_LOCALE,
gettext_noop("Sets the locale for formatting numbers."),
- NULL
+ NULL,
+ GUC_REPORT
},
&locale_numeric,
"C",
@@ -3803,7 +3805,7 @@ static struct config_string ConfigureNamesString[] =
{"search_path", PGC_USERSET, CLIENT_CONN_STATEMENT,
gettext_noop("Sets the schema search order for names that are not schema-qualified."),
NULL,
- GUC_LIST_INPUT | GUC_LIST_QUOTE | GUC_EXPLAIN
+ GUC_LIST_INPUT | GUC_REPORT | GUC_LIST_QUOTE | GUC_EXPLAIN
},
&namespace_search_path,
"\"$user\", public",
--
2.20.1 (Apple Git-117)