Thread

1. Database-Role settings behaviour and docs mismatch

   Simon Riggs <simon@2ndquadrant.com> — 2010-02-01T22:32:07Z

   In the docs it says
   "It is also possible to tie a session default to a specific database
   rather than to a role; see ALTER DATABASE. If there is a conflict,
   database-role-specific settings override role-specific ones, which in
   turn override database-specific ones."
   
   Whereas in process_settings() the sequence is this
   
   ApplySetting(databaseid, roleid, relsetting, PGC_S_DATABASE_USER);
   ApplySetting(InvalidOid, roleid, relsetting, PGC_S_USER);
   ApplySetting(databaseid, InvalidOid, relsetting, PGC_S_DATABASE);
   
   which looks to me like database-role specific settings are overridden by
   both user and database specific ones, in contrast to how the docs
   describe this.
   
   Am I confused, or is this a problem?
   
   Not that bothered, but seems like the docs provide more useful behaviour
   and the code less useful.
   
   -- 
    Simon Riggs           www.2ndQuadrant.com
   
   
   

2. Re: Database-Role settings behaviour and docs mismatch

   Alvaro Herrera <alvherre@commandprompt.com> — 2010-02-01T23:11:43Z

   Simon Riggs wrote:
   
   > Whereas in process_settings() the sequence is this
   > 
   > ApplySetting(databaseid, roleid, relsetting, PGC_S_DATABASE_USER);
   > ApplySetting(InvalidOid, roleid, relsetting, PGC_S_USER);
   > ApplySetting(databaseid, InvalidOid, relsetting, PGC_S_DATABASE);
   > 
   > which looks to me like database-role specific settings are overridden by
   > both user and database specific ones, in contrast to how the docs
   > describe this.
   
   Yeah, except that set_config_option contains this bit:
   
       /*
        * Ignore attempted set if overridden by previously processed setting.
        * However, if changeVal is false then plow ahead anyway since we are
        * trying to find out if the value is potentially good, not actually use
        * it. Also keep going if makeDefault is true, since we may want to set
        * the reset/stacked values even if we can't set the variable itself.
        */
       if (record->source > source)
       {
           if (changeVal && !makeDefault)
           {
               elog(DEBUG3, "\"%s\": setting ignored because previous source is higher priority",
                    name);
               return true;
           }
           changeVal = false;
       }
   
   
   > Not that bothered, but seems like the docs provide more useful behaviour
   > and the code less useful.
   
   It'd probably be worth changing the order of the ApplySetting calls so
   that it doesn't look suspicious.
   
   -- 
   Alvaro Herrera                                http://www.CommandPrompt.com/
   PostgreSQL Replication, Consulting, Custom Development, 24x7 support
   
   

3. Re: Database-Role settings behaviour and docs mismatch

   Simon Riggs <simon@2ndquadrant.com> — 2010-02-01T23:18:48Z

   On Mon, 2010-02-01 at 20:11 -0300, Alvaro Herrera wrote:
   
   > It'd probably be worth changing the order of the ApplySetting calls so
   > that it doesn't look suspicious.
   
   Just a comment would be enough I think on ApplySetting to make it clear
   that it really means ApplySettingIfNotAlreadySet()
   
   -- 
    Simon Riggs           www.2ndQuadrant.com
   
   
   

4. Re: Database-Role settings behaviour and docs mismatch

   Tom Lane <tgl@sss.pgh.pa.us> — 2010-02-02T00:02:45Z

   Simon Riggs <simon@2ndQuadrant.com> writes:
   > On Mon, 2010-02-01 at 20:11 -0300, Alvaro Herrera wrote:
   >> It'd probably be worth changing the order of the ApplySetting calls so
   >> that it doesn't look suspicious.
   
   > Just a comment would be enough I think
   
   Yeah.  Changing the order would mean that we'd do extra work applying
   and then removing conflicting settings.  But the general principle here
   is that GUC settings coming from different places are resolved by
   source priority, not order of execution.
   
   			regards, tom lane
   
   

5. Re: Database-Role settings behaviour and docs mismatch

   Bruce Momjian <bruce@momjian.us> — 2010-02-05T20:27:01Z

   Tom Lane wrote:
   > Simon Riggs <simon@2ndQuadrant.com> writes:
   > > On Mon, 2010-02-01 at 20:11 -0300, Alvaro Herrera wrote:
   > >> It'd probably be worth changing the order of the ApplySetting calls so
   > >> that it doesn't look suspicious.
   > 
   > > Just a comment would be enough I think
   > 
   > Yeah.  Changing the order would mean that we'd do extra work applying
   > and then removing conflicting settings.  But the general principle here
   > is that GUC settings coming from different places are resolved by
   > source priority, not order of execution.
   
   C comment patch attached and applied.
   
   -- 
     Bruce Momjian  <bruce@momjian.us>        http://momjian.us
     EnterpriseDB                             http://enterprisedb.com
   
     + If your life is a hard drive, Christ can be your backup. +