Re: alter user set local_preload_libraries.

Hello, This is about a document fix of local_preload_libraries
for the versions 9.3 and earlier to at least 8.4.

There's inconsistency between documentation about
local_preload_libraries for 9.3 and earlier and their behavior.

The 9.3 document for local_preload_libraries says that:

http://www.postgresql.org/docs/9.3/static/runtime-config-client.html

| For example, debugging could be enabled for all sessions under
| a given user name by setting this parameter with ALTER ROLE
| SET.

Ok, let's do that.

postgres=# alter user horiguti2 set local_preload_libraries='libname';
ERROR: parameter "local_preload_libraries" cannot be set after connection start

Back to 8.4 shows the same behavior.

session_preload_libraries works as expected since 9.4. It is
added at 9.4 vested the context PGC_SUSET so its setting in
pg_db_role_setting can complete its mission. On the other hand,
local_preload_libraries seems to continue to have the context
PCG_BACKEND and the behavior is same to the older versions, as
shown above. And the description about 'ALTER ROLE SET' has been
removed from config.sgml. These are done by the commit
070518ddab2.

I think we should regard the real behavior as right, right?

Eventually, local_preload_libraries seems to have almost the same
function to shared_preload_libraries, except a restriction on
load module directory.

Putting aside the meaning of its existence, anyway the third
paragraph of the description for local_preload_libraries should
be removed. The change made by the attached patch for 9.3 will
show in runtime-config-client.html "18.11. Client Connection
Defaults"

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> writes:
> postgres=# alter user horiguti2 set local_preload_libraries='libname';
> ERROR: parameter "local_preload_libraries" cannot be set after connection start

Hm ... it's kind of annoying that that doesn't work; it's certainly not
hard to imagine use-cases for per-user or per-database settings of
local_preload_libraries. However, I'm not sure if we apply per-user or
per-database settings early enough in backend startup that they could
affect library loading. If we do, then the ALTER code needs to be
taught to allow this.

regards, tom lane

On Wed, Jul 2, 2014 at 11:15 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> writes:
>> postgres=# alter user horiguti2 set local_preload_libraries='libname';
>> ERROR: parameter "local_preload_libraries" cannot be set after connection start
>
> Hm ... it's kind of annoying that that doesn't work; it's certainly not
> hard to imagine use-cases for per-user or per-database settings of
> local_preload_libraries. However, I'm not sure if we apply per-user or
> per-database settings early enough in backend startup that they could
> affect library loading. If we do, then the ALTER code needs to be
> taught to allow this.

ISTM that's what session_preload_libraries does. Anyway I agree with
Horiguchi that we should remove incorrect description from the document
of old versions.

Regards,

--
Fujii Masao

Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
> On Wed, Jul 2, 2014 at 11:15 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> writes:
>>> postgres=# alter user horiguti2 set local_preload_libraries='libname';
>>> ERROR: parameter "local_preload_libraries" cannot be set after connection start

>> Hm ... it's kind of annoying that that doesn't work; it's certainly not
>> hard to imagine use-cases for per-user or per-database settings of
>> local_preload_libraries. However, I'm not sure if we apply per-user or
>> per-database settings early enough in backend startup that they could
>> affect library loading. If we do, then the ALTER code needs to be
>> taught to allow this.

> ISTM that's what session_preload_libraries does.

No, the reason for the distinction is that session_preload_libraries is
superuser-only, and can load anything, while local_preload_libraries is
(supposed to be) settable by ordinary users, and therefore is restricted
to load only a subset of available libraries. But if you can't apply it
via ALTER USER that seems like it takes away a lot of the value of having
a non-SUSET GUC in this area.

regards, tom lane

On Thu, Jul 3, 2014 at 3:53 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
>> On Wed, Jul 2, 2014 at 11:15 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> writes:
>>>> postgres=# alter user horiguti2 set local_preload_libraries='libname';
>>>> ERROR: parameter "local_preload_libraries" cannot be set after connection start
>
>>> Hm ... it's kind of annoying that that doesn't work; it's certainly not
>>> hard to imagine use-cases for per-user or per-database settings of
>>> local_preload_libraries. However, I'm not sure if we apply per-user or
>>> per-database settings early enough in backend startup that they could
>>> affect library loading. If we do, then the ALTER code needs to be
>>> taught to allow this.
>
>> ISTM that's what session_preload_libraries does.
>
> No, the reason for the distinction is that session_preload_libraries is
> superuser-only, and can load anything, while local_preload_libraries is
> (supposed to be) settable by ordinary users, and therefore is restricted
> to load only a subset of available libraries. But if you can't apply it
> via ALTER USER that seems like it takes away a lot of the value of having
> a non-SUSET GUC in this area.

Agreed. I was also thinking we can set it per role, but got surprised
when I found that's impossible. Is this a problem of only
local_preload_libraries? I'm afraid that all PGC_BACKEND parameters
have the same problem.

Regards,

--
Fujii Masao

Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
> Agreed. I was also thinking we can set it per role, but got surprised
> when I found that's impossible. Is this a problem of only
> local_preload_libraries? I'm afraid that all PGC_BACKEND parameters
> have the same problem.

Well, there aren't that many PGC_BACKEND parameters.

Two of them are log_connections and log_disconnections, which we've
been arguing over whether they should be controllable by non-superusers
in the first place. The only other ones are ignore_system_indexes and
post_auth_delay, which are debugging things that I can't see using with
ALTER. So this may be the only one where it's really of much interest.

But I agree that the problem is triggered by the PGC_BACKEND categorization
and not the meaning of this specific GUC.

regards, tom lane

o <CAHGQGwFYiANahR_u_cHnz-zOurj3yQMMnJrr9RwP7vPsVXtKUw(at)mail(dot)gmail(dot)com>
<20408(dot)1404329822(at)sss(dot)pgh(dot)pa(dot)us>
User-Agent: Mew version 6.5 on Emacs 22.2 / Mule 5.0
=?iso-2022-jp?B?KBskQjpnGyhCKQ==?= / Meadow-3.01-dev (TSUBO-SUMIRE)
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hello, I'm getting confused.. The distinction between local_ and
session_ seems to be obscure..

At Wed, 02 Jul 2014 15:37:02 -0400, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in <20408(dot)1404329822(at)sss(dot)pgh(dot)pa(dot)us>
> Well, there aren't that many PGC_BACKEND parameters.
>
> Two of them are log_connections and log_disconnections, which we've
> been arguing over whether they should be controllable by non-superusers
> in the first place. The only other ones are ignore_system_indexes and
> post_auth_delay, which are debugging things that I can't see using with
> ALTER. So this may be the only one where it's really of much interest.
>
> But I agree that the problem is triggered by the PGC_BACKEND categorization
> and not the meaning of this specific GUC.

I put some thoughts on this.

The current behavior is:

- Considering setting them in postgresql.conf, the two are
different only in the restriction for the locaion of modules
to be load. But setting them is allowed only for superuser
equivalent(DBA) so the difference has no meaning.

- Considering setting them on-session, only session_* can be
altered by superuser, but no valuable result would be
retrieved from that.

- Considering setting them through pg_db_role_setting using
ALTER DATABASE/USER statements, only superuser can set only
session_* and both sessions for superuser and non-superuser
can perform it. local_* cannot be set anyway.

- Considering inserting directly into pg_db_role_setting, only
superuser can insert any setting, including
local_preload_libraries, but it fails on session start.

| =# INSERT INTO pg_db_role_setting VALUES(0, 16384, '{local_preload_libraries=auto_explain}');
| INSERT 0 1
...
| $ psql postgres -U hoge
| WARNING: parameter "local_preload_libraries" cannot be set after connection start.

After all, I suppose the desirable requirements utilizing the
both *_preload_libraries are,

- (local|session)_preload_libraries shouldn't be altered
on-session. (should have PGC_BACKEND)

- ALTER ... SET local_preload_libraries should be done by any
user, but specified modules should be within plugins
directory.

- ALTER ... SET session_preload_libraries should be set only by
superuser, and modules in any directory can be specified.

- Both (local|session)_preload_libraries should work at session
start.

- Direct setting of pg_db_role_setting by superuser allows
arbitrary setting but by the superuser's own responsibility.

The changes needed to achieve the above requirements are,

- Now PGC_BACKEND and PGC_SUSET/USERSET are in orthogonal
relationship, not in parallel. But it seems to big change for
the fruits to reflect it in straightforward way. The new
context PGC_BACKEND_USERSET seems to be enough.

- set_config_option should allow PGC_BACKEND(_USERSET)
variables to be checked (without changing) on-session.

- PGC_BACKEND(_USERSET) variables should be allowed to be
changed by set_config_option if teached that "now is on
session starting". Now changeVal is not a boolean but
tri-state variable including
(exec-on-session|exec-on-session-start|check-only)

The above description is based on 9.4 and later. local_* would be
applicable on 9.3 and before, but PGC_BACKEND_USERSET is not
needed because they don't have session_preload_libraries.

9.5dev apparently deserves to be applied. I personally think that
applying to all live versions is desirable. But it seems to be a
kind of feature change, enabling a function which cannot be used
before..

For 9.4, since session_preload_library have been introduced, the
coexistence of current local_preload_library seems to be somewhat
crooked. We might want to apply this for 9.4.

For the earlier than 9.4, no one seems to have considered
seriously to use local_preload_library via ALTER statements so
far. Only document fix would be enough for them.

Any suggestions?

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Hello, I made a set of patches to fix this issue.

The attached files are the followings,

0001_Add_PGC_BACKEND_USERSET_v0.patch:

Add new GUC category PGC_BACKEND_USERSET and change
local_preload_libraries to use it.

0002_dblink_follow_change_of_set_config_options_v0.patch:
0003_postgres_fdw_follow_change_of_set_config_options_v0.patch

Change contrib modules to follow the change of
set_config_options.

regards,
--
Kyoaro Horiguchi
NTT Open Source Software Center

On Thu, Jul 3, 2014 at 1:05 PM, Kyotaro HORIGUCHI
<horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> o <CAHGQGwFYiANahR_u_cHnz-zOurj3yQMMnJrr9RwP7vPsVXtKUw(at)mail(dot)gmail(dot)com>
> <20408(dot)1404329822(at)sss(dot)pgh(dot)pa(dot)us>
> User-Agent: Mew version 6.5 on Emacs 22.2 / Mule 5.0
> =?iso-2022-jp?B?KBskQjpnGyhCKQ==?= / Meadow-3.01-dev (TSUBO-SUMIRE)
> Mime-Version: 1.0
> Content-Type: Text/Plain; charset=us-ascii
> Content-Transfer-Encoding: 7bit
>
> Hello, I'm getting confused.. The distinction between local_ and
> session_ seems to be obscure..
>
> At Wed, 02 Jul 2014 15:37:02 -0400, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in <20408(dot)1404329822(at)sss(dot)pgh(dot)pa(dot)us>
>> Well, there aren't that many PGC_BACKEND parameters.
>>
>> Two of them are log_connections and log_disconnections, which we've
>> been arguing over whether they should be controllable by non-superusers
>> in the first place. The only other ones are ignore_system_indexes and
>> post_auth_delay, which are debugging things that I can't see using with
>> ALTER. So this may be the only one where it's really of much interest.
>>
>> But I agree that the problem is triggered by the PGC_BACKEND categorization
>> and not the meaning of this specific GUC.
>
> I put some thoughts on this.
>
> The current behavior is:
>
> - Considering setting them in postgresql.conf, the two are
> different only in the restriction for the locaion of modules
> to be load. But setting them is allowed only for superuser
> equivalent(DBA) so the difference has no meaning.
>
> - Considering setting them on-session, only session_* can be
> altered by superuser, but no valuable result would be
> retrieved from that.
>
> - Considering setting them through pg_db_role_setting using
> ALTER DATABASE/USER statements, only superuser can set only
> session_* and both sessions for superuser and non-superuser
> can perform it. local_* cannot be set anyway.
>
> - Considering inserting directly into pg_db_role_setting, only
> superuser can insert any setting, including
> local_preload_libraries, but it fails on session start.
>
> | =# INSERT INTO pg_db_role_setting VALUES(0, 16384, '{local_preload_libraries=auto_explain}');
> | INSERT 0 1
> ...
> | $ psql postgres -U hoge
> | WARNING: parameter "local_preload_libraries" cannot be set after connection start.
>
> After all, I suppose the desirable requirements utilizing the
> both *_preload_libraries are,
>
> - (local|session)_preload_libraries shouldn't be altered
> on-session. (should have PGC_BACKEND)
>
> - ALTER ... SET local_preload_libraries should be done by any
> user, but specified modules should be within plugins
> directory.
>
> - ALTER ... SET session_preload_libraries should be set only by
> superuser, and modules in any directory can be specified.
>
> - Both (local|session)_preload_libraries should work at session
> start.
>
> - Direct setting of pg_db_role_setting by superuser allows
> arbitrary setting but by the superuser's own responsibility.
>
> The changes needed to achieve the above requirements are,
>
> - Now PGC_BACKEND and PGC_SUSET/USERSET are in orthogonal
> relationship, not in parallel. But it seems to big change for
> the fruits to reflect it in straightforward way. The new
> context PGC_BACKEND_USERSET seems to be enough.
>
> - set_config_option should allow PGC_BACKEND(_USERSET)
> variables to be checked (without changing) on-session.
>
> - PGC_BACKEND(_USERSET) variables should be allowed to be
> changed by set_config_option if teached that "now is on
> session starting". Now changeVal is not a boolean but
> tri-state variable including
> (exec-on-session|exec-on-session-start|check-only)
>
>
> The above description is based on 9.4 and later. local_* would be
> applicable on 9.3 and before, but PGC_BACKEND_USERSET is not
> needed because they don't have session_preload_libraries.
>
> 9.5dev apparently deserves to be applied. I personally think that
> applying to all live versions is desirable. But it seems to be a
> kind of feature change, enabling a function which cannot be used
> before..
>
> For 9.4, since session_preload_library have been introduced, the
> coexistence of current local_preload_library seems to be somewhat
> crooked. We might want to apply this for 9.4.
>
> For the earlier than 9.4, no one seems to have considered
> seriously to use local_preload_library via ALTER statements so
> far. Only document fix would be enough for them.
>
>
> Any suggestions?
>
> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, 2014-07-03 at 13:05 +0900, Kyotaro HORIGUCHI wrote:
> For the earlier than 9.4, no one seems to have considered
> seriously to use local_preload_library via ALTER statements so
> far. Only document fix would be enough for them.

I think local_preload_libraries never actually worked sensibly for any
specific purpose. It was designed to work with the pldebugger, but
pldebugger doesn't use it anymore.

So before we start bending the GUC system into new directions, let's ask
ourselves what the current practical application of
local_preload_libraries is and whether the proposed changes support that
use at all. I suspect there aren't any, and we should leave it alone.

I agree that we should fix the documentation in 9.3 and earlier.

Hello,

> On Thu, 2014-07-03 at 13:05 +0900, Kyotaro HORIGUCHI wrote:
> > For the earlier than 9.4, no one seems to have considered
> > seriously to use local_preload_library via ALTER statements so
> > far. Only document fix would be enough for them.
>
> I think local_preload_libraries never actually worked sensibly for any
> specific purpose. It was designed to work with the pldebugger, but
> pldebugger doesn't use it anymore.

Hmm, I see although I don't know pldebugger.

> So before we start bending the GUC system into new directions, let's ask
> ourselves what the current practical application of
> local_preload_libraries is and whether the proposed changes support that
> use at all. I suspect there aren't any, and we should leave it alone.

I found this issue when trying per-pg_user (role) loading of
auto_analyze and some tweaking tool. It is not necessarily set by
the user by own, but the function to decide whether to load some
module by the session-user would be usable, at least, as for me:)

> I agree that we should fix the documentation in 9.3 and earlier.

It seems rather hard work if local_preload_library itself is not
removed or fixed as expressed..

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

On 8/28/14 9:01 AM, Kyotaro HORIGUCHI wrote:
> I found this issue when trying per-pg_user (role) loading of
> auto_analyze and some tweaking tool. It is not necessarily set by
> the user by own, but the function to decide whether to load some
> module by the session-user would be usable, at least, as for me:)

I think we could just set local_preload_libraries to PGC_USERSET and
document that subsequent changes won't take effect. That's the same way
session_preload_libraries works. That would avoid inventing another
very specialized GUC context.

If you're interested in improving this area, I also suggest you read the
thread of
<http://www.postgresql.org/message-id/1349829917.29682.5.camel@vanquo.pezone.net>.

Hello,

> > I found this issue when trying per-pg_user (role) loading of
> > auto_analyze and some tweaking tool. It is not necessarily set by
> > the user by own, but the function to decide whether to load some
> > module by the session-user would be usable, at least, as for me:)
>
> I think we could just set local_preload_libraries to PGC_USERSET and
> document that subsequent changes won't take effect. That's the same way
> session_preload_libraries works. That would avoid inventing another
> very specialized GUC context.

It is enough for me. Since the only advantage of
PGC_BACKEND_USERSET is the capability to inhibit in-session
modification and I don't see another use case for it, I have no
objection for your opinion.

> If you're interested in improving this area, I also suggest you read the
> thread of
> <http://www.postgresql.org/message-id/1349829917.29682.5.camel@vanquo.pezone.net>.

Although I don't understand even after reading this why
local_preload_libraries was PGC_SUSET, there seems to be no
reason it should be so.

The attached patch simply changes the context for local_... to
PGC_USERSET and edits the doc.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

On 9/1/14 7:51 AM, Kyotaro HORIGUCHI wrote:
> The attached patch simply changes the context for local_... to
> PGC_USERSET and edits the doc.

I had this ready to commit, but then

Invent PGC_SU_BACKEND and mark log_connections/log_disconnections
that way.

was committed in the meantime.

Does this affect what we should do with this change?

I guess one thing to look into would be whether we could leave
local_preload_libraries as PGC_BACKEND and change
session_preload_libraries to PGC_SU_BACKEND, and then investigate
whether we could allow settings made with ALTER ROLE / SET to change
PGC_BACKEND settings.

In the meantime, I have committed documentation fixes for the back
branches 9.0 .. 9.3.

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> On 9/1/14 7:51 AM, Kyotaro HORIGUCHI wrote:
>> The attached patch simply changes the context for local_... to
>> PGC_USERSET and edits the doc.

> I had this ready to commit, but then

> Invent PGC_SU_BACKEND and mark log_connections/log_disconnections
> that way.

> was committed in the meantime.

> Does this affect what we should do with this change?

> I guess one thing to look into would be whether we could leave
> local_preload_libraries as PGC_BACKEND and change
> session_preload_libraries to PGC_SU_BACKEND, and then investigate
> whether we could allow settings made with ALTER ROLE / SET to change
> PGC_BACKEND settings.

Yeah, I was wondering about that while I was making the other commit.
I did not touch those variables at the time, but it would make sense
to restrict them as you suggest.

regards, tom lane

On Mon, Sep 15, 2014 at 1:33 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
>> On 9/1/14 7:51 AM, Kyotaro HORIGUCHI wrote:
>>> The attached patch simply changes the context for local_... to
>>> PGC_USERSET and edits the doc.
>
>> I had this ready to commit, but then
>
>> Invent PGC_SU_BACKEND and mark log_connections/log_disconnections
>> that way.
>
>> was committed in the meantime.
>
>> Does this affect what we should do with this change?
>
>> I guess one thing to look into would be whether we could leave
>> local_preload_libraries as PGC_BACKEND and change
>> session_preload_libraries to PGC_SU_BACKEND, and then investigate
>> whether we could allow settings made with ALTER ROLE / SET to change
>> PGC_BACKEND settings.
>
> Yeah, I was wondering about that while I was making the other commit.
> I did not touch those variables at the time, but it would make sense
> to restrict them as you suggest.

+1

Also I think that it's useful to allow ALTER ROLE/DATABASE SET to
set PGC_BACKEND and PGC_SU_BACKEND parameters. So, what
about applying the attached patch? This patch allows that and
changes the context of session_preload_libraries to PGC_SU_BACKEND.

Regards,

--
Fujii Masao

Hello, I overlooked this thread.

> On Mon, Sep 15, 2014 at 1:33 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> >> On 9/1/14 7:51 AM, Kyotaro HORIGUCHI wrote:
> >>> The attached patch simply changes the context for local_... to
> >>> PGC_USERSET and edits the doc.
> >
> >> I had this ready to commit, but then
> >
> >> Invent PGC_SU_BACKEND and mark log_connections/log_disconnections
> >> that way.
> >
> >> was committed in the meantime.
> >
> >> Does this affect what we should do with this change?
> >
> >> I guess one thing to look into would be whether we could leave
> >> local_preload_libraries as PGC_BACKEND and change
> >> session_preload_libraries to PGC_SU_BACKEND, and then investigate
> >> whether we could allow settings made with ALTER ROLE / SET to change
> >> PGC_BACKEND settings.
> >
> > Yeah, I was wondering about that while I was making the other commit.
> > I did not touch those variables at the time, but it would make sense
> > to restrict them as you suggest.
>
> +1
>
> Also I think that it's useful to allow ALTER ROLE/DATABASE SET to
> set PGC_BACKEND and PGC_SU_BACKEND parameters. So, what
> about applying the attached patch? This patch allows that and
> changes the context of session_preload_libraries to PGC_SU_BACKEND.

It's not my business to decide to aplly it but I don't see
obvious problmen in it so far.

By the way, I became unable to login at all after wrongly setting
*_preload_libraries for all available users. Is there any releaf
measures for the situation? I think it's okay even if there's no
way to login again but want to know if any.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Fri, Oct 10, 2014 at 5:36 PM, Kyotaro HORIGUCHI
<horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> Hello, I overlooked this thread.
>
>> On Mon, Sep 15, 2014 at 1:33 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> > Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
>> >> On 9/1/14 7:51 AM, Kyotaro HORIGUCHI wrote:
>> >>> The attached patch simply changes the context for local_... to
>> >>> PGC_USERSET and edits the doc.
>> >
>> >> I had this ready to commit, but then
>> >
>> >> Invent PGC_SU_BACKEND and mark log_connections/log_disconnections
>> >> that way.
>> >
>> >> was committed in the meantime.
>> >
>> >> Does this affect what we should do with this change?
>> >
>> >> I guess one thing to look into would be whether we could leave
>> >> local_preload_libraries as PGC_BACKEND and change
>> >> session_preload_libraries to PGC_SU_BACKEND, and then investigate
>> >> whether we could allow settings made with ALTER ROLE / SET to change
>> >> PGC_BACKEND settings.
>> >
>> > Yeah, I was wondering about that while I was making the other commit.
>> > I did not touch those variables at the time, but it would make sense
>> > to restrict them as you suggest.
>>
>> +1
>>
>> Also I think that it's useful to allow ALTER ROLE/DATABASE SET to
>> set PGC_BACKEND and PGC_SU_BACKEND parameters. So, what
>> about applying the attached patch? This patch allows that and
>> changes the context of session_preload_libraries to PGC_SU_BACKEND.
>
> It's not my business to decide to aplly it but I don't see
> obvious problmen in it so far.
>
> By the way, I became unable to login at all after wrongly setting
> *_preload_libraries for all available users. Is there any releaf
> measures for the situation? I think it's okay even if there's no
> way to login again but want to know if any.

Yep, that's a problem. You can login to the server even in that case
by, for example, executing the following commands, though.

$ export PGOPTIONS="-c *_preload_libraries="
$ psql

Regards,

--
Fujii Masao

Wow.

> > By the way, I became unable to login at all after wrongly setting
> > *_preload_libraries for all available users. Is there any releaf
> > measures for the situation? I think it's okay even if there's no
> > way to login again but want to know if any.
>
> Yep, that's a problem. You can login to the server even in that case
> by, for example, executing the following commands, though.
>
> $ export PGOPTIONS="-c *_preload_libraries="
> $ psql

Thank you. I see. It seems enough for me. The section 18.1.3 of
9.4 document describes about it,

http://www.postgresql.org/docs/9.4/static/config-setting.html

> 18.1.4. Parameter Interaction via Shell
..
> On the libpq-client, command-line options can be specified
> using the PGOPTIONS environment variable. When connecting to
> the server, the contents of this variable are sent to the
> server as if they were being executed via SQL SET at the
> beginning of the session.

This implicitly denies PGC_BACKEND variables to be set by this
method but it also seems not proper to describe precisely...

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Tue, Oct 21, 2014 at 3:16 PM, Kyotaro HORIGUCHI
<horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> Wow.
>
>> > By the way, I became unable to login at all after wrongly setting
>> > *_preload_libraries for all available users. Is there any releaf
>> > measures for the situation? I think it's okay even if there's no
>> > way to login again but want to know if any.
>>
>> Yep, that's a problem. You can login to the server even in that case
>> by, for example, executing the following commands, though.
>>
>> $ export PGOPTIONS="-c *_preload_libraries="
>> $ psql
>
> Thank you. I see. It seems enough for me. The section 18.1.3 of
> 9.4 document describes about it,
>
> http://www.postgresql.org/docs/9.4/static/config-setting.html
>
>> 18.1.4. Parameter Interaction via Shell
> ..
>> On the libpq-client, command-line options can be specified
>> using the PGOPTIONS environment variable. When connecting to
>> the server, the contents of this variable are sent to the
>> server as if they were being executed via SQL SET at the
>> beginning of the session.
>
> This implicitly denies PGC_BACKEND variables to be set by this
> method but it also seems not proper to describe precisely...

Sorry, probably I failed to understand your point. Could you tell me
what you're suggesting? You're thinking that the description needs to
be updated? How?

Regards,

--
Fujii Masao

On 10/9/14 1:58 PM, Fujii Masao wrote:
> Also I think that it's useful to allow ALTER ROLE/DATABASE SET to
> set PGC_BACKEND and PGC_SU_BACKEND parameters. So, what
> about applying the attached patch? This patch allows that and
> changes the context of session_preload_libraries to PGC_SU_BACKEND.

After looking through this again, I wonder whether there is any reason
why ignore_system_indexes cannot be plain PGC_USERSET. With this
change, we'd allow setting it via ALTER ROLE, but the access to
pg_db_role_setting happens before it. So if there is anything unsafe
about changing ignore_system_indexes, then this would be a problem, but
I don't see anything.

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> On 10/9/14 1:58 PM, Fujii Masao wrote:
>> Also I think that it's useful to allow ALTER ROLE/DATABASE SET to
>> set PGC_BACKEND and PGC_SU_BACKEND parameters. So, what
>> about applying the attached patch? This patch allows that and
>> changes the context of session_preload_libraries to PGC_SU_BACKEND.

> After looking through this again, I wonder whether there is any reason
> why ignore_system_indexes cannot be plain PGC_USERSET. With this
> change, we'd allow setting it via ALTER ROLE, but the access to
> pg_db_role_setting happens before it. So if there is anything unsafe
> about changing ignore_system_indexes, then this would be a problem, but
> I don't see anything.

There are some, um, "interesting" consequences of setting
ignore_system_indexes; AFAIK, none that put data integrity at risk, but it
can destroy performance in ways beyond the obvious ones. See for example
the comments for get_mergejoin_opfamilies and get_ordering_op_properties.
I don't particularly want to answer user bug reports about such behaviors,
nor do I care to put any effort into making the behavior without system
indexes smarter than it is now. (We should also consider the risk that
there might be as-yet-unrecognized dependencies on catalog scan order that
would amount to actual bugs.)

So I'm -1 on any change that might make it look like we were encouraging
people to use ignore_system_indexes except as a very last resort.

regards, tom lane

On Wed, Nov 5, 2014 at 1:22 AM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> On 10/9/14 1:58 PM, Fujii Masao wrote:
>> Also I think that it's useful to allow ALTER ROLE/DATABASE SET to
>> set PGC_BACKEND and PGC_SU_BACKEND parameters. So, what
>> about applying the attached patch? This patch allows that and
>> changes the context of session_preload_libraries to PGC_SU_BACKEND.
>
> After looking through this again, I wonder whether there is any reason
> why ignore_system_indexes cannot be plain PGC_USERSET. With this
> change, we'd allow setting it via ALTER ROLE, but the access to
> pg_db_role_setting happens before it.

Even without the patch, we can set ignore_system_indexes
at the startup of the connection because it's defined with
PGC_BACKEND context, but the access to system tables
can happen before that. Am I missing something?

Regards,

--
Fujii Masao

On 11/12/14 1:01 PM, Fujii Masao wrote:
> On Wed, Nov 5, 2014 at 1:22 AM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
>> On 10/9/14 1:58 PM, Fujii Masao wrote:
>>> Also I think that it's useful to allow ALTER ROLE/DATABASE SET to
>>> set PGC_BACKEND and PGC_SU_BACKEND parameters. So, what
>>> about applying the attached patch? This patch allows that and
>>> changes the context of session_preload_libraries to PGC_SU_BACKEND.
>>
>> After looking through this again, I wonder whether there is any reason
>> why ignore_system_indexes cannot be plain PGC_USERSET. With this
>> change, we'd allow setting it via ALTER ROLE, but the access to
>> pg_db_role_setting happens before it.
>
> Even without the patch, we can set ignore_system_indexes
> at the startup of the connection because it's defined with
> PGC_BACKEND context, but the access to system tables
> can happen before that. Am I missing something?

Let's think about what would happen if we allowed PGC_BACKEND settings
to be changed by ALTER ROLE.

Here is the current set of PGC_BACKEND and PGC_SU_BACKEND settings:

- ignore_system_indexes

Reason: "interesting" consequences if changed later

- post_auth_delay

Reason: changing later would have no effect

- local_preload_libraries

Reason: changing later would have no effect

- log_connections

Reason: changing later would have no effect

- log_disconnections

Reason: dubious / consistency with log_connections?

Only ignore_system_indexes is really in the spirit of the original
PGC_BACKEND setting, namely for settings that unprivileged users can set
at the beginning of a session but should not change later. We used to
have "fsync" in that category, for example, because changing that was
not considered safe at some time. We used to have a lot more of these,
but not many stood the test of time.

The other settings are really just things that take effect during
session start but don't hurt when changed later. The problem is that
the order of these relative to ALTER ROLE processing is really an
implementation detail or a best effort type of thing. For example, it
looks as though we are making an effort to process post_auth_delay
really late, after ALTER ROLE processing (even though we currently don't
allow it to be set that way; strange), but there is no reason why that
has to be so. One could reasonably think that "post auth" is really
early, before catalog access starts. On the other hand, log_connections
is processed really early, so ALTER ROLE would have no effect.

This is going to end up inconsistent one way or the other.

My radical proposal therefore would have been to embrace this
inconsistency and get rid of PGC_BACKEND and PGC_SU_BACKEND altogether,
relying on users interpreting the parameter names to indicate that
changing them later may or may not have an effect. Unfortunately, the
concerns about ignore_system_indexes prevent that.

We could think of another way to address those concerns, e.g., with an
ad hoc way in an assign hook.

The other proposal would be to keep PGC_BACKEND and PGC_SU_BACKEND as
special-case warts, perhaps document them as such, but change everything
to use something else as much as possible, namely

post_auth_delay -> PGC_USERSET
local_preload_libraries -> PGC_USERSET
log_disconnections -> PGC_SUSET

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> My radical proposal therefore would have been to embrace this
> inconsistency and get rid of PGC_BACKEND and PGC_SU_BACKEND altogether,
> relying on users interpreting the parameter names to indicate that
> changing them later may or may not have an effect. Unfortunately, the
> concerns about ignore_system_indexes prevent that.

Yeah, I think making ignore_system_indexes USERSET would be a pretty bad
idea. People would expect that they can frob it back and forth with no
impact other than performance, and I'm doubtful that that's true.

If we wanted to make a push to get rid of PGC_BACKEND, I could see
changing ignore_system_indexes to SUSET category, and assuming that
superusers are adults who won't push a button just to see what it does.

But having said that, I don't really think that PGC_BACKEND is a useless
category. It provides a uniform way of documenting that changing a
particular setting post-session-start is useless. Therefore I'm not
on board with getting rid of it. To the extent that we can make ALTER
ROLE/DATABASE control these settings, that would be a good thing.

regards, tom lane

On Sun, Dec 7, 2014 at 9:54 AM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> My radical proposal therefore would have been to embrace this
> inconsistency and get rid of PGC_BACKEND and PGC_SU_BACKEND altogether,
> relying on users interpreting the parameter names to indicate that
> changing them later may or may not have an effect. Unfortunately, the
> concerns about ignore_system_indexes prevent that.

What exactly are those concerns? Do you have a link to previous discussion?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 12/8/14 12:39 PM, Robert Haas wrote:
> On Sun, Dec 7, 2014 at 9:54 AM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
>> My radical proposal therefore would have been to embrace this
>> inconsistency and get rid of PGC_BACKEND and PGC_SU_BACKEND altogether,
>> relying on users interpreting the parameter names to indicate that
>> changing them later may or may not have an effect. Unfortunately, the
>> concerns about ignore_system_indexes prevent that.
>
> What exactly are those concerns? Do you have a link to previous discussion?

Earlier in the thread:
http://www.postgresql.org/message-id/20108.1415120322@sss.pgh.pa.us

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> On 12/8/14 12:39 PM, Robert Haas wrote:
>> On Sun, Dec 7, 2014 at 9:54 AM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
>>> My radical proposal therefore would have been to embrace this
>>> inconsistency and get rid of PGC_BACKEND and PGC_SU_BACKEND altogether,
>>> relying on users interpreting the parameter names to indicate that
>>> changing them later may or may not have an effect. Unfortunately, the
>>> concerns about ignore_system_indexes prevent that.

>> What exactly are those concerns? Do you have a link to previous discussion?

> Earlier in the thread:
> http://www.postgresql.org/message-id/20108.1415120322@sss.pgh.pa.us

The core of the mentioned issues is that catalog searches done via the
systable_beginscan/systable_getnext API will ordinarily visit catalog
entries in the order of the specified index. However, if
ignore_system_indexes is set, you get a seqscan that will return the same
tuples in heap order (effectively, random order). There are known cases
where this results in minor planner inefficiencies, and I'm worried that
there might be outright bugs we don't know about, since that whole
operating mode can be best be described as entirely untested outside of
the bootstrap sequence.

Barring someone committing to spend the time to improve that situation
(time that would be poorly invested IMO), I don't think that we want to
open up ignore_system_indexes as USERSET, or do anything else to encourage
its use.

If we're intent on removing PGC_BACKEND then I'd be okay with
reclassifying ignore_system_indexes as SUSET; but I'm not exactly
convinced that we should be trying to get rid of PGC_BACKEND.

regards, tom lane

On Mon, Dec 8, 2014 at 9:18 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Barring someone committing to spend the time to improve that situation
> (time that would be poorly invested IMO), I don't think that we want to
> open up ignore_system_indexes as USERSET, or do anything else to encourage
> its use.
>
> If we're intent on removing PGC_BACKEND then I'd be okay with
> reclassifying ignore_system_indexes as SUSET; but I'm not exactly
> convinced that we should be trying to get rid of PGC_BACKEND.

Well, if you want to discourage its use, I think there's an argument
that marking it as SUSET would be more restrictive than what we have
at present, since it would altogether prohibit non-superuser use.

I'm not wedded to the idea of getting rid of PGC_BACKEND, but I do
like it. Peter's survey of the landscape seems to show that there's
very little left in that category and the stuff that is there is
somewhat uninspiring. And simplifying things is always nice.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Dec 9, 2014 at 11:56 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Mon, Dec 8, 2014 at 9:18 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Barring someone committing to spend the time to improve that situation
>> (time that would be poorly invested IMO), I don't think that we want to
>> open up ignore_system_indexes as USERSET, or do anything else to encourage
>> its use.
>>
>> If we're intent on removing PGC_BACKEND then I'd be okay with
>> reclassifying ignore_system_indexes as SUSET; but I'm not exactly
>> convinced that we should be trying to get rid of PGC_BACKEND.
>
> Well, if you want to discourage its use, I think there's an argument
> that marking it as SUSET would be more restrictive than what we have
> at present, since it would altogether prohibit non-superuser use.
>
> I'm not wedded to the idea of getting rid of PGC_BACKEND, but I do
> like it. Peter's survey of the landscape seems to show that there's
> very little left in that category and the stuff that is there is
> somewhat uninspiring. And simplifying things is always nice.

Documentation fixes for the use of local_preload_libraries have been
pushed, now there has been some wider discussion about changing the
mode of a couple of parameters since PGC_SU_BACKEND has been
introduced. Any problems to switch this patch to "Returned with
feedback"? The discussion done here is wider than the simple use of
local_preload_libraries in any case.
--
Michael

On 8/29/14 4:01 PM, Peter Eisentraut wrote:
> On 8/28/14 9:01 AM, Kyotaro HORIGUCHI wrote:
>> I found this issue when trying per-pg_user (role) loading of
>> auto_analyze and some tweaking tool. It is not necessarily set by
>> the user by own, but the function to decide whether to load some
>> module by the session-user would be usable, at least, as for me:)
>
> I think we could just set local_preload_libraries to PGC_USERSET and
> document that subsequent changes won't take effect. That's the same way
> session_preload_libraries works.

Committed this way.

This doesn't prevent future fine-tuning in this area, but in the
subsequent discussion, there was no clear enthusiasm for any particular
direction.

Hello, sorry for the absense,

Thank you for committing.

> On 8/29/14 4:01 PM, Peter Eisentraut wrote:
> > On 8/28/14 9:01 AM, Kyotaro HORIGUCHI wrote:
> >> I found this issue when trying per-pg_user (role) loading of
> >> auto_analyze and some tweaking tool. It is not necessarily set by
> >> the user by own, but the function to decide whether to load some
> >> module by the session-user would be usable, at least, as for me:)
> >
> > I think we could just set local_preload_libraries to PGC_USERSET and
> > document that subsequent changes won't take effect. That's the same way
> > session_preload_libraries works.
>
> Committed this way.
>
> This doesn't prevent future fine-tuning in this area, but in the
> subsequent discussion, there was no clear enthusiasm for any particular
> direction.

--
Kyotaro Horiguchi
NTT Open Source Software Center