Re: Add regression tests for autocommit-off mode for psql and fix some omissions

From: Feike Steenbergen <feikesteenbergen(at)gmail(dot)com>
To: Marko Tiikkaja <marko(at)joh(dot)to>
Cc: Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add regression tests for autocommit-off mode for psql and fix some omissions
Date: 2014-10-07 13:24:57
Message-ID: CAK_s-G1U7-CqaGSpPU0eEvtYKCJZ6HEsH9WPhf78oi9F6GT+FA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 7 October 2014 09:55, Marko Tiikkaja <marko(at)joh(dot)to> wrote:
> It's not clear to me that this is fixing a problem, to be honest. If you're
> running autocommit=off, you have an expectation that you can roll back
> commands at will. It's fine if I can't roll back a VACUUM, for example,
> since I would practically never want to do that. But ALTER TYPE .. ADD
> VALUE ..; is an entirely different beast. That one's permanent; there's no
> DROP equivalent. If the command is just executed, and I can't roll it back,
> wouldn't that be a serious violation of the principle of least astonishment?

I think you have a valid and good point; however the autocommit-off mode can
currently already execute statements which cannnot be rolled back.
Perhaps it is a good idea to not allow any of these statements in autocommit-off
mode to prevent astonishement from users, but that would be a discussion of
itself.

My reason for proposing this is to have all these commands treated
consistently.
The expectation of being able to roll back commands at will cannot be fulfilled
currently, many statemens that are allowed with autocommit-off fall into the
category "different beast".

Currently the following statemens call PreventTransactionChain and do not
generate errors in autocommit-off mode:
- REINDEX DATABASE
- CREATE INDEX CONCURRENTLY
- ALTER SYSTEM
- CREATE DATABASE
- DROP DATABASE
- CREATE TABLESPACE
- DROP TABLESPACE
- CLUSTER
- VACUUM

The following statements call PreventTransactionChain and do generate errors
in autocommit-off mode:
- DROP INDEX CONCURRENTLY
- ALTER DATABASE ... SET TABLESPACE
- ALTER TYPE ... ADD

I don't see why these last three should be treated seperately from the
first list; we should
either allow all, or none of these statements IMHO.

kind regards,

Feike Steenbergen

On 7 October 2014 09:55, Marko Tiikkaja <marko(at)joh(dot)to> wrote:
> On 10/7/14, 9:11 AM, Feike Steenbergen wrote:
>>
>> Perhaps I am the only one using autocommit-off mode
>
>
> You most definitely aren't.
>
>> and we shouldn't put effort
>> into fixing this?
>
>
> It's not clear to me that this is fixing a problem, to be honest. If you're
> running autocommit=off, you have an expectation that you can roll back
> commands at will. It's fine if I can't roll back a VACUUM, for example,
> since I would practically never want to do that. But ALTER TYPE .. ADD
> VALUE ..; is an entirely different beast. That one's permanent; there's no
> DROP equivalent. If the command is just executed, and I can't roll it back,
> wouldn't that be a serious violation of the principle of least astonishment?
> DROP INDEX CONCURRENTLY has a bit of the same problem. You can CREATE INDEX
> CONCURRENTLY, but it might take days in some cases.
>
> I think that just running the command is a bad idea, and if we want to fix
> something here we should focus on just providing a better error message.
>
>
> .marko

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2014-10-07 13:53:57 Re: TAP test breakage on MacOS X
Previous Message Robert Haas 2014-10-07 13:06:52 Re: Promise index tuples for UPSERT