[rayudu3745]

When no isolation level was set on the connection, Connection#getTransactionIsolation() and DatabaseMetaData#getDefaultTransactionIsolation() always reported TRANSACTION_SERIALIZABLE. The client then sends no isolation level, so Spanner actually runs the transaction at the database's default_transaction_isolation:

Call (database default repeatable read, no level set on the connection)	Before	After
getTransactionIsolation()	TRANSACTION_SERIALIZABLE	TRANSACTION_REPEATABLE_READ
getMetaData().getDefaultTransactionIsolation()	TRANSACTION_SERIALIZABLE	TRANSACTION_REPEATABLE_READ

If the database has no default, both still return TRANSACTION_SERIALIZABLE.

Frameworks use the reported level to decide whether to change it. Spring's DataSourceUtils.prepareConnectionForTransaction only calls setTransactionIsolation when the requested level differs from getTransactionIsolation(). HikariCP only applies a configured transactionIsolation when it differs from the reported default. With a repeatable read database default, a requested SERIALIZABLE was therefore silently skipped, and the transaction ran as REPEATABLE READ.

Changes

• getDefaultTransactionIsolation() returns the database default from DatabaseClient#getDatabaseDefaultIsolationLevel() (chore(spanner): expose database default isolation level #14806) instead of a hard-coded SERIALIZABLE. If the database has no default, it returns SERIALIZABLE. If this client does not recognize the database default, it throws a SQLException with FAILED_PRECONDITION.
• getTransactionIsolation() returns getDefaultTransactionIsolation() when no isolation level is set on the connection.

Notes

• Only the reported level changes. What is sent to Spanner is unchanged: with no level set on the connection, the client still sends no isolation level.
• The DatabaseClient reads the database default together with the dialect and caches it (chore(spanner): expose database default isolation level #14806), so this adds no RPCs. A later change to the database's default_transaction_isolation option is not reflected until the client is recreated.
• No public API changes. IsolationLevelConverter and setTransactionIsolation are unchanged.

Testing

• New mock-server test TransactionMockServerTest#testUsesDatabaseDefaultIsolationLevel. With a repeatable read database default, both methods report REPEATABLE_READ, and the transaction is sent without an isolation level. After setTransactionIsolation(SERIALIZABLE), the next transaction is sent as SERIALIZABLE.
• New test JdbcConnectionTest#testUnrecognizedDatabaseDefaultIsolationLevel: an unrecognized database default throws FAILED_PRECONDITION.
• JdbcConnectionTest and JdbcDatabaseMetaDataTest: the mocked DatabaseClient returns ISOLATION_LEVEL_UNSPECIFIED, like a real client when the database option is not set.

[gemini-code-assist]

Code Review

This pull request updates the JDBC connection and metadata implementation to dynamically retrieve and use the database's default transaction isolation level. Specifically, getTransactionIsolation in AbstractJdbcConnection now falls back to the metadata's default isolation level if the connection's default is unspecified. Additionally, getDefaultTransactionIsolation in JdbcDatabaseMetaData has been updated to fetch the default isolation level from the database client, throwing an exception if it is unrecognized. Unit and mock server tests have been added and updated to verify these behaviors. I have no feedback to provide as there are no review comments.

[rayudu3745]

/gemini review

[gemini-code-assist]

Code Review

This pull request updates the JDBC connection and metadata implementation to dynamically retrieve and use the database's default isolation level instead of hardcoding it to TRANSACTION_SERIALIZABLE. Specifically, it modifies getDefaultTransactionIsolation in JdbcDatabaseMetaData to fetch the default isolation level from the database client, handles unrecognized isolation levels by throwing a FAILED_PRECONDITION exception, and updates AbstractJdbcConnection to fall back to this metadata-defined default when the connection's default isolation level is unspecified. Corresponding unit and mock server tests have been added and updated to verify these behaviors. I have no feedback to provide as there are no review comments.

[olavloite]

I think that this method (getTransactionIsolation()) is intended to return the isolation level of the current transaction, and not the default isolation level.

This also means that line 152 above for setTransactionIsolation(..) is probably wrong.

Feel free to defer this to a follow-up PR, as this is an existing (different) bug.

[rayudu3745]

Thanks! Agreed on the getter. JDBC defines it as "this Connection object's current transaction isolation level", and BEGIN ISOLATION LEVEL … or SET TRANSACTION ISOLATION LEVEL … can give the current transaction a different level than the default. getTransactionIsolation() currently ignores that. I'll fix it in a follow-up PR. ConnectionImpl#getTransactionIsolationLevel() is package-private, so it first needs to be exposed on Connection. Then the method can use the same logic as SHOW TRANSACTION ISOLATION LEVEL: the level of the current transaction when in a transaction, otherwise the default, with an unspecified level resolved to the database default.

For setTransactionIsolation(..), I think setting the default is correct. The JDBC spec (JSR 221, §10.2.1) says "the new isolation level remains in effect for the remainder of the session or until the next invocation of the setTransactionIsolation method", and connection pools rely on that. For example, HikariCP sets its configured transactionIsolation once per physical connection. If we only changed the current transaction, every later transaction would go back to the default. setDefaultIsolationLevel already applies to the current transaction as long as that transaction hasn't started.

[olavloite]

Should we add a catch clause for Throwable as well? IsolationLevelConverter.convertToJdbc(..) could in theory throw IllegalArgumentException (although I think that in the current implementation, the code path that does that is unreachable)

[rayudu3745]

Done

[olavloite]

Could we cache the result of this? Calling getMetadata() every time actually allocates a new object, and this method could potentially be called often by frameworks or connection pools.

[rayudu3745]

Done