Database-Role settings behaviour and docs mismatch

Started by Simon Riggsalmost 16 years ago5 messages
#1Simon Riggs
simon@2ndQuadrant.com

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

#2Alvaro Herrera
alvherre@commandprompt.com
In reply to: Simon Riggs (#1)
Re: Database-Role settings behaviour and docs mismatch

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

#3Simon Riggs
simon@2ndQuadrant.com
In reply to: Alvaro Herrera (#2)
Re: Database-Role settings behaviour and docs mismatch

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

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Simon Riggs (#3)
Re: Database-Role settings behaviour and docs mismatch

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

#5Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#4)
1 attachment(s)
Re: Database-Role settings behaviour and docs mismatch

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. +

Attachments:

/rtmp/difftext/x-diffDownload
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);