Skip to content

fix(spanner): show database value for unknown isolation levels #14981

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rayudu3745 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(spanner): show database value for unknown isolation levels #14981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
57d2d56
fix(spanner): show database value for unknown isolation levels
rayudu3745 Nov 12, 2026
aabb784
address review comments
rayudu3745 Nov 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions java-spanner/google-cloud-spanner/clirr-ignored-differences.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1162,4 +1162,9 @@
<className>com/google/cloud/spanner/DatabaseClient</className>
<method>com.google.spanner.v1.TransactionOptions$IsolationLevel getDatabaseDefaultIsolationLevel()</method>
</difference>
<difference>
<differenceType>7012</differenceType>
<className>com/google/cloud/spanner/DatabaseClient</className>
<method>java.lang.String getDatabaseDefaultIsolationLevelValue()</method>
</difference>
</differences>
10 changes: 10 additions & 0 deletions java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.spanner.v1.BatchWriteResponse;
import com.google.spanner.v1.TransactionOptions.IsolationLevel;
import com.google.spanner.v1.TransactionOptions.ReadWrite.ReadLockMode;
import org.jspecify.annotations.Nullable;

/**
* Interface for all the APIs that are used to read/write data into a Cloud Spanner database. An
Expand Down Expand Up @@ -60,6 +61,15 @@ default IsolationLevel getDatabaseDefaultIsolationLevel() {
throw new UnsupportedOperationException("method should be overwritten");
}

/**
* Returns the value of the {@code default_transaction_isolation} database option as returned by
* Spanner, or {@code null} if the option has not been set.
*/
@InternalApi
default @Nullable String getDatabaseDefaultIsolationLevelValue() {
throw new UnsupportedOperationException("method should be overwritten");
}

/**
* Returns the {@link DatabaseRole} used by the client connection. The database role that is used
* determines the access permissions that a connection has. This can for example be used to create
Expand Down
6 changes: 6 additions & 0 deletions ...anner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseClientImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,12 @@ public IsolationLevel getDatabaseDefaultIsolationLevel() {
return this.multiplexedSessionDatabaseClient.getDatabaseDefaultIsolationLevel();
}

@Override
@Nullable
public String getDatabaseDefaultIsolationLevelValue() {
return this.multiplexedSessionDatabaseClient.getDatabaseDefaultIsolationLevelValue();
}
Comment thread
rayudu3745 marked this conversation as resolved.
Resolved

private final AbstractLazyInitializer<StatementFactory> statementFactorySupplier =
new AbstractLazyInitializer<StatementFactory>() {
@Override
Expand Down
18 changes: 14 additions & 4 deletions ...spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,15 @@ final class DatabaseMetadata {

private final String dialect;
private final IsolationLevel defaultIsolationLevel;
private final @Nullable String defaultIsolationLevelValue;

@olavloite olavloite Nov 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it not be better to keep both these fields? That way, you do not need to reparse the string whenever the actual isolation level enum is needed. It would also make it possible to consider the string value more as a 'view' value, meaning that we could also format it in a more logical way. E.g. PostgreSQL normally returns isolation level names in lower case and without underscores.

@rayudu3745 rayudu3745 Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, the enum is parsed once and the string is kept as returned by Spanner.


private DatabaseMetadata(String dialect, IsolationLevel defaultIsolationLevel) {
private DatabaseMetadata(
String dialect,
IsolationLevel defaultIsolationLevel,
@Nullable String defaultIsolationLevelValue) {
this.dialect = dialect;
this.defaultIsolationLevel = defaultIsolationLevel;
this.defaultIsolationLevelValue = defaultIsolationLevelValue;
}

/**
Expand All @@ -55,20 +60,21 @@ private DatabaseMetadata(String dialect, IsolationLevel defaultIsolationLevel) {
static DatabaseMetadata fromResultSet(ResultSet resultSet) {
String dialect = Dialect.GOOGLE_STANDARD_SQL.name();
IsolationLevel defaultIsolationLevel = IsolationLevel.ISOLATION_LEVEL_UNSPECIFIED;
String defaultIsolationLevelValue = null;
while (resultSet.next()) {
switch (resultSet.getString(0)) {
case DATABASE_DIALECT_OPTION:
dialect = resultSet.getString(1);
break;
case DEFAULT_TRANSACTION_ISOLATION_OPTION:
defaultIsolationLevel =
parseIsolationLevel(resultSet.isNull(1) ? null : resultSet.getString(1));
defaultIsolationLevelValue = resultSet.isNull(1) ? null : resultSet.getString(1);
defaultIsolationLevel = parseIsolationLevel(defaultIsolationLevelValue);
break;
default:
break;
}
}
return new DatabaseMetadata(dialect, defaultIsolationLevel);
return new DatabaseMetadata(dialect, defaultIsolationLevel, defaultIsolationLevelValue);
}

/**
Expand Down Expand Up @@ -98,4 +104,8 @@ Dialect getDialect() {
IsolationLevel getDefaultIsolationLevel() {
return defaultIsolationLevel;
}

@Nullable String getDefaultIsolationLevelValue() {
return defaultIsolationLevelValue;
}
}
10 changes: 10 additions & 0 deletions ...loud-spanner/src/main/java/com/google/cloud/spanner/MultiplexedSessionDatabaseClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,16 @@ public IsolationLevel getDatabaseDefaultIsolationLevel() {
}
}

@Override
@Nullable
public String getDatabaseDefaultIsolationLevelValue() {
try {
return databaseMetadataSupplier.get().getDefaultIsolationLevelValue();
} catch (Exception exception) {
throw SpannerExceptionFactory.asSpannerException(exception);
}
}
Comment thread
rayudu3745 marked this conversation as resolved.
Resolved

@Override
public String getDatabaseRole() {
return this.sessionClient.getSpanner().getOptions().getDatabaseRole();
Expand Down
31 changes: 11 additions & 20 deletions ...er/src/main/java/com/google/cloud/spanner/connection/ConnectionStatementExecutorImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,37 +654,28 @@ public StatementResult statementShowTransactionIsolationLevel() {
: getConnection().getDefaultIsolationLevel();
return resultSet(
"transaction_isolation",
formatIsolationLevel(getEffectiveIsolationLevel(isolationLevel)),
getEffectiveIsolationLevel(isolationLevel),
SHOW_TRANSACTION_ISOLATION_LEVEL);
}

@Override
public StatementResult statementShowDefaultTransactionIsolation() {
return resultSet(
"default_transaction_isolation",
formatIsolationLevel(
getEffectiveIsolationLevel(getConnection().getDefaultIsolationLevel())),
getEffectiveIsolationLevel(getConnection().getDefaultIsolationLevel()),
SHOW_DEFAULT_TRANSACTION_ISOLATION);
}

/** Resolves an unspecified isolation level to the database default. */
private TransactionOptions.IsolationLevel getEffectiveIsolationLevel(
TransactionOptions.IsolationLevel isolationLevel) {
TransactionOptions.IsolationLevel configuredIsolationLevel =
isolationLevel == TransactionOptions.IsolationLevel.ISOLATION_LEVEL_UNSPECIFIED
? getConnection().getDatabaseClient().getDatabaseDefaultIsolationLevel()
: isolationLevel;
switch (configuredIsolationLevel) {
case ISOLATION_LEVEL_UNSPECIFIED:
return TransactionOptions.IsolationLevel.SERIALIZABLE;
case UNRECOGNIZED:
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.FAILED_PRECONDITION,
"The default isolation level of the database is not supported by this version of the"
+ " client.");
default:
return configuredIsolationLevel;
/** Resolves an unspecified isolation level to the database default and formats it. */
private String getEffectiveIsolationLevel(TransactionOptions.IsolationLevel isolationLevel) {

@olavloite olavloite Nov 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that you can simplify this method to something like this:

private String getEffectiveIsolationLevel(TransactionOptions.IsolationLevel isolationLevel) {
  if (isolationLevel != TransactionOptions.IsolationLevel.ISOLATION_LEVEL_UNSPECIFIED) {
    return formatIsolationLevel(isolationLevel);
  }
  String databaseDefault =
      getConnection().getDatabaseClient().getDatabaseDefaultIsolationLevelValue();
  if (databaseDefault != null) {
    return getConnection().getDialect() == Dialect.POSTGRESQL
        ? databaseDefault.toLowerCase(Locale.ROOT)
        : databaseDefault;
  }
  return formatIsolationLevel(TransactionOptions.IsolationLevel.SERIALIZABLE);
}

(and maybe even further if we keep both the string and enum values in DatabaseMetadata)

@rayudu3745 rayudu3745 Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplified as suggested. Spanner already stores these values in lower case with spaces, so the value is returned as-is.

if (isolationLevel != TransactionOptions.IsolationLevel.ISOLATION_LEVEL_UNSPECIFIED) {
return formatIsolationLevel(isolationLevel);
}
String databaseDefault =
getConnection().getDatabaseClient().getDatabaseDefaultIsolationLevelValue();
return databaseDefault == null
? formatIsolationLevel(TransactionOptions.IsolationLevel.SERIALIZABLE)
: databaseDefault;
}

private String formatIsolationLevel(TransactionOptions.IsolationLevel isolationLevel) {
Expand Down
16 changes: 16 additions & 0 deletions ...r/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3279,6 +3279,7 @@ public void testGetDatabaseDefaultIsolationLevelDefault() {
spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE));
assertEquals(
IsolationLevel.ISOLATION_LEVEL_UNSPECIFIED, client.getDatabaseDefaultIsolationLevel());
assertNull(client.getDatabaseDefaultIsolationLevelValue());
}

@Test
Expand All @@ -3296,6 +3297,21 @@ public void testGetDatabaseDefaultIsolationLevelRepeatableRead() {
}
}

@Test
public void testGetDatabaseDefaultIsolationLevelUnrecognized() {
mockSpanner.putStatementResult(
StatementResult.detectDialectResult(Dialect.POSTGRESQL, "read committed"));
try {
DatabaseClient client =
spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE));
assertEquals(IsolationLevel.UNRECOGNIZED, client.getDatabaseDefaultIsolationLevel());
assertEquals("read committed", client.getDatabaseDefaultIsolationLevelValue());
} finally {
mockSpanner.putStatementResult(
StatementResult.detectDialectResult(Dialect.GOOGLE_STANDARD_SQL));
}
}

@Test
public void testGetDialectAndDefaultIsolationLevelCachedWithSingleQuery() {
mockSpanner.putStatementResult(
Expand Down
5 changes: 5 additions & 0 deletions ...ner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseMetadataTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.google.cloud.spanner;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;

import com.google.cloud.spanner.Type.StructField;
Expand Down Expand Up @@ -49,6 +50,7 @@ public void testFromResultSetWithoutOptionsReturnsDefaults() {

assertEquals(Dialect.GOOGLE_STANDARD_SQL, metadata.getDialect());
assertEquals(IsolationLevel.ISOLATION_LEVEL_UNSPECIFIED, metadata.getDefaultIsolationLevel());
assertNull(metadata.getDefaultIsolationLevelValue());
}

@Test
Expand All @@ -60,6 +62,7 @@ public void testFromResultSetReadsAllOptions() {

assertEquals(Dialect.POSTGRESQL, metadata.getDialect());
assertEquals(IsolationLevel.REPEATABLE_READ, metadata.getDefaultIsolationLevel());
assertEquals("repeatable read", metadata.getDefaultIsolationLevelValue());
}

@Test
Expand All @@ -76,6 +79,7 @@ public void testFromResultSetWithNullIsolationLevelReturnsDefault() {
fromOptions(option(DatabaseMetadata.DEFAULT_TRANSACTION_ISOLATION_OPTION, null));

assertEquals(IsolationLevel.ISOLATION_LEVEL_UNSPECIFIED, metadata.getDefaultIsolationLevel());
assertNull(metadata.getDefaultIsolationLevelValue());
}

@Test
Expand All @@ -85,6 +89,7 @@ public void testFromResultSetWithUnknownIsolationLevel() {
option(DatabaseMetadata.DEFAULT_TRANSACTION_ISOLATION_OPTION, "read committed"));

assertEquals(IsolationLevel.UNRECOGNIZED, metadata.getDefaultIsolationLevel());
assertEquals("read committed", metadata.getDefaultIsolationLevelValue());
}

@Test
Expand Down
14 changes: 14 additions & 0 deletions ...r/google-cloud-spanner/src/test/java/com/google/cloud/spanner/MockSpannerServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,20 @@ public static StatementResult detectDialectResult(
return detectDatabaseMetadataResult(options);
}

/**
* Creates a result for the query that detects the dialect and default transaction isolation
* level of the database, with the given value for the default transaction isolation level. Use
* this to return a value that is not known to the client.
*/
public static StatementResult detectDialectResult(
Dialect resultDialect, String defaultIsolationLevelValue) {
Map<String, String> options = new LinkedHashMap<>();
options.put(DatabaseMetadata.DATABASE_DIALECT_OPTION, resultDialect.toString());
options.put(
DatabaseMetadata.DEFAULT_TRANSACTION_ISOLATION_OPTION, defaultIsolationLevelValue);
return detectDatabaseMetadataResult(options);
}

/**
* Creates a result for the query that reads the database options that are cached by the client.
* The result contains one row for each option in the given map. Options that are not in the map
Expand Down
19 changes: 8 additions & 11 deletions ...er/src/test/java/com/google/cloud/spanner/connection/ConnectionStatementExecutorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.atLeastOnce;
Expand All @@ -36,9 +35,7 @@
import com.google.cloud.Timestamp;
import com.google.cloud.spanner.DatabaseClient;
import com.google.cloud.spanner.Dialect;
import com.google.cloud.spanner.ErrorCode;
import com.google.cloud.spanner.ResultSet;
import com.google.cloud.spanner.SpannerException;
import com.google.cloud.spanner.TimestampBound;
import com.google.cloud.spanner.connection.PgTransactionMode.AccessMode;
import com.google.cloud.spanner.connection.PgTransactionMode.IsolationLevel;
Expand Down Expand Up @@ -306,8 +303,10 @@ public void testStatementShowTransactionIsolationLevel() {
public void testStatementShowIsolationLevelUsesDatabaseDefault() {
when(connection.getDefaultIsolationLevel()).thenReturn(ISOLATION_LEVEL_UNSPECIFIED);
mockDatabaseDefaultIsolationLevel(REPEATABLE_READ);
assertEquals(name(REPEATABLE_READ), value(subject.statementShowTransactionIsolationLevel()));
assertEquals(name(REPEATABLE_READ), value(subject.statementShowDefaultTransactionIsolation()));
when(connection.getDatabaseClient().getDatabaseDefaultIsolationLevelValue())
.thenReturn("repeatable read");
assertEquals("repeatable read", value(subject.statementShowTransactionIsolationLevel()));
assertEquals("repeatable read", value(subject.statementShowDefaultTransactionIsolation()));
}

@Test
Expand All @@ -322,12 +321,10 @@ public void testStatementShowIsolationLevelFallsBackToSerializable() {
public void testStatementShowIsolationLevelWithUnrecognizedDatabaseDefault() {
when(connection.getDefaultIsolationLevel()).thenReturn(ISOLATION_LEVEL_UNSPECIFIED);
mockDatabaseDefaultIsolationLevel(UNRECOGNIZED);
SpannerException exception =
assertThrows(SpannerException.class, subject::statementShowTransactionIsolationLevel);
assertEquals(ErrorCode.FAILED_PRECONDITION, exception.getErrorCode());
exception =
assertThrows(SpannerException.class, subject::statementShowDefaultTransactionIsolation);
assertEquals(ErrorCode.FAILED_PRECONDITION, exception.getErrorCode());
when(connection.getDatabaseClient().getDatabaseDefaultIsolationLevelValue())
.thenReturn("read committed");
assertEquals("read committed", value(subject.statementShowTransactionIsolationLevel()));
assertEquals("read committed", value(subject.statementShowDefaultTransactionIsolation()));
}

private void mockDatabaseDefaultIsolationLevel(TransactionOptions.IsolationLevel isolationLevel) {
Expand Down
Loading