From 6550a9d64b3e5525085f26bf1344e4524f8d0ffb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Thu, 24 Mar 2022 17:46:11 +0100 Subject: [PATCH] fix: return errors from BatchCreateSession to dialect detection (#1760) Any error that was returned by BatchCreateSessions should also be set as the result of the auto-detect dialect query, as the query will never be able to return a result for the dialect when session creation fails. Fixes #1759 --- .../com/google/cloud/spanner/SessionPool.java | 3 ++ .../cloud/spanner/DatabaseClientImplTest.java | 40 +++++++++++++++ .../spanner/connection/ConnectionTest.java | 49 +++++++++++++++++++ 3 files changed, 92 insertions(+) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java index 1d81b1388fa..7e5ffc43d48 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java @@ -2210,6 +2210,9 @@ private void handleCreateSessionsFailure(SpannerException e, int count) { break; } } + if (!dialect.isDone()) { + dialect.setException(e); + } if (isDatabaseOrInstanceNotFound(e)) { setResourceNotFoundException((ResourceNotFoundException) e); poolMaintainer.close(); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java index 407815983e2..456d9197848 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java @@ -2261,6 +2261,46 @@ public void testGetDialectPostgreSQLPreloaded() { } } + @Test + public void testGetDialect_FailsDirectlyIfDatabaseNotFound() { + mockSpanner.setBatchCreateSessionsExecutionTime( + SimulatedExecutionTime.stickyDatabaseNotFoundException("invalid-database")); + DatabaseClient client = + spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + + SpannerException exception = assertThrows(SpannerException.class, client::getDialect); + assertEquals(ErrorCode.NOT_FOUND, exception.getErrorCode()); + assertTrue( + exception + .getMessage() + .contains( + "NOT_FOUND: Database not found: Database with id invalid-database not found")); + } + + @Test + public void testGetDialectDefaultPreloaded_FailsDirectlyIfDatabaseNotFound() { + mockSpanner.setBatchCreateSessionsExecutionTime( + SimulatedExecutionTime.stickyDatabaseNotFoundException("invalid-database")); + try (Spanner spanner = + this.spanner + .getOptions() + .toBuilder() + .setSessionPoolOption( + SessionPoolOptions.newBuilder().setAutoDetectDialect(true).build()) + .build() + .getService()) { + DatabaseClient client = + spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + SpannerException exception = assertThrows(SpannerException.class, client::getDialect); + assertEquals(ErrorCode.NOT_FOUND, exception.getErrorCode()); + assertTrue( + exception + .getMessage() + .contains( + "NOT_FOUND: Database not found: Database with id invalid-database not found")); + } + } + @Test public void testUntypedNullParameters() { Statement statement = diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionTest.java index 00f62138700..85025ad91bc 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionTest.java @@ -19,13 +19,17 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import com.google.api.core.ApiFuture; import com.google.api.core.ApiFutures; import com.google.cloud.spanner.AbortedException; +import com.google.cloud.spanner.Dialect; import com.google.cloud.spanner.ErrorCode; +import com.google.cloud.spanner.MockSpannerServiceImpl; +import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime; import com.google.cloud.spanner.ResultSet; import com.google.cloud.spanner.SpannerException; import com.google.cloud.spanner.SpannerOptions; @@ -43,6 +47,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import javax.annotation.Nonnull; +import org.junit.After; import org.junit.AfterClass; import org.junit.Test; import org.junit.experimental.runners.Enclosed; @@ -465,4 +470,48 @@ public void testShowSetRPCPriority() { } } } + + public static class DialectDetectionTest extends AbstractMockServerTest { + protected String getBaseUrl() { + return super.getBaseUrl() + ";minSessions=1"; + } + + @After + public void reset() { + // Reset dialect to default. + mockSpanner.putStatementResult( + MockSpannerServiceImpl.StatementResult.detectDialectResult(Dialect.GOOGLE_STANDARD_SQL)); + mockSpanner.reset(); + mockSpanner.removeAllExecutionTimes(); + // Close all open Spanner instances to ensure that each test run gets a fresh instance. + SpannerPool.closeSpannerPool(); + } + + @Test + public void testDefaultGetDialect() { + try (Connection connection = createConnection()) { + assertEquals(Dialect.GOOGLE_STANDARD_SQL, connection.getDialect()); + } + } + + @Test + public void testPostgreSQLGetDialect() { + mockSpanner.putStatementResult( + MockSpannerServiceImpl.StatementResult.detectDialectResult(Dialect.POSTGRESQL)); + try (Connection connection = createConnection()) { + assertEquals(Dialect.POSTGRESQL, connection.getDialect()); + } + } + + @Test + public void testGetDialect_DatabaseNotFound() throws Exception { + mockSpanner.setBatchCreateSessionsExecutionTime( + SimulatedExecutionTime.stickyDatabaseNotFoundException("invalid-database")); + try (Connection connection = createConnection()) { + SpannerException exception = assertThrows(SpannerException.class, connection::getDialect); + assertEquals(ErrorCode.NOT_FOUND, exception.getErrorCode()); + assertTrue(exception.getMessage().contains("Database with id invalid-database not found")); + } + } + } }