Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]

2013-03-15 00:48 keltezéssel, Boszormenyi Zoltan írta:
> 2013-03-13 21:28 keltezéssel, Boszormenyi Zoltan írta:
>> 2013-03-13 13:45 keltezéssel, Andres Freund írta:
>>> On 2013-03-13 09:09:24 +0100, Boszormenyi Zoltan wrote:
>>>> 2013-03-13 07:42 keltezéssel, Craig Ringer írta:
>>>>> On 03/12/2013 06:27 AM, Craig Ringer wrote:
>>>>>>>> Think also about the case where someone wants to change multiple
>>>>>>>> values together and having just some set and not others would be
>>>>>>>> inconsistent.
>>>>>> Yeah, that's a killer. The reload would need to be scheduled for COMMIT
>>>>>> time, it can't be done by `SET PERSISTENT` directly.
>>>>> Thinking about this some more, I'm not sure this is a good idea.
>>>>>
>>>>> Right now, SET takes effect immediately. Always, without exception.
>>>>> Delaying SET PERSISTENT's effects until commit would make it
>>>>> inconsistent with SET's normal behaviour.
>>>>>
>>>>> However, *not* delaying it would make it another quirky
>>>>> not-transactional not-atomic command. That's OK, but if it's not going
>>>>> to follow transactional semantics it should not be allowed to run within
>>>>> a transaction, like VACUUM .
>>>>>
>>>>> Writing the changes immediately but deferring the reload until commit
>>>>> seems to be the worst of those two worlds.
>>>> I was thinking about it a little. There is a hook that runs at the end
>>>> of (sub-)transactions. It can be abused for this purpose to make
>>>> SET PERSISTENT transactional. The subtransactions can also stack
>>>> these settings, by forgetting settings upon ROLLBACK [ TO SAVEPOINT ]
>>>> and overwriting previous settings upon COMMIT and RELEASE SAVEPOINT.
>>>> All it needs is a list and maintaining intermediate pointers when entering
>>>> into a new level of SAVEPOINT. The functions to register such hooks are
>>>> in src/include/access/xact.h:
>>>>
>>>> extern void RegisterXactCallback(XactCallback callback, void *arg);
>>>> extern void UnregisterXactCallback(XactCallback callback, void *arg);
>>>> extern void RegisterSubXactCallback(SubXactCallback callback, void *arg);
>>>> extern void UnregisterSubXactCallback(SubXactCallback callback, void *arg);
>>> (sub)xact commit/abort already calls AtEOXact_GUC(commit, nestlevel). So you
>>> wouldn't even need that.
>>
>> Yes, thanks.
>>
>>> It seems we could add another value to enum
>>> GucStackState, like GUC_SET_PERSISTENT - and process those only if commit &&
>>> nestlevel == 1.
>>
>> Maybe it's not needed, only enum GucAction needs a new
>> GUC_ACTION_PERSISTENT value since that's what has any
>> business in push_old_value(). Adding two new members to
>> GucStack like these are enough
>> bool has_persistent;
>> config_var_value persistent;
>> and SET PERSISTENT can be treated as GUC_SET. push_old_value()
>> can merge GUC values set in the same transaction level.
>
> It seems both were needed. The attached patch makes
> SET PERSISTENT transactional and uses one setting per file.
> It uses the currently existing parsing and validating code
> and because of this, the patch is half the size of v12 from Amit.

The only missing piece is the check for superuser.

>
> Best regards,
> Zoltán Böszörményi
>
>>
>>> Everytime you see one with commit && nestlevel > 1 you put them into them into
>>> the stack one level up.
>>>
>>> This seems like its somewhat in line with the way SET LOCAL is implemented?
>>>
>>>> On the other hand, it would make a lot of sense to implement it
>>>> as one setting per file or extending the code to allow modifying
>>>> settings in bulk. The one setting per file should be easier and
>>>> it would also allow extensions to drop their settings automatically
>>>> into the automatic config directory. I don't know who mentioned
>>>> this idea about extensions but it also came up a while back.
>>> I still think one-setting-per-file is the right way to go, yes.
>>>
>>> Greetings,
>>>
>>> Andres Freund
>>>
>>
>>
>
>
>
>

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/