Re: Error on failed COMMIT
Am 24.02.2020 um 13:34 schrieb Robert Haas: As I said upthread, I think one of the things that would be pretty badly broken by this is psql -f something.sql, where something.sql contains a series of blocks of the form "begin; something; something; something; commit;". Right now whichever transactions succeed get committed. With the proposed change, if one transaction block fails, it'll merge with all of the following blocks. No, that's *not* true. The only difference with the proposed change would be another error in the logs for the commit following the block with the failed insert. Note: Nobody has suggested that the commit that returns with an error should not end the transaction. Do just the same as with any other commit error in response to a constraint violation! Am 24.02.2020 um 18:53 schrieb David Fetter: On Mon, Feb 24, 2020 at 06:40:16PM +0100, Vik Fearing wrote: On 24/02/2020 18:37, David Fetter wrote: If we'd done this from a clean sheet of paper, it would have been the right decision. We're not there, and haven't been for decades. OTOH, it's never too late to do the right thing. Some right things take a lot of prep work in order to actually be right things. This is one of them. Defaulting to SERIALIZABLE isolation is another. Here the proposed changes is really much much less noticable - please report the error (again) instead of giving an incomprehensible status code. Nothing else must be changed - the failing commit should do the rollback and end the transaction - but it should report this situation as an error! Regards Bernhard
Re: Error on failed COMMIT
Am 17.02.2020 um 23:12 schrieb Dave Cramer:
On Mon, 17 Feb 2020 at 13:02, Haumacher, Bernhard <mailto:h...@haumacher.de>> wrote:
... would be an appropriate solution. PostgreSQL reports the
"unsuccessful" commit through the "ROLLBACK" status code and the
driver
translates this into a Java SQLException, because this is the only
way
to communicate the "non-successfullness" from the void commit()
method.
Since the commit() was not successful, from the API point of view
this
is an error and it is fine to report this using an exception.
Well it doesn't always report the unsuccessful commit as a rollback
sometimes it says
"there is no transaction" depending on what happened in the transaction
even worse...
Also when there is an error there is also a status provided by the
backend.
Since this is not an error to the backend there is no status that the
exception can provide.
be free to choose/define one...
Doing this in a (non-default) driver setting is not ideal, because I
expect do be notified *by default* from a database (driver) if a
commit
was not successful (and since the API is void, the only notification
path is an exception). We already have a non-default option named
"autosafe", which fixes the problem somehow.
The challenge with making this the default, is as Tom noted, many
other people don't expect this.
Nobody expects a database reporting a successful commit, while it
internally rolled back.
If there is code out there depending on this bug, it is fair to provide
a backwards-compatible option to re-activate this unexpected behavior.
What many other frameworks do is have vendor specific behaviour.
Perhaps writing a proxying driver might solve the problem?
That's exactly what we do - extending our database abstraction layer to
work around database-specific interpretations of the JDBC API.
But of cause, the abstraction layer is not able to reconstruct an error
from a commit() call, that has been dropped by the driver. Of cause, I
could try to insert another dummy entry into a dummy table immediately
before each commit to get again the exception reporting that the
transaction is in rollback-only-mode... but this does not sound
reasonable to me.
If we really need both behaviors ("silently ignore failed commits"
and
"notify about failed commits") I would prefer adding a
backwards-compatible option
"silently-ignore-failed-commit-due-to-auto-rollback" (since it is a
really aburd setting from my point of view, since consistency is
at risk
if this happens - the worst thing to expect from a database).
The error has been reported to the client. At this point the client is
expected to do a rollback.
As I explained, there is not "the client" but there are several software
layers - and the error only has been reported to some of these layers
that may decide not to communicate the problem down the road. Therefore,
the final commit() must report the problem again.
Best regard, Bernhard
Re: Error on failed COMMIT
Am 14.02.2020 um 20:36 schrieb Robert Haas:
On Fri, Feb 14, 2020 at 2:08 PM Dave Cramer wrote:
Well now you are asking the driver to re-interpret the results in a different
way than the server which is not what we tend to do.
The server throws an error we throw an error. We really aren't in the business
of re-interpreting the servers responses.
I don't really see a reason why the driver has to throw an exception
if and only if there is an ERROR on the PostgreSQL side. But even if
you want to make that rule for some reason, it doesn't preclude
correct behavior here. All you really need is to have con.commit()
return some indication of what the command tag was, just as, say, psql
would do.
I think, this would be an appropriate solution. PostgreSQL reports the
"unsuccessful" commit through the "ROLLBACK" status code and the driver
translates this into a Java SQLException, because this is the only way
to communicate the "non-successfullness" from the void commit() method.
Since the commit() was not successful, from the API point of view this
is an error and it is fine to report this using an exception.
Am 14.02.2020 um 21:07 schrieb Tom Lane:
Dave Cramer writes:
We have the same blast radius.
I have offered to make the behaviour requested dependent on a configuration
parameter but apparently this is not sufficient.
Nope, that is absolutely not happening. We learned very painfully, back
around 7.3 when we tried to put in autocommit on/off, that if server
behaviors like this are configurable then most client code has to be
prepared to work with every possible setting. The argument that "you can
just set it to work the way you expect" is a dangerous falsehood. I see
no reason to think that a change like this wouldn't suffer the same sort
of embarrassing and expensive failure that autocommit did.
Doing this in a (non-default) driver setting is not ideal, because I
expect do be notified *by default* from a database (driver) if a commit
was not successful (and since the API is void, the only notification
path is an exception). We already have a non-default option named
"autosafe", which fixes the problem somehow.
If we really need both behaviors ("silently ignore failed commits" and
"notify about failed commits") I would prefer adding a
backwards-compatible option
"silently-ignore-failed-commit-due-to-auto-rollback" (since it is a
really aburd setting from my point of view, since consistency is at risk
if this happens - the worst thing to expect from a database).
Regards, Bernhard
Re: Error on failed COMMIT
Am 12.02.2020 um 00:27 schrieb Tom Lane:
Vik Fearing writes:
Actually, I was imagining that it would end the transaction as it does
today, just with an error code.
This is backed up by General Rule 9 which says "The current
SQL-transaction is terminated."
Hm ... that would be sensible, but I'm not entirely convinced. There
are several preceding rules that say that an exception condition is
raised, and normally you can stop reading at that point; nothing else
is going to happen. If COMMIT acts specially in this respect, they
ought to say so.
In any case, while this interpretation might change the calculus a bit,
I think we still end up concluding that altering this behavior has more
downside than upside.
Let me illustrate this issue from an application (framework) developer's
perspective:
When an application interacts with a database, it must be clearly
possible to determine, whether a commit actually succeeded (and made all
changes persistent), or the commit failed for any reason (and all of the
changes have been rolled back). If a commit succeeds, an application
must be allowed to assume that all changes it made in the preceeding
transaction are made persistent and it is valid to update its internal
state (e.g. caches) to the values updated in the transaction. This must
be possible, even if the transaction is constructed collaboratively by
multipe independent layers of the application (e.g. a framework and an
application layer). Unfortunately, this seems not to be possible with
the current implementation - at least not with default settings:
Assume the application is written in Java and sees Postgres through the
JDBC driver:
composeTransaction() {
Connection con = getConnection(); // implicitly "begin"
try {
insertFrameworkLevelState(con);
insertApplicationLevelState(con);
con.commit();
publishNewState();
} catch (Throwable ex) {
con.rollback();
}
}
With the current implementation, it is possible, that the control flow
reaches "publishNewState()" without the changes done in
"insertFrameworkLevelState()" have been made persistent - without the
framework-level code (which is everything except
"insertApplicationLevelState()") being able to detect the problem (e.g.
if "insertApplicationLevelState()" tries add a null into a non-null
column catching the exception or any other application-level error that
is not properly handled through safepoints).
From a framework's perspective, this behavior is absolutely
unacceptable. Here, the framework-level code sees a database that
commits successfully but does not make its changes persistent.
Therefore, I don't think that altering this behavior has more downside
than upside.
Best regards
Bernhard
