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

On Wed, Sep 25, 2013 at 2:55 AM, Bruce Momjian <bruce@momjian.us> wrote:
> On Thu, Sep 12, 2013 at 09:38:43AM +0530, Amit Kapila wrote:
>> > I have created the attached patch which issues an error when SET
>> > TRANSACTION and SET LOCAL are used outside of transactions:
>> >
>> >         test=> set transaction isolation level serializable;
>> >         ERROR:  SET TRANSACTION can only be used in transaction blocks
>> >         test=> reset transaction isolation level;
>> >         ERROR:  RESET TRANSACTION can only be used in transaction blocks
>> >
>> >         test=> set local effective_cache_size = '3MB';
>> >         ERROR:  SET LOCAL can only be used in transaction blocks
>> >         test=> set local effective_cache_size = default;
>> >         ERROR:  SET LOCAL can only be used in transaction blocks
>>
>> Shouldn't we do it for Set Constraints as well?
>
> Oh, very good point.  I missed that one.  Updated patch attached.

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)

2. I think we should update the documentation as well.

For example:
The documentation of LOCK TABLE
(http://www.postgresql.org/docs/devel/static/sql-lock.html) clearly
indicates that it will give error if used outside transaction block.

"LOCK TABLE is useless outside a transaction block: the lock would
remain held only to the completion of the statement. Therefore
PostgreSQL reports an error if LOCK is used outside a transaction
block. Use BEGINand COMMIT (or ROLLBACK) to define a transaction
block."


The documentation of SET TRANSACTION
(http://www.postgresql.org/docs/devel/static/sql-set-transaction.html)
has below line which doesn't indicate that it will give error if used
outside transaction block.

"If SET TRANSACTION is executed without a prior START TRANSACTION or
BEGIN, it will appear to have no effect, since the transaction will
immediately end."


3.

void
ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
{
..
..
else if (strcmp(stmt->name, "TRANSACTION SNAPSHOT") == 0)
{
A_Const    *con = (A_Const *) linitial(stmt->args);

RequireTransactionChain(isTopLevel, "SET TRANSACTION");

if (stmt->is_local)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("SET LOCAL TRANSACTION SNAPSHOT is not implemented")));
..
}
..
..
}

Wouldn't it be better if call to RequireTransactionChain() is done
after if (stmt->is_local)?

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