Fix nested transaction savepoint handling in JDBC transaction manager #3047
Conversation
| @@ -63,6 +63,10 @@ abstract class AbstractTransactionSpec extends Specification implements TestProp | |||
| return false | |||
| } | |||
|
|
|||
| boolean failsInsertInReadOnlyTx() { | |||
There was a problem hiding this comment.
Some drivers don't fail when saving in read only transaction so needed to adjust existing read only tests
| * @param transactionName The transaction name | ||
| * @return savepoint name for given transaction name | ||
| */ | ||
| private static String getSavepointName(String transactionName) { |
There was a problem hiding this comment.
The issue was when transaction name is like SomeService.someMethod and Oracle does not accept '.' in savepoint names. On the other hand, MSSQL limits savepoint name to 32 characters which was failing in tests for some transactional methods so this is what could work for both cases - return 'SavePoint' + transactionName.hashCode(). Trimming to 32 might cause some duplicated savepoint names from different methods/transactions so I think this is safer, unless we have to have human readable savepoint name for debug purposes.
There was a problem hiding this comment.
There's also (possibly?) the option of not using a save point name at all:
https://docs.oracle.com/javase/8/docs/api/java/sql/Connection.html#setSavepoint--
There was a problem hiding this comment.
It makes sense, since we don't refer to the savepoint name later, if I am not wrong.
Do you agree @dstepanov ?
There was a problem hiding this comment.
I have changed to set savepoint without passing name and then don't need to deal with special chars and max length.
|
Please also check the reactive TX manager |
Updated logic for Hibernate transaction manager, reactive currently throwing |
| * @return true if exception is thrown for unsupported operation | ||
| */ | ||
| protected boolean isUnsupportedOperation(Exception exception) { | ||
| if (exception instanceof SQLFeatureNotSupportedException) { |
There was a problem hiding this comment.
R2DBC wouldn't throw JDBC exceptions. Maybe Oracle will do it, because it's based on it but otherwise it's not expected
There was a problem hiding this comment.
I see, moved back code from AbstractDefaultTransactionOperations into classes that will use this check.
…ctionOperations class
|
Added transaction tests for other dialects that were missing.