Skip to content

Commit 6550a9d

Browse files
authored
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
1 parent 6d64104 commit 6550a9d

File tree

3 files changed

+92
-0
lines changed

3 files changed

+92
-0
lines changed

google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2210,6 +2210,9 @@ private void handleCreateSessionsFailure(SpannerException e, int count) {
22102210
break;
22112211
}
22122212
}
2213+
if (!dialect.isDone()) {
2214+
dialect.setException(e);
2215+
}
22132216
if (isDatabaseOrInstanceNotFound(e)) {
22142217
setResourceNotFoundException((ResourceNotFoundException) e);
22152218
poolMaintainer.close();

google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2261,6 +2261,46 @@ public void testGetDialectPostgreSQLPreloaded() {
22612261
}
22622262
}
22632263

2264+
@Test
2265+
public void testGetDialect_FailsDirectlyIfDatabaseNotFound() {
2266+
mockSpanner.setBatchCreateSessionsExecutionTime(
2267+
SimulatedExecutionTime.stickyDatabaseNotFoundException("invalid-database"));
2268+
DatabaseClient client =
2269+
spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE));
2270+
2271+
SpannerException exception = assertThrows(SpannerException.class, client::getDialect);
2272+
assertEquals(ErrorCode.NOT_FOUND, exception.getErrorCode());
2273+
assertTrue(
2274+
exception
2275+
.getMessage()
2276+
.contains(
2277+
"NOT_FOUND: Database not found: Database with id invalid-database not found"));
2278+
}
2279+
2280+
@Test
2281+
public void testGetDialectDefaultPreloaded_FailsDirectlyIfDatabaseNotFound() {
2282+
mockSpanner.setBatchCreateSessionsExecutionTime(
2283+
SimulatedExecutionTime.stickyDatabaseNotFoundException("invalid-database"));
2284+
try (Spanner spanner =
2285+
this.spanner
2286+
.getOptions()
2287+
.toBuilder()
2288+
.setSessionPoolOption(
2289+
SessionPoolOptions.newBuilder().setAutoDetectDialect(true).build())
2290+
.build()
2291+
.getService()) {
2292+
DatabaseClient client =
2293+
spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE));
2294+
SpannerException exception = assertThrows(SpannerException.class, client::getDialect);
2295+
assertEquals(ErrorCode.NOT_FOUND, exception.getErrorCode());
2296+
assertTrue(
2297+
exception
2298+
.getMessage()
2299+
.contains(
2300+
"NOT_FOUND: Database not found: Database with id invalid-database not found"));
2301+
}
2302+
}
2303+
22642304
@Test
22652305
public void testUntypedNullParameters() {
22662306
Statement statement =

google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionTest.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,17 @@
1919
import static com.google.common.truth.Truth.assertThat;
2020
import static org.junit.Assert.assertEquals;
2121
import static org.junit.Assert.assertFalse;
22+
import static org.junit.Assert.assertThrows;
2223
import static org.junit.Assert.assertTrue;
2324
import static org.junit.Assert.fail;
2425

2526
import com.google.api.core.ApiFuture;
2627
import com.google.api.core.ApiFutures;
2728
import com.google.cloud.spanner.AbortedException;
29+
import com.google.cloud.spanner.Dialect;
2830
import com.google.cloud.spanner.ErrorCode;
31+
import com.google.cloud.spanner.MockSpannerServiceImpl;
32+
import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime;
2933
import com.google.cloud.spanner.ResultSet;
3034
import com.google.cloud.spanner.SpannerException;
3135
import com.google.cloud.spanner.SpannerOptions;
@@ -43,6 +47,7 @@
4347
import java.util.concurrent.TimeUnit;
4448
import java.util.concurrent.TimeoutException;
4549
import javax.annotation.Nonnull;
50+
import org.junit.After;
4651
import org.junit.AfterClass;
4752
import org.junit.Test;
4853
import org.junit.experimental.runners.Enclosed;
@@ -465,4 +470,48 @@ public void testShowSetRPCPriority() {
465470
}
466471
}
467472
}
473+
474+
public static class DialectDetectionTest extends AbstractMockServerTest {
475+
protected String getBaseUrl() {
476+
return super.getBaseUrl() + ";minSessions=1";
477+
}
478+
479+
@After
480+
public void reset() {
481+
// Reset dialect to default.
482+
mockSpanner.putStatementResult(
483+
MockSpannerServiceImpl.StatementResult.detectDialectResult(Dialect.GOOGLE_STANDARD_SQL));
484+
mockSpanner.reset();
485+
mockSpanner.removeAllExecutionTimes();
486+
// Close all open Spanner instances to ensure that each test run gets a fresh instance.
487+
SpannerPool.closeSpannerPool();
488+
}
489+
490+
@Test
491+
public void testDefaultGetDialect() {
492+
try (Connection connection = createConnection()) {
493+
assertEquals(Dialect.GOOGLE_STANDARD_SQL, connection.getDialect());
494+
}
495+
}
496+
497+
@Test
498+
public void testPostgreSQLGetDialect() {
499+
mockSpanner.putStatementResult(
500+
MockSpannerServiceImpl.StatementResult.detectDialectResult(Dialect.POSTGRESQL));
501+
try (Connection connection = createConnection()) {
502+
assertEquals(Dialect.POSTGRESQL, connection.getDialect());
503+
}
504+
}
505+
506+
@Test
507+
public void testGetDialect_DatabaseNotFound() throws Exception {
508+
mockSpanner.setBatchCreateSessionsExecutionTime(
509+
SimulatedExecutionTime.stickyDatabaseNotFoundException("invalid-database"));
510+
try (Connection connection = createConnection()) {
511+
SpannerException exception = assertThrows(SpannerException.class, connection::getDialect);
512+
assertEquals(ErrorCode.NOT_FOUND, exception.getErrorCode());
513+
assertTrue(exception.getMessage().contains("Database with id invalid-database not found"));
514+
}
515+
}
516+
}
468517
}

0 commit comments

Comments
 (0)