[rayudu3745]

Spanner databases can set a default isolation level with the default_transaction_isolation option. Clients send ISOLATION_LEVEL_UNSPECIFIED and Spanner applies that default, but the client had no way to find out what it is. This PR exposes it on DatabaseClient, so that the Connection API and tools like PGAdapter can report it instead of forcing SERIALIZABLE.

• The dialect-detection query now also reads default_transaction_isolation. It is still a single information_schema.database_options query, so there are no extra RPCs, and the result is cached for the lifetime of the client, like the dialect.
• New DatabaseClient#getDatabaseDefaultIsolationLevel() (@InternalApi). It returns SERIALIZABLE when the option isn't set. Connection API users can reach it through Connection#getDatabaseClient().
• Transactions are unchanged: Connection#getDefaultIsolationLevel() still returns ISOLATION_LEVEL_UNSPECIFIED unless it has been set, and BeginTransaction still sends ISOLATION_LEVEL_UNSPECIFIED.
• New test helper MockSpannerServiceImpl.StatementResult.detectDialectResult(Dialect, IsolationLevel). The existing detectDialectResult(Dialect) keeps working.

Note: a change made with ALTER DATABASE after the value has been read is only seen by new clients.

[rayudu3745]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces the ability to retrieve and cache the default transaction isolation level configured for a Spanner database. It consolidates the retrieval of the database dialect and default isolation level into a single query using a new DatabaseMetadata class, and exposes this configuration via DatabaseClient and Connection interfaces. The feedback highlights a critical compilation error in DatabaseClient.java due to a missing import of IsolationLevel, and suggests a robustness improvement in DatabaseMetadata.java to use case-insensitive comparison when parsing the isolation level.

[gemini-code-assist]

Code Review

This pull request introduces a DatabaseMetadata class to query and cache database options, specifically the dialect and default transaction isolation level, from information_schema.database_options in a single query. It exposes getDatabaseDefaultIsolationLevel() on the DatabaseClient and Connection interfaces, updates their implementations, and adds comprehensive tests. The reviewer suggested using equalsIgnoreCase instead of equals when parsing the isolation level value in DatabaseMetadata to improve robustness against case variations.

[rayudu3745]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces the retrieval and caching of database metadata, specifically the database dialect and the default transaction isolation level, by querying information_schema.database_options in a single query. It adds the getDatabaseDefaultIsolationLevel() method to the DatabaseClient and Connection interfaces and their implementations, backed by a new DatabaseMetadata helper class. The feedback highlights a missing import for IsolationLevel in DatabaseClient.java that will cause a compilation failure, and suggests adding a defensive null check when reading the database dialect in DatabaseMetadata.java to prevent potential runtime exceptions.

[rayudu3745]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces the ability to retrieve and cache the default transaction isolation level of a Spanner database alongside its dialect in a single query, exposing it via the new getDatabaseDefaultIsolationLevel() method on DatabaseClient. The feedback highlights a missing import for IsolationLevel in DatabaseClient.java that will cause a compilation failure, as well as a potential NullPointerException in DatabaseMetadata.java if the database dialect option value is null.

[olavloite]

Why do we return SERIALIZABLE when it is not set? Would it not be better to return UNSPECIFIED, so a caller knows the difference between:

1. It has explicitly been set to SERIALIZABLE
2. It has not been set

[rayudu3745]

Agreed. we can return ISOLATION_LEVEL_UNSPECIFIED when the option has not been set, so callers can tell an explicit SERIALIZABLE apart from "no default configured". Callers that need a concrete isolation level should treat ISOLATION_LEVEL_UNSPECIFIED as SERIALIZABLE?. That's also what Spanner does when a transaction doesn't specify an isolation level and the database has no default

[olavloite]

It would be better/safer to use a switch statement here. Consider the possibility that Spanner adds a new isolation level in the future and someone uses an older client with a database that uses that new isolation level. That client would then read SERIALIZABLE for that new isolation level.

[olavloite]

This needs some more safety. Spanner could in theory introduce a new dialect in the future. An old client that connects to a database with that dialect would get an unexpected exception here.

[olavloite]

See also my comment above. I am not sure whether defaulting to GOOGLE_SQL and SERIALIZABLE is a good idea. It means that we are returning a value that seems to be 'correct' in a case where Spanner has introduced a new value that is unknown to this client version.