pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name
7+ messages / 5 participants
[nested] [flat]

* BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name
@ 2024-07-18 09:34 PG Bug reporting form <noreply@postgresql.org>
  2024-07-18 11:46 ` Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name Aleksander Alekseev <aleksander@timescale.com>
  2024-07-18 13:55 ` Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 2 replies; 7+ messages in thread

From: PG Bug reporting form @ 2024-07-18 09:34 UTC (permalink / raw)
  To: pgsql-bugs@lists.postgresql.org; +Cc: haydenwillsim@gmail.com

The following bug has been logged on the website:

Bug reference:      18544
Logged by:          Hayden Sim
Email address:      haydenwillsim@gmail.com
PostgreSQL version: 16.3
Operating system:   Docker Image (Ubuntu 24.04)
Description:        

When a connection is initialised, calling `SELECT
current_setting('hasura.user', 't');`, as expected will return a NULL.
However if you call `SET "hasura.user" TO DEFAULT;`, this will actually
intitialise the value to an empty string. 
Note this only occurs when using a period in the config name.

This can be seen in the following logs:
```
psql (16.3 (Ubuntu 16.3-0ubuntu0.24.04.1))
Type "help" for help.

postgres=# \pset null [NULL]
Null display is "[NULL]".
postgres=# SELECT current_setting('server_version_num');
 current_setting
-----------------
 160003
(1 row)

postgres=# SELECT current_setting('foo');
ERROR:  unrecognized configuration parameter "foo"
postgres=# SET "foo" TO DEFAULT;
ERROR:  unrecognized configuration parameter "foo"
postgres=# SELECT current_setting('hasura.user');
ERROR:  unrecognized configuration parameter "hasura.user"
postgres=# SET "hasura.user" TO DEFAULT;
SET
postgres=# SELECT current_setting('hasura.user');
 current_setting
-----------------

(1 row)
```

This presents a huge problem, since even when called inside a transaction
and using `SET LOCAL`. The newly corrupted config value will affect the
session even after ROLLBACK. This can be seen here:
```
psql (16.3 (Ubuntu 16.3-0ubuntu0.24.04.1))
Type "help" for help.

postgres=# SELECT current_setting('server_version_num');
 current_setting
-----------------
 160003
(1 row)

postgres=# \pset null [NULL]
Null display is "[NULL]".
postgres=# SELECT current_setting('hasura.user', 't');
 current_setting
-----------------
 [NULL]
(1 row)

postgres=# BEGIN;
BEGIN
postgres=*# SELECT current_setting('hasura.user', 't');
 current_setting
-----------------
 [NULL]
(1 row)

postgres=*# SET LOCAL "hasura.user" TO DEFAULT;
SET
postgres=*# SELECT current_setting('hasura.user', 't');
 current_setting
-----------------

(1 row)

postgres=*# ROLLBACK;
ROLLBACK
postgres=# SELECT current_setting('hasura.user', 't');
 current_setting
-----------------

(1 row)

postgres=# SELECT current_setting('hasura.user');
 current_setting
-----------------

(1 row)
```

Thank you!
Hayden



^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name
  2024-07-18 09:34 BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name PG Bug reporting form <noreply@postgresql.org>
@ 2024-07-18 11:46 ` Aleksander Alekseev <aleksander@timescale.com>
  2024-07-18 11:58   ` Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name Hayden Sim <haydenwillsim@gmail.com>
  1 sibling, 1 reply; 7+ messages in thread

From: Aleksander Alekseev @ 2024-07-18 11:46 UTC (permalink / raw)
  To: pgsql-bugs@lists.postgresql.org; +Cc: haydenwillsim@gmail.com

Hi,

> However if you call `SET "hasura.user" TO DEFAULT;`, this will actually
> intitialise the value to an empty string.

The documentation for SET says [1]:

"""
DEFAULT can be written to specify resetting the parameter to its
default value (that is, whatever value it would have had if no SET had
been executed in the current session).
"""

And for current_setting() [2]:

"""
Returns the current value of the setting setting_name. If there is no
such setting, current_setting throws an error unless missing_ok is
supplied and is true (in which case NULL is returned).
""

Personally I find it confusing. If no SET had been executed,
current_setting() throws an error because there is no such setting.
Reading this literally I would expect that SET ... DEFAULT should
destroy the setting.

It seems to me that the actual behavior is correct. Even if not,
changing it would mean breaking backward compatibility.

I believe an actionable item would be to better document what SET and
current_setting() do in three cases:

- there is no given setting
- there is a setting with default value
- there is a setting with non-default value

Thoughts?

[1]: https://www.postgresql.org/docs/current/sql-set.html
[2]: https://www.postgresql.org/docs/current/functions-admin.html

-- 
Best regards,
Aleksander Alekseev





^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name
  2024-07-18 09:34 BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name PG Bug reporting form <noreply@postgresql.org>
  2024-07-18 11:46 ` Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name Aleksander Alekseev <aleksander@timescale.com>
@ 2024-07-18 11:58   ` Hayden Sim <haydenwillsim@gmail.com>
  2024-07-18 13:53     ` Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name David G. Johnston <david.g.johnston@gmail.com>
  0 siblings, 1 reply; 7+ messages in thread

From: Hayden Sim @ 2024-07-18 11:58 UTC (permalink / raw)
  To: Aleksander Alekseev <aleksander@timescale.com>; +Cc: pgsql-bugs@lists.postgresql.org

Hi,

I think the most confusing part is the documentation saying:
"""
DEFAULT can be written to specify resetting the parameter to its
default value (that is, whatever value it would have had if no SET had
been executed in the current session).
"""

It would be expected that subsequent calls to `current_setting()` should
throw an error (without the "t" parameter.) As would have been the case if
`SET` hadn't ever been executed in the session. This is increasingly
confusing when a call to `SET LOCAL "foo.bar" TO 'baz';` runs inside a
transaction, as this will affect behaviour outside of the transaction. As
can be seen here:

```
psql (16.3 (Ubuntu 16.3-0ubuntu0.24.04.1))
Type "help" for help.

postgres=# SELECT current_setting('foo.bar');
ERROR:  unrecognized configuration parameter "foo.bar"
postgres=# BEGIN;
BEGIN
postgres=*# SET LOCAL "foo.bar" TO 'baz';
SET
postgres=*# SELECT current_setting('foo.bar');
 current_setting
-----------------
 baz
(1 row)

postgres=*# COMMIT;
COMMIT
postgres=# SELECT current_setting('foo.bar');
 current_setting
-----------------

(1 row)
```


Thank you,
Hayden


On Thu, Jul 18, 2024 at 9:46 PM Aleksander Alekseev <
aleksander@timescale.com> wrote:

> Hi,
>
> > However if you call `SET "hasura.user" TO DEFAULT;`, this will actually
> > intitialise the value to an empty string.
>
> The documentation for SET says [1]:
>
> """
> DEFAULT can be written to specify resetting the parameter to its
> default value (that is, whatever value it would have had if no SET had
> been executed in the current session).
> """
>
> And for current_setting() [2]:
>
> """
> Returns the current value of the setting setting_name. If there is no
> such setting, current_setting throws an error unless missing_ok is
> supplied and is true (in which case NULL is returned).
> ""
>
> Personally I find it confusing. If no SET had been executed,
> current_setting() throws an error because there is no such setting.
> Reading this literally I would expect that SET ... DEFAULT should
> destroy the setting.
>
> It seems to me that the actual behavior is correct. Even if not,
> changing it would mean breaking backward compatibility.
>
> I believe an actionable item would be to better document what SET and
> current_setting() do in three cases:
>
> - there is no given setting
> - there is a setting with default value
> - there is a setting with non-default value
>
> Thoughts?
>
> [1]: https://www.postgresql.org/docs/current/sql-set.html
> [2]: https://www.postgresql.org/docs/current/functions-admin.html
>
> --
> Best regards,
> Aleksander Alekseev
>

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name
  2024-07-18 09:34 BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name PG Bug reporting form <noreply@postgresql.org>
  2024-07-18 11:46 ` Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name Aleksander Alekseev <aleksander@timescale.com>
  2024-07-18 11:58   ` Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name Hayden Sim <haydenwillsim@gmail.com>
@ 2024-07-18 13:53     ` David G. Johnston <david.g.johnston@gmail.com>
  0 siblings, 0 replies; 7+ messages in thread

From: David G. Johnston @ 2024-07-18 13:53 UTC (permalink / raw)
  To: Hayden Sim <haydenwillsim@gmail.com>; +Cc: Aleksander Alekseev <aleksander@timescale.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>

On Thursday, July 18, 2024, Hayden Sim <haydenwillsim@gmail.com> wrote:

> Hi,
>
> I think the most confusing part is the documentation saying:
> """
> DEFAULT can be written to specify resetting the parameter to its
> default value (that is, whatever value it would have had if no SET had
> been executed in the current session).
> """
>
> It would be expected that subsequent calls to `current_setting()` should
> throw an error (without the "t" parameter.) As would have been the case if
> `SET` hadn't ever been executed in the session. This is increasingly
> confusing when a call to `SET LOCAL "foo.bar" TO 'baz';` runs inside a
> transaction, as this will affect behaviour outside of the transaction. As
> can be seen here:
>

I need to review exactly what I changed but my Document NULL patch in the
commitfest does cover this behavior.  In short you cannot rely on either
the error or NULL to represent unknown/unset.  The system was not designed
to allow settings that were not externally defined to the session to be
used.  If externally defined every setting both exists and has a non-null
value.

David J.

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name
  2024-07-18 09:34 BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name PG Bug reporting form <noreply@postgresql.org>
@ 2024-07-18 13:55 ` Tom Lane <tgl@sss.pgh.pa.us>
  2024-07-19 02:39   ` Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name Hayden Sim <haydenwillsim@gmail.com>
  1 sibling, 1 reply; 7+ messages in thread

From: Tom Lane @ 2024-07-18 13:55 UTC (permalink / raw)
  To: haydenwillsim@gmail.com; +Cc: pgsql-bugs@lists.postgresql.org

PG Bug reporting form <noreply@postgresql.org> writes:
> When a connection is initialised, calling `SELECT
> current_setting('hasura.user', 't');`, as expected will return a NULL.
> However if you call `SET "hasura.user" TO DEFAULT;`, this will actually
> intitialise the value to an empty string. 

That's a side effect of SET causing such a custom GUC to spring
into existence --- with an empty-string default, because there
is no better value.

> This presents a huge problem,

If you don't like it, don't use custom GUCs.  They're not officially
supported --- the only reason this is allowed at all is to allow
setting of an extension's GUCs before the extension is loaded.
See

https://www.postgresql.org/docs/current/runtime-config-custom.html

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name
  2024-07-18 09:34 BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name PG Bug reporting form <noreply@postgresql.org>
  2024-07-18 13:55 ` Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name Tom Lane <tgl@sss.pgh.pa.us>
@ 2024-07-19 02:39   ` Hayden Sim <haydenwillsim@gmail.com>
  2024-07-19 13:17     ` Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name David G. Johnston <david.g.johnston@gmail.com>
  0 siblings, 1 reply; 7+ messages in thread

From: Hayden Sim @ 2024-07-19 02:39 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs@lists.postgresql.org

I understand it is a side effect of SET causing the custom GUC to be
initialised. But the behaviour of `SET LOCAL` affecting the entire session,
even outside of the transaction seems bizarre. Should exiting the
transaction or calling `SET ... TO DEFAULT` not cause the parameter to be
deleted?
```
SELECT current_setting('param.value', 't'); -- is NULL
BEGIN;
SET LOCAL "param.value" TO 'some_value';
COMMIT;
SELECT current_setting('param.value', 't'); -- is empty string
```

This is extremely uncommon, but presented a problem when using Hasura
<https://hasura.io; Audit Logging (relevant docs
<https://hasura.io/docs/latest/schema/postgres/postgres-guides/auditing-tables/;)
with PGBouncer, as Hasura relied on this functionality and tried to assign
this value to a JSON parameter inside of a trigger.

The repro steps are as follows:

   1. Hasura connects to PGB
   2. Hasura sends transaction which looks like
   ```
   BEGIN;
   SET LOCAL "hasura.user" = '{"x-hasura-role": "role", ... various session
   variables}'
   -- Some mutation ...
   COMMIT;
   ```
   3. Another SQL Client connects to PGB and is given the same underlying
   SQL connection as the previous Hasura connection.
   4. SQL Client triggers a Hasura trigger that expects this value to be set
   ```
   UPDATE tableWithTrigger ...;
   ```
   5. SQL Client is faced with a JSON parse exception


Example of a trigger:
```
...
DECLARE
    session_variables json;
    ...
BEGIN
    ...
    session_variables = current_setting('hasura.user', 't');
```

In this case, NULL is an acceptable value and translates to JSON `null`,
however an empty string will cause an invalid JSON exception to be raised
and thus fail the trigger and the whole write.

We've lodged a bug request with Hasura and asked them to expect in their
triggers that the value could potentially be an empty string.

Though I still believe this behaviour is unexpected and if `SET LOCAL`
exists, it would be expected that the GUC is returned to its previous state
of being uninitialised, after the transaction has been finalised.

Thank you,
Hayden


On Thu, Jul 18, 2024 at 11:55 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> PG Bug reporting form <noreply@postgresql.org> writes:
> > When a connection is initialised, calling `SELECT
> > current_setting('hasura.user', 't');`, as expected will return a NULL.
> > However if you call `SET "hasura.user" TO DEFAULT;`, this will actually
> > intitialise the value to an empty string.
>
> That's a side effect of SET causing such a custom GUC to spring
> into existence --- with an empty-string default, because there
> is no better value.
>
> > This presents a huge problem,
>
> If you don't like it, don't use custom GUCs.  They're not officially
> supported --- the only reason this is allowed at all is to allow
> setting of an extension's GUCs before the extension is loaded.
> See
>
> https://www.postgresql.org/docs/current/runtime-config-custom.html
>
>                         regards, tom lane
>

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name
  2024-07-18 09:34 BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name PG Bug reporting form <noreply@postgresql.org>
  2024-07-18 13:55 ` Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name Tom Lane <tgl@sss.pgh.pa.us>
  2024-07-19 02:39   ` Re: BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name Hayden Sim <haydenwillsim@gmail.com>
@ 2024-07-19 13:17     ` David G. Johnston <david.g.johnston@gmail.com>
  0 siblings, 0 replies; 7+ messages in thread

From: David G. Johnston @ 2024-07-19 13:17 UTC (permalink / raw)
  To: Hayden Sim <haydenwillsim@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>

On Thursday, July 18, 2024, Hayden Sim <haydenwillsim@gmail.com> wrote:

> I understand it is a side effect of SET causing the custom GUC to be
> initialised. But the behaviour of `SET LOCAL` affecting the entire session,
> even outside of the transaction seems bizarre. Should exiting the
> transaction or calling `SET ... TO DEFAULT` not cause the parameter to be
> deleted?
>

Yes, it is a POLA violation.  But there is no interest in fixing this,
especially not as a bug fix. I suggest you instead support the commitfest
patch to get proper variables into PostgreSQL.

To reiterate, it is a bug in client code to rely on NULL being a setting
value (i.e., we made a mistake in providing a current setting function that
produces null instead of an error.).
There is pending documentation, that probably either needs tweaks or could
be modified to update different areas, in light of this discussion
(touching current_setting seems warranted) to add this behavior more
prominently to the docs.  Reviewing that would also help.

David J.

^ permalink  raw  reply  [nested|flat] 7+ messages in thread


end of thread, other threads:[~2024-07-19 13:17 UTC | newest]

Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-07-18 09:34 BUG #18544: Setting local config_parameter to DEFAULT behaves unexpectedly when using period in config name PG Bug reporting form <noreply@postgresql.org>
2024-07-18 11:46 ` Aleksander Alekseev <aleksander@timescale.com>
2024-07-18 11:58   ` Hayden Sim <haydenwillsim@gmail.com>
2024-07-18 13:53     ` David G. Johnston <david.g.johnston@gmail.com>
2024-07-18 13:55 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-07-19 02:39   ` Hayden Sim <haydenwillsim@gmail.com>
2024-07-19 13:17     ` David G. Johnston <david.g.johnston@gmail.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