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

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Morten Hustveit <morten(at)eventures(dot)vc>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
Date: 2013-09-29 06:10:51
Message-ID: CAA4eK1JKZZo=Z7aJsFi5FPAF4tHd5qF0t39ZZuN-t80V9zmOTQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Sep 25, 2013 at 2:55 AM, Bruce Momjian <bruce(at)momjian(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ian Lawrence Barwick 2013-09-29 08:09:12 Patch: FORCE_NULL option for copy COPY in CSV mode
Previous Message Alvaro Herrera 2013-09-28 23:56:21 Re: logical changeset generation v6.1