Thread: custom variables and PGC_SUSET issue
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
Attachment
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
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
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
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
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
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
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
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
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
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
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