Thread: 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 ALTERROLE| 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 diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index cc6dcaf..a88c58c 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -5672,17 +5672,6 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir' </para> <para> - Unlike <xref linkend="guc-shared-preload-libraries">, there is no - performance advantage to loading a library at session - start rather than when it is first used. Rather, the intent of - this feature is to allow debugging or performance-measurement - libraries to be loaded into specific sessions without an explicit - <command>LOAD</> command being given. For example, debugging could - be enabled for all sessions under a given user name by setting - this parameter with <command>ALTER ROLE SET</>. - </para> - - <para> If a specified library is not found, the connection attempt will fail. </para>
Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.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@sss.pgh.pa.us> wrote: > Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.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.fujii@gmail.com> writes:
> On Wed, Jul 2, 2014 at 11:15 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.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@sss.pgh.pa.us> wrote: > Fujii Masao <masao.fujii@gmail.com> writes: >> On Wed, Jul 2, 2014 at 11:15 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.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.fujii@gmail.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@mail.gmail.com><20408.1404329822@sss.pgh.pa.us>
User-Agent: Mew version 6.5 on Emacs 22.2 / Mule 5.0(榊) / 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@sss.pgh.pa.us> wrote in <20408.1404329822@sss.pgh.pa.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
retrievedfrom 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
sessionstarting". 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.kyotaro@lab.ntt.co.jp> wrote:
> o <CAHGQGwFYiANahR_u_cHnz-zOurj3yQMMnJrr9RwP7vPsVXtKUw@mail.gmail.com>
> <20408.1404329822@sss.pgh.pa.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@sss.pgh.pa.us> wrote in <20408.1404329822@sss.pgh.pa.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@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
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 diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 49547ee..8803709 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -6052,14 +6052,16 @@ SET XML OPTION { DOCUMENT | CONTENT }; <listitem> <para> This variable specifiesone or more shared libraries that are to be - preloaded at connection start. This parameter cannot be changed after - the start of a particular session. If a specified library is not + preloaded at connection start. This option is effective only when it + is set at session start via <command>ALTER USER ... SET</> command (or + postgresq.conf) so changing this variable after the start of a + particular session has no effect. If a specified library is not found, the connection attempt will fail. </para> <para> - This option can be set by any user. Because of that, the libraries - that can be loaded are restricted to those appearing in the + Since non-supersers are allowed to set it, the libraries that can be + loaded are restricted to those appearing in the <filename>plugins</> subdirectory of the installation's standard library directory. (It is the database administrator's responsibility to ensure that only <quote>safe</>libraries diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index a8a17c2..f128f32 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -2895,7 +2895,7 @@ static struct config_string ConfigureNamesString[] = }, { - {"local_preload_libraries", PGC_BACKEND, CLIENT_CONN_PRELOAD, + {"local_preload_libraries", PGC_USERSET, CLIENT_CONN_PRELOAD, gettext_noop("Lists unprivileged sharedlibraries to preload into each backend."), NULL, GUC_LIST_INPUT | GUC_LIST_QUOTE
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@gmx.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@sss.pgh.pa.us> wrote: > Peter Eisentraut <peter_e@gmx.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
Attachment
Hello, I overlooked this thread. > On Mon, Sep 15, 2014 at 1:33 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Peter Eisentraut <peter_e@gmx.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.kyotaro@lab.ntt.co.jp> wrote: > Hello, I overlooked this thread. > >> On Mon, Sep 15, 2014 at 1:33 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> > Peter Eisentraut <peter_e@gmx.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.kyotaro@lab.ntt.co.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@gmx.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@gmx.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@gmx.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@gmx.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@gmx.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@gmx.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@gmx.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@gmx.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@sss.pgh.pa.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@gmail.com> wrote: > On Mon, Dec 8, 2014 at 9:18 PM, Tom Lane <tgl@sss.pgh.pa.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