BUG #15271: Documentation / Error reporting on GUC parameter change

On Tue, Jul 10, 2018 at 08:59:03AM +0000, PG Bug reporting form wrote:

The following bug has been logged on the website:

Bug reference: 15271
Logged by: Akos Vandra
Email address: axos88@gmail.com
PostgreSQL version: 10.4
Operating system: Mac OS X, Linux
Description:

I am using the pg_trgm extension, and would like to change the
similarity_threshold GUC parameter default value.

Seems like when trying to alter a GUC parameter of an extension that was not
yet loaded into session memory, the ALTER DATABASE command returns with an
unexpected message, `ERROR: permission denied to set parameter
"pg_trgm.similarity_threshold"`, although that is NOT the problem.

I understand this may have sever implications, but obviously the expected
behaviour would be to be able to set that GUC parameter regardless if the
extension has been loaded into session memory (and probably load it if
not).

Workaround:
Before the `alter database` command issue a command such as `select
show_limit();` to load the extension into session memory.

Repro:
1. CONNECT as superuser
1. CREATE USER test PASSWORD 'test';
2. CREATE DATABASE test OWNER test;
3. DISCONNECT AND CONNECT as test user
4. ALTER DATABASE test SET pg_trgm.similarity_threshold = 0.42;

Expected:
Successful alter

Actual:
ERROR: permission denied to set parameter
"pg_trgm.similarity_threshold"

Workaround:

test=> alter database test set pg_trgm.similarity_threshold = 0.42;
ERROR: permission denied to set parameter "pg_trgm.similarity_threshold"
test=> select show_limit();
show_limit
------------
0.2
(1 row)

test=> alter database test set pg_trgm.similarity_threshold = 0.42;
ALTER DATABASE

Workaround effect:

test=> select show_limit();
show_limit
------------
0.2
(1 row)

test=> \q
$ psql -U test -d test
psql (10.4)
Type "help" for help.

test=> select show_limit();
show_limit
------------
0.42
(1 row)

I looked at this report and the cause seems deeper than reported. The
reporter states that having the extension loaded would fix it, but doing
the ALTER DATABASE as superuser also fixes it:

$ psql -U postgres postgres
psql (10.5)
Type "help" for help.

postgres=> CREATE USER test PASSWORD 'test';
CREATE ROLE
postgres=> CREATE DATABASE test OWNER test;
CREATE DATABASE

postgres=> \c test test
You are now connected to database "test" as user "test".
test=> ALTER DATABASE test SET pg_trgm.similarity_threshold = 0.42;
--> ERROR: permission denied to set parameter "pg_trgm.similarity_threshold"
test=> ALTER DATABASE test SET work_mem = '200MB';
--> ALTER DATABASE
test=> SET x.y = 0;
--> SET

test=> \c test postgres
You are now connected to database "test" as user "postgres".
test=> ALTER DATABASE test SET pg_trgm.similarity_threshold = 0.42;
--> ALTER DATABASE

The pastern I see is that non-superusers can't set custom GUCs via ALTER
DATABASE, though they can via plain SET. Our ALTER DATABASE
documentation has vague wording wording about this:

Only the database owner or a superuser can change the session defaults
for a database. Certain variables cannot be set this way, or can only be
set by a superuser.

I am not sure how we could improve this.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

Bruce Momjian <bruce@momjian.us> writes:

I looked at this report and the cause seems deeper than reported. The
reporter states that having the extension loaded would fix it, but doing
the ALTER DATABASE as superuser also fixes it:

Well, yeah, see guc.c's validate_option_array_item:

* There are three cases to consider:
*
* name is a known GUC variable. Check the value normally, check
* permissions normally (i.e., allow if variable is USERSET, or if it's
* SUSET and user is superuser).
*
* name is not known, but exists or can be created as a placeholder (i.e.,
* it has a prefixed name). We allow this case if you're a superuser,
* otherwise not. Superusers are assumed to know what they're doing. We
* can't allow it for other users, because when the placeholder is
* resolved it might turn out to be a SUSET variable;
* define_custom_variable assumes we checked that.
*
* name is not known and can't be created as a placeholder. Throw error,
* unless skipIfNoPermissions is true, in which case return false.

AFAICS it's behaving as designed.

Conceivably we could improve this by extending pg_db_role_setting to track
whether the assigner of a value was superuser or not, and then deciding
at value apply time whether to allow the assignment. However, that would
have downsides of its own, that you might not find out till far distant
from the mistake that you'd made a mistake.

regards, tom lane

On Tue, Aug 7, 2018 at 03:10:09PM -0400, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

I looked at this report and the cause seems deeper than reported. The
reporter states that having the extension loaded would fix it, but doing
the ALTER DATABASE as superuser also fixes it:

Well, yeah, see guc.c's validate_option_array_item:

* There are three cases to consider:
*
* name is a known GUC variable. Check the value normally, check
* permissions normally (i.e., allow if variable is USERSET, or if it's
* SUSET and user is superuser).
*
* name is not known, but exists or can be created as a placeholder (i.e.,
* it has a prefixed name). We allow this case if you're a superuser,
* otherwise not. Superusers are assumed to know what they're doing. We
* can't allow it for other users, because when the placeholder is
* resolved it might turn out to be a SUSET variable;
* define_custom_variable assumes we checked that.
*
* name is not known and can't be created as a placeholder. Throw error,
* unless skipIfNoPermissions is true, in which case return false.

AFAICS it's behaving as designed.

Conceivably we could improve this by extending pg_db_role_setting to track
whether the assigner of a value was superuser or not, and then deciding
at value apply time whether to allow the assignment. However, that would
have downsides of its own, that you might not find out till far distant
from the mistake that you'd made a mistake.

Great, thanks for the background. It is probably not worth trying to
enhance this further.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

Hey guys, I reported this a while back.

But the problem persists even as the database owner, I can't ALTER DATABASE
(as the db owner) before the extension is loaded into the session.
Actually that was the original issue. Non-owners or non-superusers can't
use ALTER DATABASE, and that's fine, but not even the DB OWNER can use
ALTER DB before a SELECT set_limit(); in case of pg_trgm.

Regards,
Akos

On Tue, 7 Aug 2018 at 21:36, Bruce Momjian <bruce@momjian.us> wrote:

Akos Vandra <axos88@gmail.com> writes:

But the problem persists even as the database owner, I can't ALTER DATABASE
(as the db owner) before the extension is loaded into the session.
Actually that was the original issue. Non-owners or non-superusers can't
use ALTER DATABASE, and that's fine, but not even the DB OWNER can use
ALTER DB before a SELECT set_limit(); in case of pg_trgm.

Yeah. It's a design limitation with no easy fix. You just have to
load the extension so that the code knows the properties of the variable.

regards, tom lane

Yeah I guessed as much :(

It's definitely something worth mentioning in the docs with a fat warning
sign though.

Thanks for looking into it.
Akos

On Tue, Aug 7, 2018, 22:53 Tom Lane <tgl@sss.pgh.pa.us> wrote:

On Tue, Aug 7, 2018 at 11:11:16PM +0200, Akos Vandra wrote:

Yeah I guessed as much :(

It's definitely something worth mentioning in the docs with a fat warning sign
though.

We do have a _vague_ warning sign. ;-)

---------------------------------------------------------------------------

Thanks for looking into it.
Akos

On Tue, Aug 7, 2018, 22:53 Tom Lane <tgl@sss.pgh.pa.us> wrote:

Akos Vandra <axos88@gmail.com> writes:

But the problem persists even as the database owner, I can't ALTER

DATABASE

(as the db owner) before the extension is loaded into the session.
Actually that was the original issue. Non-owners or non-superusers can't
use ALTER DATABASE, and that's fine, but not even the DB OWNER can use
ALTER DB before a SELECT set_limit(); in case of pg_trgm.

Yeah.� It's a design limitation with no easy fix.� You just have to
load the extension so that the code knows the properties of the variable.

� � � � � � � � � � � � regards, tom lane

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +