Thread

custom variables and PGC_SUSET issue

Pavel Stehule <pavel.stehule@gmail.com> — 2011-09-07T17:23:43Z

Hello

Andy Colson found a bug in GUC implementation.

When we have a custom SUSET variable, like plpgsql.variable_conflikt,
then setting this variable before plpgsql initalisation raises a
exception, but it raise a exception when some plpgsql function is
created. Try to execute a attached script - a set statement is ok, but
CREATE FUNCTION fails.

repeated setting this GUC raise a strange message

postgres=# \i script.sql
SET
before create function
psql:script.sql:13: ERROR:  42501: permission denied to set parameter
"plpgsql.variable_conflict"
LOCATION:  set_config_option, guc.c:5208
after function
postgres=# \i script.sql
SET
before create function
psql:script.sql:13: ERROR:  XX000: attempt to redefine parameter
"plpgsql.variable_conflict"
LOCATION:  define_custom_variable, guc.c:6333
after function

Regards

Pavel Stehule

Re: custom variables and PGC_SUSET issue

Alvaro Herrera <alvherre@commandprompt.com> — 2011-09-07T17:34:55Z

Excerpts from Pavel Stehule's message of mié sep 07 14:23:43 -0300 2011:
> Hello
> 
> Andy Colson found a bug in GUC implementation.
> 
> When we have a custom SUSET variable, like plpgsql.variable_conflikt,
> then setting this variable before plpgsql initalisation raises a
> exception, but it raise a exception when some plpgsql function is
> created. Try to execute a attached script - a set statement is ok, but
> CREATE FUNCTION fails.

Another thing we detected some days ago is that a custom variable in a
module not loaded by postmaster, does not seem to get reported
appropriately when an invalid value is set on postgresql.conf and
reloaded: backends report problems with DEBUG3, only postmaster uses
LOG, but since postmaster isn't loading the module, nothing happens.

(Maybe this is our bug but it doesn't seem like it.)

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Re: custom variables and PGC_SUSET issue

Tom Lane <tgl@sss.pgh.pa.us> — 2011-09-07T17:49:45Z

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Another thing we detected some days ago is that a custom variable in a
> module not loaded by postmaster, does not seem to get reported
> appropriately when an invalid value is set on postgresql.conf and
> reloaded: backends report problems with DEBUG3, only postmaster uses
> LOG, but since postmaster isn't loading the module, nothing happens.

This is just an instance of the general problem that the current design
assumes all processes will have the same opinion about the validity of
settings read from postgresql.conf.  We discussed that back in July:
http://archives.postgresql.org/pgsql-hackers/2011-07/msg00850.php
but it wasn't clear to me that any consensus had been reached about how
to change the behavior.  I proposed that we should let processes adopt
individual settings even if they thought other ones were invalid; that
gets rid of some of the issues, but it doesn't really address how we
should report problems when only some of the processes think there's a
problem.  I don't think we can just s/DEBUG3/LOG/, because of the
log clutter that will result when they all think there's a problem.

			regards, tom lane

Re: custom variables and PGC_SUSET issue

Tom Lane <tgl@sss.pgh.pa.us> — 2011-09-07T17:55:11Z

Pavel Stehule <pavel.stehule@gmail.com> writes:
> When we have a custom SUSET variable, like plpgsql.variable_conflikt,
> then setting this variable before plpgsql initalisation raises a
> exception, but it raise a exception when some plpgsql function is
> created. Try to execute a attached script - a set statement is ok, but
> CREATE FUNCTION fails.

You can't set a custom SUSET variable in advance of loading the module,
because the system has no way to know it should enforce superuser
restrictions on setting it.  For the most part, this is a good reason to
avoid custom SUSET variables.

			regards, tom lane

Re: custom variables and PGC_SUSET issue

Robert Haas <robertmhaas@gmail.com> — 2011-09-07T17:59:49Z

On Wed, Sep 7, 2011 at 1:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Pavel Stehule <pavel.stehule@gmail.com> writes:
>> When we have a custom SUSET variable, like plpgsql.variable_conflikt,
>> then setting this variable before plpgsql initalisation raises a
>> exception, but it raise a exception when some plpgsql function is
>> created. Try to execute a attached script - a set statement is ok, but
>> CREATE FUNCTION fails.
>
> You can't set a custom SUSET variable in advance of loading the module,
> because the system has no way to know it should enforce superuser
> restrictions on setting it.  For the most part, this is a good reason to
> avoid custom SUSET variables.

Apparently we haven't taken that advice ourselves?

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

Re: custom variables and PGC_SUSET issue

Tom Lane <tgl@sss.pgh.pa.us> — 2011-09-07T18:05:32Z

Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Sep 7, 2011 at 1:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> You can't set a custom SUSET variable in advance of loading the module,
>> because the system has no way to know it should enforce superuser
>> restrictions on setting it. For the most part, this is a good reason to
>> avoid custom SUSET variables.

> Apparently we haven't taken that advice ourselves?

The ones in auto_explain and pg_stat_statements aren't too problematic
because those modules are designed to be preloaded by the postmaster.
We've avoided adding such variables in modules that aren't intended
to be used that way.

			regards, tom lane

Re: custom variables and PGC_SUSET issue

Robert Haas <robertmhaas@gmail.com> — 2011-09-07T18:07:25Z

On Wed, Sep 7, 2011 at 2:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Wed, Sep 7, 2011 at 1:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> You can't set a custom SUSET variable in advance of loading the module,
>>> because the system has no way to know it should enforce superuser
>>> restrictions on setting it. For the most part, this is a good reason to
>>> avoid custom SUSET variables.
>
>> Apparently we haven't taken that advice ourselves?
>
> The ones in auto_explain and pg_stat_statements aren't too problematic
> because those modules are designed to be preloaded by the postmaster.
> We've avoided adding such variables in modules that aren't intended
> to be used that way.

What about plpgsql.variable_conflict?

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

Re: custom variables and PGC_SUSET issue

Tom Lane <tgl@sss.pgh.pa.us> — 2011-09-07T18:21:16Z

Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Sep 7, 2011 at 2:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The ones in auto_explain and pg_stat_statements aren't too problematic
>> because those modules are designed to be preloaded by the postmaster.
>> We've avoided adding such variables in modules that aren't intended
>> to be used that way.

> What about plpgsql.variable_conflict?

That one's not really meant to be changed on the fly either; we have
#variable_conflict instead.

The reason why this is hard, and not just a bug to be fixed, is that
it's not clear what to do.  Part of the issue is that we don't remember
whether the current setting was applied by a superuser or not, but even
if we did remember that, what happens at LOAD time if it wasn't?
Silently replacing the value isn't appealing, and neither are the other
options.  It's better to not have such a variable in the first place.

			regards, tom lane

Re: custom variables and PGC_SUSET issue

Alvaro Herrera <alvherre@commandprompt.com> — 2011-09-07T19:07:55Z

Excerpts from Tom Lane's message of mié sep 07 14:49:45 -0300 2011:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
> > Another thing we detected some days ago is that a custom variable in a
> > module not loaded by postmaster, does not seem to get reported
> > appropriately when an invalid value is set on postgresql.conf and
> > reloaded: backends report problems with DEBUG3, only postmaster uses
> > LOG, but since postmaster isn't loading the module, nothing happens.
> 
> This is just an instance of the general problem that the current design
> assumes all processes will have the same opinion about the validity of
> settings read from postgresql.conf.  We discussed that back in July:
> http://archives.postgresql.org/pgsql-hackers/2011-07/msg00850.php
> but it wasn't clear to me that any consensus had been reached about how
> to change the behavior.  I proposed that we should let processes adopt
> individual settings even if they thought other ones were invalid; that
> gets rid of some of the issues, but it doesn't really address how we
> should report problems when only some of the processes think there's a
> problem.

Yeah; in this particular case, the value is just plain invalid for
everybody.  I think it just happens that postmaster didn't see it
because it was valid when it was started (i.e. the file got edited and a
SIGHUP sent, introducing the invalid value but not adopted anywhere).

> I don't think we can just s/DEBUG3/LOG/, because of the
> log clutter that will result when they all think there's a problem.

I was thinking that the solution would be to promote DEBUG3 to LOG in
the case of a custom variable.  This would be noisy as you say, but I
don't see any other option; at least it would just be the custom
variables.  This didn't work for reasons that we haven't been
investigated yet (we discovered this late last week and I've been busy
with 9.1 translations).

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Re: custom variables and PGC_SUSET issue

Robert Haas <robertmhaas@gmail.com> — 2011-09-07T19:18:00Z

On Wed, Sep 7, 2011 at 2:21 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Wed, Sep 7, 2011 at 2:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> The ones in auto_explain and pg_stat_statements aren't too problematic
>>> because those modules are designed to be preloaded by the postmaster.
>>> We've avoided adding such variables in modules that aren't intended
>>> to be used that way.
>
>> What about plpgsql.variable_conflict?
>
> That one's not really meant to be changed on the fly either; we have
> #variable_conflict instead.
>
> The reason why this is hard, and not just a bug to be fixed, is that
> it's not clear what to do.  Part of the issue is that we don't remember
> whether the current setting was applied by a superuser or not, but even
> if we did remember that, what happens at LOAD time if it wasn't?
> Silently replacing the value isn't appealing, and neither are the other
> options.  It's better to not have such a variable in the first place.

Yeah, I guess we don't have many good short-term options.  I continue
to think that this whole facility is mis-designed, though.

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

Re: custom variables and PGC_SUSET issue

Andrew Dunstan <andrew@dunslane.net> — 2011-09-07T19:23:25Z


On 09/07/2011 03:18 PM, Robert Haas wrote:
> Yeah, I guess we don't have many good short-term options.  I continue
> to think that this whole facility is mis-designed, though.

I agree. I have tripped over it several times.

cheers

andrew

Re: custom variables and PGC_SUSET issue

Tom Lane <tgl@sss.pgh.pa.us> — 2011-09-07T19:26:20Z

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Excerpts from Tom Lane's message of mi sep 07 14:49:45 -0300 2011:
>> I don't think we can just s/DEBUG3/LOG/, because of the
>> log clutter that will result when they all think there's a problem.

> I was thinking that the solution would be to promote DEBUG3 to LOG in
> the case of a custom variable.  This would be noisy as you say, but I
> don't see any other option; at least it would just be the custom
> variables.

That's not much of a fix because (a) the behavior is still pretty
undesirable, and (b) it supposes that this sort of failure can only
occur for custom variables.  The previous discussion arose from a
different case entirely --- IIRC, it was from trying to specify
client_encoding in postgresql.conf, which the postmaster was happy with
but some backends were not because they had a database_encoding for
which there was no conversion.  There are probably a bunch of other
possibilities out there that we haven't hit yet, and if not today, there
certainly will be more in the future.  So I'm not very excited by a
proposed fix that makes assumptions about the exact reason why a process
rejects a particular GUC value.

			regards, tom lane