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. I'm not convinced of this. I doubt we actually have any real numbers? 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. > Clients can code around this as well for old servers. This is something that is more palatable if the server defines this behaviour. > > > 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. > I think his point is that the error is emitted before we actually do the rollback and it could fail. > > 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. > I guess the error has to be sent after the rollback completes. > > 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. > Pretty sure I can code around this. -- > Vik Fearing >
