Обсуждение: BUG #17810: Update from 13.09 to 13.10 breaks SQLs with VACUUM

The following bug has been logged on the website:

Bug reference:      17810
Logged by:          Yuri Cherio
Email address:      cherio@gmail.com
PostgreSQL version: 13.10
Operating system:   Ubuntu Linux 20.04.5 LTS
Description:

My system has numerous SQL scripts (each script contains multiple SQL
statements). Many of them look like

UPDATE ...
INSERT ...
...
VACUUM ...

These scripts are being run from a Java program, with AutoCommit=true, via
JDBC as:

Statement statement = connection.createStatement();
connection.setAutoCommit(true);
statement.execute(sql);
...
statement.getMoreResults();
...

This worked well for years until the server (not the client program) was
upgraded to 13.10. After the upgrade all compound SQL statements with VACUUM
at the end fail with this error:

09:48:56.585 SEVERE Script processing failed:
org.postgresql.util.PSQLException: ERROR: VACUUM cannot be executed within a
pipeline
        at
org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(QueryExecutorImpl.java:2675)
        at
org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:2365)
        at
org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:355)
        at
org.postgresql.jdbc.PgStatement.executeInternal(PgStatement.java:490)
        at org.postgresql.jdbc.PgStatement.execute(PgStatement.java:408)
        at
org.postgresql.jdbc.PgStatement.executeWithFlags(PgStatement.java:329)
        at
org.postgresql.jdbc.PgStatement.executeCachedSql(PgStatement.java:315)
        at
org.postgresql.jdbc.PgStatement.executeWithFlags(PgStatement.java:291)
        at org.postgresql.jdbc.PgStatement.execute(PgStatement.java:286)
 
JDBC driver version is 42.3.3 (i tried the latest 42.5.4 and it doesn't make
a difference).
The same SQL scripts are still working with the latest postgresql 14.7

PG Bug reporting form <noreply@postgresql.org> writes:
> My system has numerous SQL scripts (each script contains multiple SQL
> statements). Many of them look like

> UPDATE ...
> INSERT ...
> ...
> VACUUM ...

> These scripts are being run from a Java program, with AutoCommit=true, via
> JDBC as:

Sorry, you'll have to fix your scripts to do that separately.
(It might be sufficient to add COMMIT before the VACUUM.)
It was a bug that we didn't enforce this requirement all along.

> The same SQL scripts are still working with the latest postgresql 14.7

I'm quite skeptical of that claim; I see the restriction being
enforced the same way in all supported branches.

            regards, tom lane

Cherio <cherio@gmail.com> writes:
> I just realized that the ramifications of this change go further than just
> VACUUM related statements in the scripts. Assume 2 scripts

> UPDATE tableA
> UPDATE tableB

> and

> UPDATE tableA
> UPDATE tableB

> Before the change they could run in parallel without issues. After the
> change this will cause one of the queries to fail due to transaction locks.

Uh ... really?  Please provide evidence.  AFAIK this set of changes
only affects commands that are meant to not run inside tranaction blocks.

            regards, tom lane

Cherio <cherio@gmail.com> writes:
> The second script should have tables used in the reversed order:
> UPDATE tableA
> UPDATE tableB

> and

> UPDATE tableB
> UPDATE tableA

> Since they will run in individual transactions tableA gets locked by the
> 1st script and tableB by the 2nd; as execution flow proceeds to the next
> update in each script, those tables would be locked in separate
> transactions.

I think you are working with a completely wrong mental model of what
this change did.  It will not affect a pipeline that doesn't contain
any VACUUM, ANALYZE, or similar maintenance commands.

            regards, tom lane

Cherio <cherio@gmail.com> writes:
> You are right. My mental model of the change is a speculation, but this is
> only because the specifics of the change are not clarified anywhere.

The problem is that statements that aren't allowed to execute in a
transaction block (usually because they need an immediate commit)
weren't checking that properly in the context of pipelined queries.
This should never have been allowed, but we weren't enforcing that
properly.  The net effect in some cases is that such commands
unexpectedly committed the effects of prior commands in the pipeline,
and in other cases that a rollback occurring later in the pipeline
would leave you with corrupted on-disk state.  We judged that to be
a bad enough bug that the fix should be backpatched.

The original fix for that went in six months ago (13.8 et al) but
we later discovered that there was still a hole in it.  The fact
that the 13.10 release notes only mention ANALYZE is a result of
my inadequate summarization of the commit log entry :-(.

> BTW, why ANALYZE?

ANALYZE is a slightly different case.  It can work inside a transaction
block or not, but it has two different strategies depending on that: one
uses internal commits and the other doesn't.  It was applying the wrong
one in this context, which again had the effect of prematurely committing
previous commands in the pipeline.

> Are VACUUM and ANALYZE the only commands that must be executed
> separately?

There's a couple dozen such commands --- easiest way to find them
is to grep the source code for PreventInTransactionBlock calls.
I believe "can't run inside a transaction block" is mentioned in
all their manual pages, but we don't have a central list.

            regards, tom lane

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> As for your usage of "conn.setAutocommit(true)" - IIUC that is irrelevant
> to this entire discussion.  You've chosen to bundle up multiple statements
> into a single Statement.execute(string) call which obeys the rules of the
> simple query protocol - multiple statements:

I doubt it.  We closed the not-in-transaction-block loophole decades
ago for simple query protocol.  What's at stake here is what happens
when a series of extended-protocol commands are given without Sync
between them, which we interpret as a request to run them all in the
same transaction.  I'm a bit surprised that the JDBC driver is choosing
to issue them that way, because it implies (at least) that it's parsing
the string enough to break it down into separate SQL commands.  But
we'd not be having this conversation if that weren't happening.
Anyway, that scenario *should* be subject to the same rules as multiple
statements in one simple-query message, only it wasn't up till now.

> Is there room for improved communication on the minor-release change in
> behavior?  Probably.

Yeah, I wish I could have that release note back :-(.  We don't really
have any mechanism for updating release notes after the fact.  I could
go and fix it in the git repo, but nobody would see it until after the
next quarterly releases which seems a bit too late.

> As for discussing reverting this in the back-branches given new evidence
> and scenarios, that is possible and I've yet to go back and fully review
> that discussion thread in light of this new information.

I think there's zero chance we'd revert the bug fix at this point.
If we'd realized that the JDBC driver behaves this way, we might have
debated a little harder about whether this really had to be fixed in
the back branches --- but it's done now, and it is a bug fix.

            regards, tom lane