On Thu, Feb 13, 2020 at 2:38 AM Haumacher, Bernhard <haui@haumacher.de> wrote: > 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.
I am not sure that this example really proves anything. If insertFrameworkLevelState(con), insertApplicationLevelState(con), and con.commit() throw exceptions, or if they return a status code and you check it and throw an exception if it's not what you expect, then it will work.
Thing is that con.commit() DOESN'T return a status code, nor does it throw an exception as we silently ROLLBACK here.
As noted up thread it's somewhat worse as depending on how the transaction failed we seem to do different things
In Tom's example we do issue a warning and say there is no transaction running. I would guess we silently rolled back earlier.
In my example we don't issue the warning we just roll back.
I do agree with Tom that changing this behaviour at this point would make things worse for more people than it would help so I am not advocating throwing an error here.
I would however advocate for consistently doing the same thing with failed transactions