Обсуждение: BUG #15271: Documentation / Error reporting on GUC parameter change

Поиск
Список
Период
Сортировка

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

От
PG Bug reporting form
Дата:
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)


Re: BUG #15271: Documentation / Error reporting on GUC parameterchange

От
Bruce Momjian
Дата:
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 +


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

От
Tom Lane
Дата:
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


Re: BUG #15271: Documentation / Error reporting on GUC parameterchange

От
Bruce Momjian
Дата:
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 +


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

От
Akos Vandra
Дата:
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:
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 +

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

От
Tom Lane
Дата:
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


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

От
Akos Vandra
Дата:
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:
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

Re: BUG #15271: Documentation / Error reporting on GUC parameterchange

От
Bruce Momjian
Дата:
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 +