Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review] - Mailing list pgsql-hackers

From Boszormenyi Zoltan
Subject Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
Date
Msg-id 5142E7EF.9090306@cybertec.at
Whole thread Raw
In response to Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]  (Boszormenyi Zoltan <zb@cybertec.at>)
List pgsql-hackers
<div class="moz-cite-prefix">2013-03-15 00:48 keltezéssel, Boszormenyi Zoltan írta:<br /></div><blockquote
cite="mid:514261B4.6080104@cybertec.at"type="cite">2013-03-13 21:28 keltezéssel, Boszormenyi Zoltan írta: <br
/><blockquotetype="cite">2013-03-13 13:45 keltezéssel, Andres Freund írta: <br /><blockquote type="cite">On 2013-03-13
09:09:24+0100, Boszormenyi Zoltan wrote: <br /><blockquote type="cite">2013-03-13 07:42 keltezéssel, Craig Ringer írta:
<br/><blockquote type="cite">On 03/12/2013 06:27 AM, Craig Ringer wrote: <br /><blockquote type="cite"><blockquote
type="cite"><blockquotetype="cite">Think also about the case where someone wants to change multiple <br /> values
togetherand having just some set and not others would be <br /> inconsistent. <br /></blockquote></blockquote> Yeah,
that'sa killer. The reload would need to be scheduled for COMMIT <br /> time, it can't be done by `SET PERSISTENT`
directly.<br /></blockquote> Thinking about this some more, I'm not sure this is a good idea. <br /><br /> Right now,
SETtakes effect immediately. Always, without exception. <br /> Delaying SET PERSISTENT's effects until commit would
makeit <br /> inconsistent with SET's normal behaviour. <br /><br /> However, *not* delaying it would make it another
quirky<br /> not-transactional not-atomic command. That's OK, but if it's not going <br /> to follow transactional
semanticsit should not be allowed to run within <br /> a transaction, like VACUUM . <br /><br /> Writing the changes
immediatelybut deferring the reload until commit <br /> seems to be the worst of those two worlds. <br /></blockquote>
Iwas thinking about it a little. There is a hook that runs at the end <br /> of (sub-)transactions. It can be abused
forthis purpose to make <br /> SET PERSISTENT transactional. The subtransactions can also stack <br /> these settings,
byforgetting settings upon ROLLBACK [ TO SAVEPOINT ] <br /> and overwriting previous settings upon COMMIT and RELEASE
SAVEPOINT.<br /> All it needs is a list and maintaining intermediate pointers when entering <br /> into a new level of
SAVEPOINT.The functions to register such hooks are <br /> in src/include/access/xact.h: <br /><br /> extern void
RegisterXactCallback(XactCallbackcallback, void *arg); <br /> extern void UnregisterXactCallback(XactCallback callback,
void*arg); <br /> extern void RegisterSubXactCallback(SubXactCallback callback, void *arg); <br /> extern void
UnregisterSubXactCallback(SubXactCallbackcallback, void *arg); <br /></blockquote> (sub)xact commit/abort already calls
AtEOXact_GUC(commit,nestlevel). So you <br /> wouldn't even need that. <br /></blockquote><br /> Yes, thanks. <br /><br
/><blockquotetype="cite">  It seems we could add another value to enum <br /> GucStackState, like GUC_SET_PERSISTENT -
andprocess those only if commit && <br /> nestlevel == 1. <br /></blockquote><br /> Maybe it's not needed, only
enumGucAction needs a new <br /> GUC_ACTION_PERSISTENT value since that's what has any <br /> business in
push_old_value().Adding two new members to <br /> GucStack like these are enough <br />     bool has_persistent; <br />
   config_var_value persistent; <br /> and SET PERSISTENT can be treated as GUC_SET. push_old_value() <br /> can merge
GUCvalues set in the same transaction level. <br /></blockquote><br /> It seems both were needed. The attached patch
makes<br /> SET PERSISTENT transactional and uses one setting per file. <br /> It uses the currently existing parsing
andvalidating code <br /> and because of this, the patch is half the size of v12 from Amit. <br /></blockquote><br />
Theonly missing piece is the check for superuser.<br /><br /><br /><blockquote cite="mid:514261B4.6080104@cybertec.at"
type="cite"><br/> Best regards, <br /> Zoltán Böszörményi <br /><br /><blockquote type="cite"><br /><blockquote
type="cite">Everytimeyou see one with commit && nestlevel > 1 you put them into them into <br /> the stack
onelevel up. <br /><br /> This seems like its somewhat in line with the way SET LOCAL is implemented? <br /><br
/><blockquotetype="cite">On the other hand, it would make a lot of sense to implement it <br /> as one setting per file
orextending the  code to allow modifying <br /> settings in bulk. The one setting per file should be easier and <br />
itwould also allow extensions to drop their settings automatically <br /> into the automatic config directory. I don't
knowwho mentioned <br /> this idea about extensions but it also came up a while back. <br /></blockquote> I still think
one-setting-per-fileis the right way to go, yes. <br /><br /> Greetings, <br /><br /> Andres Freund <br /><br
/></blockquote><br/><br /></blockquote><br /><br /><br /><fieldset class="mimeAttachmentHeader"></fieldset><br /><pre
wrap="">
</pre></blockquote><br /><br /><pre class="moz-signature" cols="90">-- 
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: <a class="moz-txt-link-freetext" href="http://www.postgresql-support.de">http://www.postgresql-support.de</a>
<aclass="moz-txt-link-freetext" href="http://www.postgresql.at/">http://www.postgresql.at/</a>
 
</pre>

pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: Identity projection
Next
From: Amit Kapila
Date:
Subject: Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review] (Fix memory growth)