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.

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/



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to