Обсуждение: Database-Role settings behaviour and docs mismatch

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

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
plowahead anyway since we are    * trying to find out if the value is potentially good, not actually use    * it. Also
keepgoing if makeDefault is true, since we may want to set    * the reset/stacked values even if we can't set the
variableitself.    */   if (record->source > source)   {       if (changeVal && !makeDefault)       {
elog(DEBUG3,"\"%s\": setting ignored because previous source is higher priority",                name);
returntrue;       }       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

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

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

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. +
Index: src/backend/utils/init/postinit.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/init/postinit.c,v
retrieving revision 1.201
diff -c -c -r1.201 postinit.c
*** src/backend/utils/init/postinit.c    15 Jan 2010 09:19:04 -0000    1.201
--- src/backend/utils/init/postinit.c    5 Feb 2010 20:25:38 -0000
***************
*** 855,860 ****
--- 855,861 ----

      relsetting = heap_open(DbRoleSettingRelationId, AccessShareLock);

+     /* Later settings are ignored if set earlier. */
      ApplySetting(databaseid, roleid, relsetting, PGC_S_DATABASE_USER);
      ApplySetting(InvalidOid, roleid, relsetting, PGC_S_USER);
      ApplySetting(databaseid, InvalidOid, relsetting, PGC_S_DATABASE);