Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

On Tue, Oct 1, 2013 at 7:49 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> On Sun, Sep 29, 2013 at 11:40:51AM +0530, Amit Kapila wrote:
>> >> Shouldn't we do it for Set Constraints as well?
>> >
>> > Oh, very good point. I missed that one. Updated patch attached.
>
> I am glad you are seeing things I am not. :-)
>
>> 1. The function set_config also needs similar functionality, else
>> there will be inconsistency, the SQL statement will give error but
>> equivalent function set_config() will succeed.
>>
>> SQL Command
>> postgres=# set local search_path='public';
>> ERROR: SET LOCAL can only be used in transaction blocks
>>
>> Function
>> postgres=# select set_config('search_path', 'public', true);
>> set_config
>> ------------
>> public
>> (1 row)
>
> I looked at this but could not see how to easily pass the value of
> 'isTopLevel' down to the SELECT. All the other checks have isTopLevel
> passed down from the utility case statement.

Yes, we cannot pass isTopLevel, but as isTopLevel is used to decide
whether we are in function (user defined) call, so if we can find
during statement execution (current case set_config execution) that
current statement is inside user function execution, then it can be
handled.
For example, one of the ways could be to use a mechanism similar to
setting of user id and sec context used by fmgr_security_definer() (by
calling function SetUserIdAndSecContext()), once userid and sec
context are set by fmgr_security_definer(), later we can use
InSecurityRestrictedOperation() anywhere to give error.

For current case, what we can do is after analyze
(pg_analyze_and_rewrite), check if its not a builtin function (as we
can have functionid after analyze, so it can be checked
fmgr_isbuiltin(functionId)) and set variable to indicate that we are
in function call.

Any better or simpler idea can also be used to identify isTopLevel
during function execution.

Doing it only for detection of transaction chain in set_config path
might seem to be more work, but I think it can be used at other places
for detection of transaction chain as well.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com