On Wed, 26 Feb 2020 at 16:57, Vik Fearing <v...@postgresfriends.org> wrote:
> On 26/02/2020 22:22, Tom Lane wrote: > > Dave Cramer <davecramer@postgres.rocks> writes: > >> OK, here is a patch that actually doesn't leave the transaction in a > failed > >> state but emits the error and rolls back the transaction. > > > >> This is far from complete as it fails a number of tests and does not > cover > >> all of the possible paths. > >> But I'd like to know if this is strategy will be acceptable ? > > > > I really don't think that changing the server's behavior here is going to > > fly. The people who are unhappy that we changed it are going to vastly > > outnumber the people who are happy. Even the people who are happy are > not > > going to find that their lives are improved all that much, because > they'll > > still have to deal with old servers with the old behavior for the > > foreseeable future. > > Dealing with old servers for a while is something that everyone is used to. > > > Even granting that a behavioral incompatibility is acceptable, I'm not > > sure how a client is supposed to be sure that this "error" means that a > > rollback happened, as opposed to real errors that prevented any state > > change from occurring. > > Because the error is a Class 40 — Transaction Rollback. > > My original example was: > > postgres=!# commit; > ERROR: 40P00: transaction cannot be committed > DETAIL: First error was "42601: syntax error at or near "error"". > > > > (A trivial example of that is misspelling the > > COMMIT command; which I'll grant is unlikely in practice. But there are > > less-trivial examples involving internal server malfunctions.) > > Misspelling the COMMIT command is likely a syntax error, which is Class > 42. Can you give one of those less-trivial examples please? > > > The only > > way to be sure you're out of the transaction is to check the transaction > > state that's sent along with ReadyForQuery ... but if you need to do > > that, it's not clear why we should change the server behavior at all. > > How does this differ from the deferred constraint violation example you > provided early on in the thread? That gave the error 23505 and > terminated the transaction. If you run the same scenario with the > primary key immediate, you get the *exact same error* but the > transaction is *not* terminated! > > I won't go so far as to suggest we change all COMMIT errors to Class 40 > (as the spec says), but I'm thinking it very loudly. > > > I also don't think that this scales to the case of subtransaction > > commit/rollback. That should surely act the same, but your patch doesn't > > change it. > > How does one commit a subtransaction? > > > Lastly, introducing a new client-visible message level seems right out. > > That's a very fundamental protocol break, independently of all else. > > Yeah, this seemed like a bad idea to me, too. > Ok, here is a much less obtrusive solution thanks to Vladimir. FWIW, only 10 of 196 tests fail. Dave Cramer www.postgres.rocks > >
throwerror2.patch
Description: Binary data
