pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #17045: 14 Beta Tighten up allowed names for custom GUC parameters breaks PostgREST
4+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2021-06-02 2955c2be79 Re-allow custom GUC names that have more than two components.
* BUG #17045: 14 Beta Tighten up allowed names for custom GUC parameters breaks PostgREST
@ 2021-06-02 19:34 PG Bug reporting form <noreply@postgresql.org>
2021-06-02 20:19 ` Re: BUG #17045: 14 Beta Tighten up allowed names for custom GUC parameters breaks PostgREST Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 4+ messages in thread
From: PG Bug reporting form @ 2021-06-02 19:34 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: robert.sosinski@reactive.io
The following bug has been logged on the website:
Bug reference: 17045
Logged by: Robert Sosinski
Email address: robert.sosinski@reactive.io
PostgreSQL version: 14beta1
Operating system: All
Description:
Recently (April 7th 2020) a new change was made to the PostgreSQL 14 beta
that changes allowed names for custom GUC keys. Please see the following:
1. The code change:
https://github.com/postgres/postgres/commit/3db826bd55cd1df0dd8c3d811f8e5b936d7ba1e4
2. Initial question about valid GUC keys:
https://www.postgresql.org/message-id/flat/20210209144059.GA21360%40depesz.com
3. More conversation about what valid GUC keys should be:
https://www.postgresql-archive.org/Tightening-up-allowed-custom-GUC-names-td6178392.html
This change allows there to only be one `.` in the GUC key. The problem is
that PostgreSQL uses multiple `.` in GUC keys to nest parameters, such as
`request.jwt.claim.role`. As such this would be a change that significantly
breaks PostgREST.
You can see the conversation at PostgREST here:
https://github.com/PostgREST/postgrest/issues/1857
It seems this change was mainly targeting special characters such as `-` and
`=`, could this be updated to allow multiple `.` characters for GUC keys as
well? There does not seem there is any issues with multiple `.` characters
in GUC keys.
Thanks!
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: BUG #17045: 14 Beta Tighten up allowed names for custom GUC parameters breaks PostgREST
2021-06-02 19:34 BUG #17045: 14 Beta Tighten up allowed names for custom GUC parameters breaks PostgREST PG Bug reporting form <noreply@postgresql.org>
@ 2021-06-02 20:19 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-06-02 22:20 ` Re: BUG #17045: 14 Beta Tighten up allowed names for custom GUC parameters breaks PostgREST Robert Sosinski <robert.sosinski@reactive.io>
0 siblings, 1 reply; 4+ messages in thread
From: Tom Lane @ 2021-06-02 20:19 UTC (permalink / raw)
To: robert.sosinski@reactive.io; +Cc: pgsql-bugs@lists.postgresql.org
PG Bug reporting form <noreply@postgresql.org> writes:
> This change allows there to only be one `.` in the GUC key. The problem is
> that PostgreSQL uses multiple `.` in GUC keys to nest parameters, such as
> `request.jwt.claim.role`. As such this would be a change that significantly
> breaks PostgREST.
Hmm. Reading the link you provide, it seems like PostgREST might be
moving away from that anyway. So I think "significantly breaks" may
be an overstatement. Still, we did expect that this wouldn't break
any reasonable usage, and there's an argument that what PostgREST did
is reasonable. (But ... do they have any cases where individual
components of such a name aren't valid identifiers?)
The larger question here is whether we (core PG) would ever want to
introduce special interpretations of custom GUC names with more than
two components. It doesn't sound out of the question, but on the
other hand I don't know of any active work in such a direction.
It might be better to let this usage alone until there's a more
pressing reason to break it.
Question for you: if we did modify this, how would you restate the
hint:
DETAIL: Custom parameter names must be of the form "identifier.identifier".
I'm having a hard time coming up with a similarly succinct explanation
of "two or more identifiers separated by dots".
regards, tom lane
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: BUG #17045: 14 Beta Tighten up allowed names for custom GUC parameters breaks PostgREST
2021-06-02 19:34 BUG #17045: 14 Beta Tighten up allowed names for custom GUC parameters breaks PostgREST PG Bug reporting form <noreply@postgresql.org>
2021-06-02 20:19 ` Re: BUG #17045: 14 Beta Tighten up allowed names for custom GUC parameters breaks PostgREST Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-06-02 22:20 ` Robert Sosinski <robert.sosinski@reactive.io>
2021-06-02 22:58 ` Re: BUG #17045: 14 Beta Tighten up allowed names for custom GUC parameters breaks PostgREST Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 4+ messages in thread
From: Robert Sosinski @ 2021-06-02 22:20 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs@lists.postgresql.org
Hi Tom,
Thanks for the quick and thoughtful reply. Really appreciate your support!
I would revise my statement to say this change to PostgreSQL 14 would
significantly break existing PostgREST applications, to the point that they
would be unable to upgrade to 14. The way PostgREST works, these GUC
parameters are used extensively in user defined functions and SQL queries,
with the "." used as the standard way to namespace parameter elements.
Syntax-wise, it would be similar as if PostgreSQL 14 changed the
schema/table qualifier from `.` to `_`. Developers could migrate all their
functions and queries before upgrading to 14, but that would be a
significant and required change for applications to continue working.
Regarding the error detail, here are some ideas for adding multiple "."
characters. Happy for any feedback:
1. Custom parameter names must include at least one namespace, such
as "namespace.custom".
2. Custom parameter names must include at least one namespace character
".", such as "namespace.parameter" or " namespace.parameter.one".
3. Custom parameter names must include at least one ".", such
as "custom.parameter".
4. Custom parameter names must include at least one ".", such
as "custom.one" or "custom.one.two".
5. Custom parameter names must include at least one ".", such
as "one.two" or "one.two.three".
6. Custom parameter names must include two or more components, such as
"one.two" or "one.two.three".
With these examples, I'm hoping to show that custom parameters must have at
least one "namespace" or "." character. Some give an extra example with two
dots to show that you can have multiple "namespaces". Again, happy to hear
your thoughts!
Also, thanks for all your work on PostgreSQL! This database is Amazing
(been using it for 20 years now) and excited to get production workloads on
14!
-Robert
On Wed, Jun 2, 2021 at 3:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> PG Bug reporting form <noreply@postgresql.org> writes:
> > This change allows there to only be one `.` in the GUC key. The problem
> is
> > that PostgreSQL uses multiple `.` in GUC keys to nest parameters, such as
> > `request.jwt.claim.role`. As such this would be a change that
> significantly
> > breaks PostgREST.
>
> Hmm. Reading the link you provide, it seems like PostgREST might be
> moving away from that anyway. So I think "significantly breaks" may
> be an overstatement. Still, we did expect that this wouldn't break
> any reasonable usage, and there's an argument that what PostgREST did
> is reasonable. (But ... do they have any cases where individual
> components of such a name aren't valid identifiers?)
>
> The larger question here is whether we (core PG) would ever want to
> introduce special interpretations of custom GUC names with more than
> two components. It doesn't sound out of the question, but on the
> other hand I don't know of any active work in such a direction.
> It might be better to let this usage alone until there's a more
> pressing reason to break it.
>
> Question for you: if we did modify this, how would you restate the
> hint:
>
> DETAIL: Custom parameter names must be of the form
> "identifier.identifier".
>
> I'm having a hard time coming up with a similarly succinct explanation
> of "two or more identifiers separated by dots".
>
> regards, tom lane
>
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: BUG #17045: 14 Beta Tighten up allowed names for custom GUC parameters breaks PostgREST
2021-06-02 19:34 BUG #17045: 14 Beta Tighten up allowed names for custom GUC parameters breaks PostgREST PG Bug reporting form <noreply@postgresql.org>
2021-06-02 20:19 ` Re: BUG #17045: 14 Beta Tighten up allowed names for custom GUC parameters breaks PostgREST Tom Lane <tgl@sss.pgh.pa.us>
2021-06-02 22:20 ` Re: BUG #17045: 14 Beta Tighten up allowed names for custom GUC parameters breaks PostgREST Robert Sosinski <robert.sosinski@reactive.io>
@ 2021-06-02 22:58 ` Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 4+ messages in thread
From: Tom Lane @ 2021-06-02 22:58 UTC (permalink / raw)
To: Robert Sosinski <robert.sosinski@reactive.io>; +Cc: pgsql-bugs@lists.postgresql.org
Robert Sosinski <robert.sosinski@reactive.io> writes:
> I would revise my statement to say this change to PostgreSQL 14 would
> significantly break existing PostgREST applications, to the point that they
> would be unable to upgrade to 14.
Fair enough. I also dug around a bit and noted that the core grammar
explicitly allows SET/SHOW variable names to have any number of
components:
var_name: ColId { $$ = $1; }
| var_name '.' ColId
{ $$ = psprintf("%s.%s", $1, $3); }
;
That dates clear back to commit 3dc37cd8d of 2004-05-26. While it's
fair to wonder how intentional it was, given that the commit log
message only mentions "qualified name in the form <ID>.<ID>",
nonetheless people have been able to write this --- without any tricks
like double-quotes --- for a mighty long time. So I now agree that
we shouldn't forbid it. Done at
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=2955c2be79b35fa369c83fa3b5f44661c...
regards, tom lane
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2021-06-02 22:58 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-06-02 19:34 BUG #17045: 14 Beta Tighten up allowed names for custom GUC parameters breaks PostgREST PG Bug reporting form <noreply@postgresql.org>
2021-06-02 20:19 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-06-02 22:20 ` Robert Sosinski <robert.sosinski@reactive.io>
2021-06-02 22:58 ` Tom Lane <tgl@sss.pgh.pa.us>
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