Re: Per-function GUC settings: trickier than it looked

Tom Lane wrote:
> "Florian G. Pflug" <fgp(at)phlo(dot)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 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.

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