Thread: Per-function GUC settings: trickier than it looked
So I coded up a patch for this, based on the idea of creating a quasi-subtransaction that affects only GUC while entering/exiting a function that has GUC settings attached. The specified settings are applied as if by SET LOCAL before starting function execution, and then they drop out during "subtransaction" exit. (I'll post the code to pgsql-patches in a moment.) But on reflection I realize that there are some interesting properties to this approach: * if you do "SET LOCAL foo" when you are in a function that has a "SET foo" property, the setting disappears at functionexit. But if you do "SET foo" it persists. This might be OK, but it seems a bit odd. * in fact, if you do "SET LOCAL foo" when you are in a function that has any "SET" property at all, the setting disappearsat function exit, whether foo was one of the variables SET by the function definition or not. We could perhaps get away with defining that as being the behavior, but it doubtless will surprise someone sometime. What *should* these interactions be like, and has anyone got an idea how to implement their suggestion? regards, tom lane
Tom Lane wrote: > So I coded up a patch for this, based on the idea of creating a > quasi-subtransaction that affects only GUC while entering/exiting a > function that has GUC settings attached. The specified settings are > applied as if by SET LOCAL before starting function execution, and then > they drop out during "subtransaction" exit. (I'll post the code to > pgsql-patches in a moment.) > > But on reflection I realize that there are some interesting properties > to this approach: > > * if you do "SET LOCAL foo" when you are in a function that has a > "SET foo" property, the setting disappears at function exit. But if > you do "SET foo" it persists. This might be OK, but it seems a bit odd. That seems OK - the same happens inside a BEGIN/EXCEPTION/END block, no? > * in fact, if you do "SET LOCAL foo" when you are in a function that has > any "SET" property at all, the setting disappears at function exit, > whether foo was one of the variables SET by the function definition or > not. Hm... That is a bit surprising... Maybe all functions should create a such GUC-only substransaction-like thing. That might create problems for inlining - but only if you can actually change GUCs from plsql function, which maybe you cant... > We could perhaps get away with defining that as being the behavior, > but it doubtless will surprise someone sometime. What *should* these > interactions be like, and has anyone got an idea how to implement their > suggestion? What will happen if you have two functions, foo and bar, were the search-path is overridden for foo, and foo calls bar. I guess bar would be executed with foo's overridden searchpath. Thats seems a bit surprising - I'd make more sense to me if bar would use the session's search-path, but that seems hard to do... Especially because bar *should* use foo's searchpath if foo contained an explicit "SET LOCAL search_path" Or maybe I'm just crazy, and the current behavior is fine.... greetings, Florian Pflug
"Florian G. Pflug" <fgp@phlo.org> writes: > Tom Lane wrote: >> We could perhaps get away with defining that as being the behavior, >> but it doubtless will surprise someone sometime. What *should* these >> interactions be like, and has anyone got an idea how to implement their >> suggestion? > What will happen if you have two functions, foo and bar, were the search-path > is overridden for foo, and foo calls bar. I guess bar would be executed with > foo's overridden searchpath. Thats seems a bit surprising - I think it's correct; if bar doesn't SET a search_path then it should use the caller's. I thought a bit more about this and there are at least some cases we can probably agree on without trouble: * If a transaction or subtransaction aborts, all GUC changes made within it disappear, whether they're from per-function GUC attributes or SET commands. This seems clearly correct. So we need only consider cases where no error occurs. * A regular SET (without LOCAL) propagates clear out to the top level and becomes the session setting, if not aborted. Hence it must/will override any per-function settings, either in its own function or callers. So it seems that only SET LOCAL within a function with per-function GUC settings is at issue. I think that there is a pretty strong use-case for saying that if you have a per-function setting of a particular variable foo, then any "SET LOCAL foo" within the function ought to vanish at function end --- for instance a function could want to try a few different search_path settings and automatically revert to the caller's setting on exit. The question is what about SET LOCAL on a variable that *hasn't* been explicitly SET by the function definition. Either approach we take with it could be surprising, but probably having it revert at function end is more surprising... I notice BTW that we have never updated the SET reference page since subtransactions were introduced --- it still says only that SET LOCAL is "local to the current transaction", without a word about subtransactions. So we have a documentation problem anyway. I recall that we had some discussion during the 8.0 dev cycle about whether having SET LOCAL's effects end at the end of the current subtransaction was really a good idea, given that subtransactions aren't the conceptual model the SQL spec defines, but nothing was ever done about changing the implementation. In fact, our current recommendation for implementing secure SECURITY DEFINER functions (use SET LOCAL to change search_path) really depends on that nowhere-documented behavior ... so it's probably too late to consider changing it now. But this would be the time, if we ever are going to reconsider it. Comments? regards, tom lane
On Sun, Sep 02, 2007 at 12:08:00PM -0400, Tom Lane wrote: > I notice BTW that we have never updated the SET reference page since > subtransactions were introduced --- it still says only that SET LOCAL > is "local to the current transaction", without a word about > subtransactions. So we have a documentation problem anyway. I recall > that we had some discussion during the 8.0 dev cycle about whether > having SET LOCAL's effects end at the end of the current subtransaction > was really a good idea, given that subtransactions aren't the conceptual > model the SQL spec defines, but nothing was ever done about changing > the implementation. ISTM that's the real problem; SET LOCAL wasn't fully updated/considered when subtransactions were added. One way to handle this would be to have 3 different behaviors for SET: session-level, transaction-level, and sub-transaction level. If we had that, we could probably make an across-the-board call that all functions operate as if in their own sub-transaction, at least when it comes to SET. Whatever we decide on, least-surprise would dictate that it's the same whether you apply function-specific settings or not. -- Decibel!, aka Jim Nasby decibel@decibel.org EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)
On Mon, 2007-09-03 at 04:09 -0500, Decibel! wrote: > On Sun, Sep 02, 2007 at 12:08:00PM -0400, Tom Lane wrote: > > I notice BTW that we have never updated the SET reference page since > > subtransactions were introduced --- it still says only that SET LOCAL > > is "local to the current transaction", without a word about > > subtransactions. So we have a documentation problem anyway. I recall > > that we had some discussion during the 8.0 dev cycle about whether > > having SET LOCAL's effects end at the end of the current subtransaction > > was really a good idea, given that subtransactions aren't the conceptual > > model the SQL spec defines, but nothing was ever done about changing > > the implementation. > > ISTM that's the real problem; SET LOCAL wasn't fully updated/considered > when subtransactions were added. > > One way to handle this would be to have 3 different behaviors for SET: > session-level, transaction-level, and sub-transaction level. If we had > that, we could probably make an across-the-board call that all functions > operate as if in their own sub-transaction, at least when it comes to > SET. What would be the use case for that? I can't see a single reason to do a SET LOCAL SUBTRANSACTION or whatever you'd call it. What you suggest sounds nicely symmetrical, but I don't think we need it in practice. ISTM that SET LOCAL is mostly superceded by per-function parameters. Most parameters need to be tied to code, not transactions. Of course, my wish to use synchronous_commit *was* tied to a transaction, but not a subtransaction. per-function parameters are sorely needed anyhow, since with session pools we can't easily use the username for SET parameters. -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com
Simon Riggs <simon@2ndquadrant.com> writes: > ISTM that SET LOCAL is mostly superceded by per-function parameters. Mostly, but not entirely. The case where you still need SET LOCAL is where the value you want to use locally has to be computed, or where you need to change it more than once within the function. Yet in such cases it'd still be handy to let the SET-clause mechanism deal with the detail of restoring the caller's value at exit. There is also a fairly nasty backward-compatibility problem. Suppose that security definer function OldSD uses the recommended-up-to-now method for setting a secure search path, which I quote from the 8.2 manual: old_path := pg_catalog.current_setting('search_path'); PERFORM pg_catalog.set_config('search_path', 'admin, pg_temp', true); -- Do whatever secure work we came for. PERFORM pg_catalog.set_config('search_path', old_path, true); (The set_config calls are equivalent to SET LOCAL.) Also suppose that security definer function NewSD uses the fancy new function-local- SET-clause method to avoid all that tedious stuff there. Now suppose that NewSD calls OldSD. If SET LOCAL overrides SET-clauses, this happens: * NewSD saves outer search path and sets its own. * OldSD saves NewSD's search path, then sets its own with SET LOCAL. * OldSD restores NewSD's search path using SET LOCAL. * NewSD tries to restore outer search path, but silently fails because SET LOCAL takes precedence. * We exit to the caller with NewSD's search path still in effect. This scenario will surely happen in the field, and therefore I argue that we *must* not allow SET LOCAL's effects to persist beyond the exit from a surrounding function-local SET clause on the same variable. I'm not sure what conclusions that leads to for other cases, though. We don't necessarily have to be consistent between the case where SET affects a variable and the case where it doesn't. regards, tom lane
Tom Lane wrote: > "Florian G. Pflug" <fgp@phlo.org> writes: >> Tom Lane wrote: > So it seems that only SET LOCAL within a function with per-function > GUC settings is at issue. I think that there is a pretty strong > use-case for saying that if you have a per-function setting of a > particular variable foo, then any "SET LOCAL foo" within the function > ought to vanish at function end --- for instance a function could want > to try a few different search_path settings and automatically revert to > the caller's setting on exit. Agreed. > The question is what about SET LOCAL > on a variable that *hasn't* been explicitly SET by the function > definition. Either approach we take with it could be surprising, > but probably having it revert at function end is more surprising... At least for me, the least surprising behaviour would be to revert it too. Than the rule becomes "a function is always executed in a pseudo-subtransaction that affects only GUCs" Since at least for pl/pgsql, a function body *alreay* is a BEGIN/END block - and therefore syntactically even looks like a subtransaction - this seems quite logical. And it would mean that the semantics of "SET LOCAL" won't change, just because you add an EXCEPTION clause to the function's toplevel BEGIN/END block. greetings, Florian Pflug
"Florian G. Pflug" <fgp@phlo.org> writes: > At least for me, the least surprising behaviour would be to > revert it too. Than the rule becomes "a function is always > executed in a pseudo-subtransaction that affects only GUCs" Only if it has at least one SET clause. The overhead is too high to insist on this for every function call. regards, tom lane
Tom Lane wrote: > "Florian G. Pflug" <fgp@phlo.org> writes: >> At least for me, the least surprising behaviour would be to >> revert it too. Than the rule becomes "a function is always >> executed in a pseudo-subtransaction that affects only GUCs" > > Only if it has at least one SET clause. The overhead is too high > to insist on this for every function call. In that case, I agree that only variables specified in a SET-clause should be reverted. Otherwise, adding or removing SET-clauses (e.g, because you chose a different implementation of a function that suddenly doesn't need regexps anymore) will cause quite arbitrary behavior changes. And the rule becomes (I tend to forget things, so I like simple rules that I can remember ;-) ) "For each SET-clause, there is a pseudo-subtransaction affecting only *this* GUC". greetings, Florian Pflug
"Florian G. Pflug" <fgp@phlo.org> writes: > And the rule becomes (I tend to forget things, so I like simple > rules that I can remember ;-) ) "For each SET-clause, there is > a pseudo-subtransaction affecting only *this* GUC". The other question is whether we want to change the behavior of SET LOCAL even in the absence of function SET-clauses. The current rule is that a LOCAL setting goes away at subtransaction commit, leading to this behavior: regression=# show regex_flavor;regex_flavor --------------advanced (1 row) regression=# begin; BEGIN regression=# savepoint x; SAVEPOINT regression=# set local regex_flavor to basic; SET regression=# release x; RELEASE regression=# show regex_flavor;regex_flavor --------------advanced (1 row) which makes some sense if you think of "release" as "subtransaction end", but not a lot if you think of it as forgetting a savepoint. Likewise, SET LOCAL within a plpgsql exception block goes away at successful block exit, which is not the first thing you'd expect. Neither of these behaviors are documented anywhere AFAIR; certainly the SET reference page doesn't explain 'em. I think we should probably take this opportunity to fix that, and make SET LOCAL mean "persists until end of current top-level transaction, unless rolled back earlier or within a function SET clause". So: * Plain SET takes effect immediately and persists unless rolled back or overridden by another explicit SET. In particular the value will escape out of a function that has a SET-clause for the same variable. * SET LOCAL takes effect immediately and persists until rolled back, overridden by another SET, or we exit a function that has a SET-clause for the same variable. * Rollback of a transaction or subtransaction cancels any SET or SET LOCAL within it. Otherwise, the latest un-rolled-back SET or SET LOCAL determines the active value within a transaction, and the latest un-rolled-back SET determines the value that will prevail after the transaction commits. * A function SET clause saves the entry-time value, and restores it at function exit, except when overridden by an un-rolled-back SET (but not SET LOCAL) within the function. Clear to everyone? Any objections? As far as implementation, I think this can be made to happen by rejiggering the value stacking and unstacking rules within guc.c. I'm tempted to try to get rid of the "tentative" value slots at the same time. That's a hangover from the pre-subtransaction implementation, when we only had to remember one inactive value for the case of SET followed by SET LOCAL within a transaction. Now that we have a stack of saved values, it seems to make more sense to try to handle this case by stacking the SET value when we hit SET LOCAL at the same nesting level. regards, tom lane
Tom Lane wrote: > "Florian G. Pflug" <fgp@phlo.org> writes: >> And the rule becomes (I tend to forget things, so I like simple >> rules that I can remember ;-) ) "For each SET-clause, there is >> a pseudo-subtransaction affecting only *this* GUC". > > The other question is whether we want to change the behavior of SET > LOCAL even in the absence of function SET-clauses. The current rule > is that a LOCAL setting goes away at subtransaction commit, leading > to this behavior: > > regression=# show regex_flavor; > regex_flavor > -------------- > advanced > (1 row) > > regression=# begin; > BEGIN > regression=# savepoint x; > SAVEPOINT > regression=# set local regex_flavor to basic; > SET > regression=# release x; > RELEASE > regression=# show regex_flavor; > regex_flavor > -------------- > advanced > (1 row) > > which makes some sense if you think of "release" as "subtransaction > end", but not a lot if you think of it as forgetting a savepoint. > Likewise, SET LOCAL within a plpgsql exception block goes away at > successful block exit, which is not the first thing you'd expect. > Neither of these behaviors are documented anywhere AFAIR; certainly > the SET reference page doesn't explain 'em. > > I think we should probably take this opportunity to fix that, and > make SET LOCAL mean "persists until end of current top-level > transaction, unless rolled back earlier or within a function SET > clause". > > So: > > * Plain SET takes effect immediately and persists unless rolled back > or overridden by another explicit SET. In particular the value will > escape out of a function that has a SET-clause for the same variable. > > * SET LOCAL takes effect immediately and persists until rolled back, > overridden by another SET, or we exit a function that has a SET-clause > for the same variable. > > * Rollback of a transaction or subtransaction cancels any SET or SET > LOCAL within it. Otherwise, the latest un-rolled-back SET or SET LOCAL > determines the active value within a transaction, and the latest > un-rolled-back SET determines the value that will prevail after the > transaction commits. > > * A function SET clause saves the entry-time value, and restores it at > function exit, except when overridden by an un-rolled-back SET (but not > SET LOCAL) within the function. > > Clear to everyone? Any objections? That makes "SET LOCAL" completely equivalent to "SET", except when used inside a function that has a corresponding SET-clause, right? So I think *if* this is done, "SET LOCAL" should be renamed to "SET FUNCTION". This would also prevent confusion, because everyone who currently uses SET LOCAL will have to change his code anyway, since the semantics change for every use-case apart from functions with SET-clauses, which don't exist in 8.2. Or am I overlooking something? And renaming SET LOCAL also emphasized that point that we are taking away functionality here - even if that functionality might not seem very useful. BTW, I *did* check the documentation before responding to Simon's original mail, and I *did* read it as "SET LOCAL goes away a subtransaction end". I figured that since there is no word on subtransactions in that part of the documentation, transaction will apply equally to both toplevel and subtransaction. It might very well be that I'm the only one who read it that way, though ;-) And I must admin that I wasn't completely sure, so I *did* try it out before I posted... I'd strong prefer "SET LOCAL" to kept it's current semantics, only that "SET LOCAL" changes will now be rolled back if the function has a matching SET-clause. For multiple reasons: .) It's useful to be able to temporarily change GUCs from a client, and being ableto reset them afterwards. Using a subtransaction for this is maybe a bit wastefull, but at least it works. .) Inpl/pgsql, that fact that "SET LOCAL" goes away after the current BEGIN/END block seems entirely logical. .) It doesn'ttake away existing functionality greetings, Florian Pflug
"Florian G. Pflug" <fgp@phlo.org> writes: > Tom Lane wrote: >> Clear to everyone? Any objections? > That makes "SET LOCAL" completely equivalent to "SET", except > when used inside a function that has a corresponding SET-clause, right? Maybe it wasn't clear :-(. They aren't equivalent because in the absence of rollback, SET's effects persist past main-transaction end; SET LOCAL's don't. That's the way they were defined originally (pre-subtransactions) and it still seems to make sense. > So I think *if* this is done, "SET LOCAL" should be renamed to > "SET FUNCTION". This would also prevent confusion, because everyone > who currently uses SET LOCAL will have to change his code anyway, > since the semantics change for every use-case apart from functions > with SET-clauses, which don't exist in 8.2. I'm not sure how many people have really written code that depends on the behavior of SET LOCAL rolling back at successful subtransaction end. I think we'd have heard about it if very many people had noticed, because it's not what the manual says. For the one use we've actually advocated (setting a temporary value within a function and then reverting to the old setting before exit), there isn't any visible change in behavior, since abandonment of the restored value at subtransaction end still ends up with the same result. > And renaming SET LOCAL also emphasized that point that we are taking > away functionality here - even if that functionality might not seem > very useful. We can't break the officially advocated solution for secure search_path. However, that particular coding pattern will still work with the change I'm proposing. It's only where you *don't* manually restore the prior value that you might notice a difference. > BTW, I *did* check the documentation before responding to Simon's original > mail, and I *did* read it as "SET LOCAL goes away a subtransaction end". > I figured that since there is no word on subtransactions in that part > of the documentation, transaction will apply equally to both toplevel > and subtransaction. Yeah, but you know that it's subtransactions under the hood, whereas someone who's thinking in terms of SAVEPOINT/RELEASE and BEGIN/EXCEPTION probably hasn't a clue about that. > .) In pl/pgsql, that fact that "SET LOCAL" goes away after the current > BEGIN/END block seems entirely logical. I don't think so ... your other side-effects such as table updates don't disappear, so why should SET's? I'm not necessarily averse to inventing a third version of SET, but I don't see a well-thought-out proposal here. In particular, we should be making an effort to *not* expose the concept of subtransaction at the SQL level at all, because that's not what the spec has. regards, tom lane
Tom Lane wrote: > "Florian G. Pflug" <fgp@phlo.org> writes: >> Tom Lane wrote: >>> Clear to everyone? Any objections? > >> That makes "SET LOCAL" completely equivalent to "SET", except >> when used inside a function that has a corresponding SET-clause, right? > > Maybe it wasn't clear :-(. They aren't equivalent because in the > absence of rollback, SET's effects persist past main-transaction end; > SET LOCAL's don't. That's the way they were defined originally > (pre-subtransactions) and it still seems to make sense. Ah, OK - things make much more sense now. >> So I think *if* this is done, "SET LOCAL" should be renamed to >> "SET FUNCTION". This would also prevent confusion, because everyone >> who currently uses SET LOCAL will have to change his code anyway, >> since the semantics change for every use-case apart from functions >> with SET-clauses, which don't exist in 8.2. > > I'm not sure how many people have really written code that depends on > the behavior of SET LOCAL rolling back at successful subtransaction end. > I think we'd have heard about it if very many people had noticed, > because it's not what the manual says. > > For the one use we've actually advocated (setting a temporary value > within a function and then reverting to the old setting before exit), > there isn't any visible change in behavior, since abandonment of the > restored value at subtransaction end still ends up with the same result. > >> And renaming SET LOCAL also emphasized that point that we are taking >> away functionality here - even if that functionality might not seem >> very useful. > > We can't break the officially advocated solution for secure search_path. > However, that particular coding pattern will still work with the change > I'm proposing. It's only where you *don't* manually restore the prior > value that you might notice a difference. > >> BTW, I *did* check the documentation before responding to Simon's original >> mail, and I *did* read it as "SET LOCAL goes away a subtransaction end". >> I figured that since there is no word on subtransactions in that part >> of the documentation, transaction will apply equally to both toplevel >> and subtransaction. > > Yeah, but you know that it's subtransactions under the hood, whereas > someone who's thinking in terms of SAVEPOINT/RELEASE and BEGIN/EXCEPTION > probably hasn't a clue about that. I plead guilty here ;-). That whole SAVEPOINT/RELEASE thing always seemed strange to me - I just accepted it at some point, but still translated it into something hierarchical I guess.... > >> .) In pl/pgsql, that fact that "SET LOCAL" goes away after the current >> BEGIN/END block seems entirely logical. > > I don't think so ... your other side-effects such as table updates don't > disappear, so why should SET's I guess because LOCAL to me implies some lexical locality - like the surrounding BEGIN/END block. > I'm not necessarily averse to inventing a third version of SET, but I > don't see a well-thought-out proposal here. In particular, we should be > making an effort to *not* expose the concept of subtransaction at the > SQL level at all, because that's not what the spec has. Thanks for your explanation - I can see your point now, after realizing why the spec has SAVEPOINT/RELEASE and *not* nested BEGIN/COMMIT blocks. So, at least on the SQL-level, I guess I agree - your new semantics fit better with the sql spec, even if they seemed quite strange to me at first sight. Though maybe we should add "SET TRANSACTION" as a synonym for "SET LOCAL"? - the former seems to convey your new semantics much better than the later. It still seems a bit strange that "SET LOCAL" is undone at function-exit, if the function has a matching SET-clause. But we need that for backwards- compatibility of the secure-search_path workaround, right? Maybe we could make "SET TRANSACTION" different from "SET LOCAL" in pl/pgsql, and warn if "SET LOCAL" is used? That would enable us either get rid of "SET LOCAL" in the long term, or to really make it local to the surrounding BEGIN/END block. So, to reiterate, my idea is .) Make "SET TRANSACTION" a synonym for "SET LOCAL" at the SQL-Level. .) In pl/pgsql, "SET TRANSACTION" sets a new value that is kept after the function exits, even if the function has a matchingSET-clause. .) "SET LOCAL" in pl/pgsql set a new value that is kept if the function has no matching SET-clause. If it has one, thevalue is restored. In any case, we emit a warning that "SET LOCAL" is going away. .) One day, make "SET LOCAL" in pl/pgsql mean "local to the surrounding BEGIN/END block". Independent of any SET-clausesthe function might or might not have. The last idea might seem to create a inconsistency between the SQL-level and pl/pgsql, but I think it does not. "SET LOCAL" is local to the surrounding BEGIN/{END|COMMIT} block in both cases - it's just that you have nested such blocks in pl/pgsql, but not in plain SQL. greetings, Florian Pflug
"Florian G. Pflug" <fgp@phlo.org> writes: > It still seems a bit strange that "SET LOCAL" is undone at function-exit, > if the function has a matching SET-clause. But we need that for backwards- > compatibility of the secure-search_path workaround, right? Yeah, I'm afraid we backed ourselves into a corner on that one. > So, to reiterate, my idea is > .) Make "SET TRANSACTION" a synonym for "SET LOCAL" at the SQL-Level. > .) In pl/pgsql, "SET TRANSACTION" sets a new value that is kept after the > function exits, even if the function has a matching SET-clause. > .) "SET LOCAL" in pl/pgsql set a new value that is kept if the function > has no matching SET-clause. If it has one, the value is restored. > In any case, we emit a warning that "SET LOCAL" is going away. > .) One day, make "SET LOCAL" in pl/pgsql mean "local to the surrounding > BEGIN/END block". Independent of any SET-clauses the function > might or might not have. I don't think it's a good idea to change SET LOCAL now and plan on changing it again later ;-). If we really want BEGIN-block-local SET capability, I'd prefer to think of some new keyword for that. But I'm not convinced it's interesting --- given the proposed behavior of function SET-clauses, attaching a SET to your function seems like it'll cover the need for restoring outer values. regards, tom lane
Tom Lane wrote: >> So, to reiterate, my idea is >> .) Make "SET TRANSACTION" a synonym for "SET LOCAL" at the SQL-Level. >> .) In pl/pgsql, "SET TRANSACTION" sets a new value that is kept after the >> function exits, even if the function has a matching SET-clause. >> .) "SET LOCAL" in pl/pgsql set a new value that is kept if the function >> has no matching SET-clause. If it has one, the value is restored. >> In any case, we emit a warning that "SET LOCAL" is going away. >> .) One day, make "SET LOCAL" in pl/pgsql mean "local to the surrounding >> BEGIN/END block". Independent of any SET-clauses the function >> might or might not have. > > I don't think it's a good idea to change SET LOCAL now and plan on > changing it again later ;-). If we really want BEGIN-block-local > SET capability, I'd prefer to think of some new keyword for that. > But I'm not convinced it's interesting --- given the proposed behavior > of function SET-clauses, attaching a SET to your function seems like > it'll cover the need for restoring outer values. Hm... could we still have "SET TRANSACTION" as a synonym for "SET LOCAL"? That would blend nicely with "SET TRANSACTION ISOLATION LEVEL" and "SET TRANSACTION READ ONLY". [ thinking... ] Hey, wait a moment. Regarding "SET TRANSACTION READ ONLY" - This is not strictly speaking a GUC, but still, if we pretend that there are no subtransaction, that command should too propage to the outermost transaction on release, shouldn't it? This is what happens currently (CVS HEAD with at least your initial function-SET-clause patch already in) regression=# begin ; BEGIN regression=# savepoint s1 ; SAVEPOINT regression=# set transaction read only ; SET regression=# release s1 ; RELEASE regression=# create table test (id int) ; CREATE TABLE regression=# commit ; COMMIT compared to: regression=# begin ; BEGIN regression=# set transaction read only ; SET regression=# create table test (id int) ; ERROR: transaction is read-only I believe that for consistencies sake, the "set transaction read only" should have propagated to the outermost transaction on "release s1". greetings, Florian Pflug
Florian G. Pflug wrote: > Tom Lane wrote: >>> So, to reiterate, my idea is >>> .) Make "SET TRANSACTION" a synonym for "SET LOCAL" at the SQL-Level. >>> .) In pl/pgsql, "SET TRANSACTION" sets a new value that is kept after >>> the >>> function exits, even if the function has a matching SET-clause. >>> .) "SET LOCAL" in pl/pgsql set a new value that is kept if the function >>> has no matching SET-clause. If it has one, the value is restored. >>> In any case, we emit a warning that "SET LOCAL" is going away. >>> .) One day, make "SET LOCAL" in pl/pgsql mean "local to the surrounding >>> BEGIN/END block". Independent of any SET-clauses the function >>> might or might not have. >> >> I don't think it's a good idea to change SET LOCAL now and plan on >> changing it again later ;-). If we really want BEGIN-block-local >> SET capability, I'd prefer to think of some new keyword for that. >> But I'm not convinced it's interesting --- given the proposed behavior >> of function SET-clauses, attaching a SET to your function seems like >> it'll cover the need for restoring outer values. > > Hm... could we still have "SET TRANSACTION" as a synonym for "SET LOCAL"? > That would blend nicely with "SET TRANSACTION ISOLATION LEVEL" and > "SET TRANSACTION READ ONLY". I don't think it's a very good idea to make SET TRANSACTION an alias for SET LOCAL, because SET TRANSACTION has already got its own meaning in the SQL spec - it sets transaction modes. Although I agree with you that variables set with SET LOCAL are also attached to the transaction (by definition), I would still rather separate transaction-local GUCs from spec-defined transaction modes. As precedence, they have two separate reference pages already: http://www.postgresql.org/docs/8.1/interactive/sql-set.html http://www.postgresql.org/docs/8.1/interactive/sql-set-transaction.html > [ thinking... ] Hey, wait a moment. Regarding "SET TRANSACTION READ ONLY" - > This is not strictly speaking a GUC, but still, if we pretend that > there are no subtransaction, that command should too propage to the > outermost transaction on release, shouldn't it? > ... > > I believe that for consistencies sake, the "set transaction read only" > should have propagated to the outermost transaction on "release s1". Sounds reasonable to me. I understand SAVEPOINT/RELEASE come from the SQL standard. So does the SQL standard say anything about this? Best Regards Michael Paesold
Michael Paesold <mpaesold@gmx.at> writes: > Florian G. Pflug wrote: >> Hm... could we still have "SET TRANSACTION" as a synonym for "SET LOCAL"? >> That would blend nicely with "SET TRANSACTION ISOLATION LEVEL" and >> "SET TRANSACTION READ ONLY". > I don't think it's a very good idea to make SET TRANSACTION an alias for > SET LOCAL, because SET TRANSACTION has already got its own meaning in the > SQL spec - it sets transaction modes. Yeah --- I'm not sure we could even do it without getting shift/reduce conflicts in bison. There is some attraction to the idea of keeping SET LOCAL's current behavior and inventing a third form of SET that has the lasts-till-end-of-current-main-transaction behavior. However (1) we'd have to pick some other keyword than TRANSACTION; (2) I still don't see how to document SET LOCAL's current behavior without introducing the concept of "subtransaction" into it, and I think we shouldn't do that. Basically my perspective on SET LOCAL is that its current behavior is a bug, and even though it's been that way for a couple major releases now, it's still something we oughta fix while we are busy whacking that part of the code around. Florian's example with SET TRANSACTION READ ONLY proves that it's a bug --- RELEASE is not defined to change any transaction modes. regards, tom lane
Tom Lane wrote: > Michael Paesold <mpaesold@gmx.at> writes: >> I don't think it's a very good idea to make SET TRANSACTION an alias for >> SET LOCAL, because SET TRANSACTION has already got its own meaning in the >> SQL spec - it sets transaction modes. > > Yeah --- I'm not sure we could even do it without getting shift/reduce > conflicts in bison. > > There is some attraction to the idea of keeping SET LOCAL's current > behavior and inventing a third form of SET that has the > lasts-till-end-of-current-main-transaction behavior. However > (1) we'd have to pick some other keyword than TRANSACTION; > (2) I still don't see how to document SET LOCAL's current behavior > without introducing the concept of "subtransaction" into it, and > I think we shouldn't do that. > > Basically my perspective on SET LOCAL is that its current behavior is a > bug, and even though it's been that way for a couple major releases now, > it's still something we oughta fix while we are busy whacking that part > of the code around. Florian's example with SET TRANSACTION READ ONLY > proves that it's a bug --- RELEASE is not defined to change any > transaction modes. Yeah, I think your original proposal was really sound. I would not expect the current SET LOCAL behaviour in the context of savepoints. If we really need the current behaviour, we should find a new name for this lasts-until-savepoint-release-or-transaction-end thingy. Best Regards Michael Paesold
On 9/5/07, Michael Paesold <mpaesold@gmx.at> wrote: > Tom Lane wrote: > > Basically my perspective on SET LOCAL is that its current behavior is a > > bug, and even though it's been that way for a couple major releases now, > > it's still something we oughta fix while we are busy whacking that part > > of the code around. Florian's example with SET TRANSACTION READ ONLY > > proves that it's a bug --- RELEASE is not defined to change any > > transaction modes. > > Yeah, I think your original proposal was really sound. I would not > expect the current SET LOCAL behaviour in the context of savepoints. > If we really need the current behaviour, we should find a new name for > this lasts-until-savepoint-release-or-transaction-end thingy. So, if I read you correctly, in summary we'd like to: * make SET LOCAL local to the transaction (i.e., make it behave as documented),* abandon the idea of a subtransaction-localSET, because the new function-local SET takes care of the interesting use-cases for that,* somehow deal with the incompatibility with the 8.2"security definer" workaround. Tom's proposal to handle the latter was that when a function-local SET reverts, it overrides any inner SET LOCALs. Am I on the right page? Cheers, BJ
"Brendan Jurd" <direvus@gmail.com> writes: > So, if I read you correctly, in summary we'd like to: > * make SET LOCAL local to the transaction (i.e., make it behave as documented), > * abandon the idea of a subtransaction-local SET, because the new > function-local SET takes care of the interesting use-cases for that, > * somehow deal with the incompatibility with the 8.2 "security > definer" workaround. > Tom's proposal to handle the latter was that when a function-local SET > reverts, it overrides any inner SET LOCALs. > Am I on the right page? Got it in one, I believe. regards, tom lane
On 9/5/07, Tom Lane <tgl@sss.pgh.pa.us> wrote: > "Brendan Jurd" <direvus@gmail.com> writes: > > Am I on the right page? > > Got it in one, I believe. In that case, +1 for your proposed changes. At first, like Florian, I found the idea of a SET LOCAL ever persisting beyond a function astonishing, but that's because I was approaching the term LOCAL from a programming frame of mind, not an SQL one. Once you appreciate that LOCAL means local to the transaction, rather than local to the programming context, it all makes sense. Cheers, BJ
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Wed, Sep 05, 2007 at 02:13:57PM +1000, Brendan Jurd wrote: > On 9/5/07, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > "Brendan Jurd" <direvus@gmail.com> writes: > > > Am I on the right page? > > > > Got it in one, I believe. > > In that case, +1 for your proposed changes. > > At first, like Florian, I found the idea of a SET LOCAL ever > persisting beyond a function astonishing, but that's because I was > approaching the term LOCAL from a programming frame of mind, not an > SQL one [...] As an unqualified POV, seeing that this got at least two people confused - -- wouldn't it make sense to be more verbose and call the thing SET TRANSACTION LOCAL (not just TRANSACTION, which is ambiguous as we have already seen). May be even SET LOCAL TO TRANSACTION (that gives at least some room for possible extensibility). I know too little about the parser to have even an idea whether this would be feasible at all. Regards - -- tomás -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFG3mFABcgs9XrR2kYRAug1AJ9FJdFEjDGpYWSj09+LgRv218efdwCcDBR8 kjE8O+QCdD/DMntr6mjHBoA= =FI+2 -----END PGP SIGNATURE-----