Re: autocommit vs TRUNCATE et al

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > Tom Lane wrote:
> >> There are a number of statements, such as TRUNCATE TABLE, that refuse to
> >> run in a transaction block because they perform actions that can't be
> >> rolled back later.
> >>
> >> These statements currently do not look at autocommit, which means that
> >> if autocommit is off, their tests will succeed ... but then a
> >> transaction block is started anyway, defeating the point of the test.
>
> > ... I think we
> > should just do an automatic COMMIT if it is the first statement of a
> > transaction, and if not, throw the same error we used to throw. We are
> > performing autocommit for SET at the start of a transaction now anyway,
> > so it isn't totally strange to do it for TRUNCATE, etc. too.
>
> There is another aspect of this, which is: what if one of these
> statements is called by a user-defined function, say a plpgsql function
> that does a TRUNCATE and then other stuff? If the function is called
> by a SELECT that's not inside a transaction block, then its
> IsTransactionBlock() test will succeed --- but the possibility remains
> that the later actions of the function could cause an elog and
> transaction rollback. Which is what we wanted to prevent.
>
> We can go with the auto-COMMIT idea for statements that are invoked at
> the outer interactive level, but that doesn't work for stuff invoked
> inside a function. I think we need to forbid these statements inside
> functions, too. We already have that for VACUUM, because of its
> internal commits causing problems for functions, but we'll need to
> extend it to all of them.
>
> Just FYI, the statements involved are
> VACUUM
> TRUNCATE TABLE
> CREATE/DROP DATABASE
> REINDEX (all forms)
> ALTER USER changing password
> DROP USER
>
> ALTER and DROP USER just issue NOTICEs rather than failing, which seems
> pretty bogus in itself. The reason they are worried is that there's
> no mechanism for rolling back updates of the flat password file.
> I think we could fix that by arranging not to write the flat password
> file at all until we are ready to commit the current transaction;
> will take a look at it.

Yes, I thought we had those secure, but I see now we don't. We only
have interlocking so the file is reread at the proper time.

> REINDEX perhaps could be treated as transaction-safe in the forms that
> build a new index file rather than truncating. Will look at that, too.
>
> Another place that is calling IsTransactionBlock is parser/analyze.c
> while processing DECLARE CURSOR. I think this is pretty bogus for
> several reasons:
> 1. There's no good reason to make DECLARE CURSOR outside a transaction
> block be an ERROR; at most it should be a NOTICE.
> 2. Parse analysis is the wrong place anyway; it should be tested
> at execution time, methinks.
> 3. If the cursor is being declared and used inside a function, then
> it could be used successfully without being inside a transaction
> block at all.
>
> Point 3 makes me think we should just get rid of the test entirely.
> Comments?

I thought the transaction test was in DECLARE so people didn't create
cursors outside of transactions and then wonder why they didn't work.
If it is going to fail, an ERROR seems more appropriate than a NOTICE.
I can see it happening inside a function, yes.

Another question related to this is the handling of SET/SHOW/RESET in
functions. People should realize it isn't really the first command in
the transaction so will be part of the transaction. The big issue is
that SET has a fallback when it is not first in a transaciton, namely to
be part of the transaction, while TRUNCATE doesn't have that fallback
because it can't be rolled back.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073