diff --git a/google-cloud-spanner/clirr-ignored-differences.xml b/google-cloud-spanner/clirr-ignored-differences.xml index f65bfd94332..464f6e9e9f8 100644 --- a/google-cloud-spanner/clirr-ignored-differences.xml +++ b/google-cloud-spanner/clirr-ignored-differences.xml @@ -6,6 +6,11 @@ com/google/cloud/spanner/connection/Connection com.google.cloud.spanner.Dialect getDialect() + + 7012 + com/google/cloud/spanner/DatabaseClient + com.google.cloud.spanner.Dialect getDialect() + 7012 com/google/cloud/spanner/BatchReadOnlyTransaction @@ -15,4 +20,19 @@ 8001 com/google/cloud/spanner/connection/StatementParser + + 7002 + com/google/cloud/spanner/SpannerOptions + com.google.cloud.spanner.Dialect getDialect() + + + 7002 + com/google/cloud/spanner/SpannerOptions$Builder + com.google.cloud.spanner.SpannerOptions$Builder setDialect(com.google.cloud.spanner.Dialect) + + + 7002 + com/google/cloud/spanner/connection/ConnectionOptions + com.google.cloud.spanner.Dialect getDialect() + diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseClient.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseClient.java index 60e8b6910cf..a09c06bcbe2 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseClient.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseClient.java @@ -27,6 +27,15 @@ */ public interface DatabaseClient { + /** + * Returns the SQL dialect that is used by the database. + * + * @return the SQL dialect that is used by the database. + */ + default Dialect getDialect() { + throw new UnsupportedOperationException("method should be overwritten"); + } + /** * Writes the given mutations atomically to the database. * diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseClientImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseClientImpl.java index e2121910ad8..d0ee29ada17 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseClientImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseClientImpl.java @@ -53,6 +53,11 @@ PooledSessionFuture getSession() { return pool.getSession(); } + @Override + public Dialect getDialect() { + return pool.getDialect(); + } + @Override public Timestamp write(final Iterable mutations) throws SpannerException { return writeWithOptions(mutations).getCommitTimestamp(); 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 95cc167fe10..5c13a410984 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 @@ -1484,6 +1484,34 @@ private void keepAlive() { } } + private void determineDialectAsync(final SettableFuture dialect) { + Preconditions.checkNotNull(dialect); + executor.submit( + () -> { + try { + dialect.set(determineDialect()); + } catch (Throwable t) { + // Catch-all as we want to propagate all exceptions to anyone who might be interested + // in the database dialect, and there's nothing sensible that we can do with it here. + dialect.setException(t); + } finally { + releaseSession(this, Position.FIRST); + } + }); + } + + private Dialect determineDialect() { + try (ResultSet dialectResultSet = + delegate.singleUse().executeQuery(DETERMINE_DIALECT_STATEMENT)) { + if (dialectResultSet.next()) { + return Dialect.fromName(dialectResultSet.getString(0)); + } else { + throw SpannerExceptionFactory.newSpannerException( + ErrorCode.NOT_FOUND, "No dialect found for database"); + } + } + } + private void markBusy(Span span) { this.delegate.setCurrentSpan(span); this.state = SessionState.BUSY; @@ -1724,7 +1752,27 @@ private enum Position { RANDOM } + /** + * This statement is (currently) used to determine the dialect of the database that is used by the + * session pool. This statement is subject to change when the INFORMATION_SCHEMA contains a table + * where the dialect of the database can be read directly, and any tests that want to detect the + * specific 'determine dialect statement' should rely on this constant instead of the actual + * value. + */ + @VisibleForTesting + static final Statement DETERMINE_DIALECT_STATEMENT = + Statement.newBuilder( + "SELECT 'POSTGRESQL' AS DIALECT\n" + + "FROM INFORMATION_SCHEMA.SCHEMATA\n" + + "WHERE SCHEMA_NAME='pg_catalog'\n" + + "UNION ALL\n" + + "SELECT 'GOOGLE_STANDARD_SQL' AS DIALECT\n" + + "FROM INFORMATION_SCHEMA.SCHEMATA\n" + + "WHERE SCHEMA_NAME='INFORMATION_SCHEMA' AND CATALOG_NAME=''") + .build(); + private final SessionPoolOptions options; + private final SettableFuture dialect = SettableFuture.create(); private final SessionClient sessionClient; private final ScheduledExecutorService executor; private final ExecutorFactory executorFactory; @@ -1734,6 +1782,9 @@ private enum Position { private final Object lock = new Object(); private final Random random = new Random(); + @GuardedBy("lock") + private boolean detectDialectStarted; + @GuardedBy("lock") private int pendingClosure; @@ -1861,6 +1912,39 @@ private SessionPool( this.initMetricsCollection(metricRegistry, labelValues); } + /** + * @return the {@link Dialect} of the underlying database. This method will block until the + * dialect is available. It will potentially execute one or two RPCs to get the dialect if + * necessary: One to create a session if there are no sessions in the pool (yet), and one to + * query the database for the dialect that is used. It is recommended that clients that always + * need to know the dialect set {@link + * SessionPoolOptions.Builder#setAutoDetectDialect(boolean)} to true. This will ensure that + * the dialect is fetched automatically in a background task when a session pool is created. + */ + Dialect getDialect() { + boolean mustDetectDialect = false; + synchronized (lock) { + if (!detectDialectStarted) { + mustDetectDialect = true; + detectDialectStarted = true; + } + } + if (mustDetectDialect) { + try (PooledSessionFuture session = getSession()) { + dialect.set(session.get().determineDialect()); + } + } + try { + return dialect.get(60L, TimeUnit.SECONDS); + } catch (ExecutionException executionException) { + throw SpannerExceptionFactory.asSpannerException(executionException); + } catch (InterruptedException interruptedException) { + throw SpannerExceptionFactory.propagateInterrupt(interruptedException); + } catch (TimeoutException timeoutException) { + throw SpannerExceptionFactory.propagateTimeout(timeoutException); + } + } + @VisibleForTesting int getNumberOfSessionsInUse() { synchronized (lock) { @@ -2290,10 +2374,17 @@ public void onSessionReady(SessionImpl session) { } else { Preconditions.checkState(totalSessions() <= options.getMaxSessions() - 1); allSessions.add(pooledSession); - // Release the session to a random position in the pool to prevent the case that a batch - // of sessions that are affiliated with the same channel are all placed sequentially in - // the pool. - releaseSession(pooledSession, Position.RANDOM); + if (options.isAutoDetectDialect() && !detectDialectStarted) { + // Get the dialect of the underlying database if that has not yet been done. Note that + // this method will release the session into the pool once it is done. + detectDialectStarted = true; + pooledSession.determineDialectAsync(SessionPool.this.dialect); + } else { + // Release the session to a random position in the pool to prevent the case that a batch + // of sessions that are affiliated with the same channel are all placed sequentially in + // the pool. + releaseSession(pooledSession, Position.RANDOM); + } } } if (closeSession) { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java index 208c025e6da..408b3d73965 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java @@ -50,6 +50,7 @@ public class SessionPoolOptions { private final ActionOnSessionNotFound actionOnSessionNotFound; private final ActionOnSessionLeak actionOnSessionLeak; private final long initialWaitForSessionTimeoutMillis; + private final boolean autoDetectDialect; private SessionPoolOptions(Builder builder) { // minSessions > maxSessions is only possible if the user has only set a value for maxSessions. @@ -67,6 +68,7 @@ private SessionPoolOptions(Builder builder) { this.loopFrequency = builder.loopFrequency; this.keepAliveIntervalMinutes = builder.keepAliveIntervalMinutes; this.removeInactiveSessionAfter = builder.removeInactiveSessionAfter; + this.autoDetectDialect = builder.autoDetectDialect; } @Override @@ -87,7 +89,8 @@ public boolean equals(Object o) { this.initialWaitForSessionTimeoutMillis, other.initialWaitForSessionTimeoutMillis) && Objects.equals(this.loopFrequency, other.loopFrequency) && Objects.equals(this.keepAliveIntervalMinutes, other.keepAliveIntervalMinutes) - && Objects.equals(this.removeInactiveSessionAfter, other.removeInactiveSessionAfter); + && Objects.equals(this.removeInactiveSessionAfter, other.removeInactiveSessionAfter) + && Objects.equals(this.autoDetectDialect, other.autoDetectDialect); } @Override @@ -104,7 +107,8 @@ public int hashCode() { this.initialWaitForSessionTimeoutMillis, this.loopFrequency, this.keepAliveIntervalMinutes, - this.removeInactiveSessionAfter); + this.removeInactiveSessionAfter, + this.autoDetectDialect); } public Builder toBuilder() { @@ -163,6 +167,10 @@ public boolean isBlockIfPoolExhausted() { return actionOnExhaustion == ActionOnExhaustion.BLOCK; } + public boolean isAutoDetectDialect() { + return autoDetectDialect; + } + @VisibleForTesting long getInitialWaitForSessionTimeoutMillis() { return initialWaitForSessionTimeoutMillis; @@ -220,6 +228,7 @@ public static class Builder { private long loopFrequency = 10 * 1000L; private int keepAliveIntervalMinutes = 30; private Duration removeInactiveSessionAfter = Duration.ofMinutes(55L); + private boolean autoDetectDialect = false; public Builder() {} @@ -237,6 +246,7 @@ private Builder(SessionPoolOptions options) { this.loopFrequency = options.loopFrequency; this.keepAliveIntervalMinutes = options.keepAliveIntervalMinutes; this.removeInactiveSessionAfter = options.removeInactiveSessionAfter; + this.autoDetectDialect = options.autoDetectDialect; } /** @@ -327,6 +337,24 @@ public Builder setBlockIfPoolExhausted() { return this; } + /** + * Sets whether the client should automatically execute a background query to detect the dialect + * that is used by the database or not. Set this option to true if you do not know what the + * dialect of the database will be. + * + *

Note that you can always call {@link DatabaseClient#getDialect()} to get the dialect of a + * database regardless of this setting, but by setting this to true, the value will be + * pre-populated and cached in the client. + * + * @param autoDetectDialect Whether the client should automatically execute a background query + * to detect the dialect of the underlying database + * @return this builder for chaining + */ + public Builder setAutoDetectDialect(boolean autoDetectDialect) { + this.autoDetectDialect = autoDetectDialect; + return this; + } + /** * The initial number of milliseconds to wait for a session to become available when one is * requested. The session pool will keep retrying to get a session, and the timeout will be diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 14f07314e8f..65de63b720f 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -125,7 +125,6 @@ public class SpannerOptions extends ServiceOptions { private final CallCredentialsProvider callCredentialsProvider; private final CloseableExecutorProvider asyncExecutorProvider; private final String compressorName; - private final Dialect dialect; /** * Interface that can be used to provide {@link CallCredentials} instead of {@link Credentials} to @@ -593,7 +592,6 @@ private SpannerOptions(Builder builder) { callCredentialsProvider = builder.callCredentialsProvider; asyncExecutorProvider = builder.asyncExecutorProvider; compressorName = builder.compressorName; - dialect = builder.dialect; } /** @@ -693,7 +691,6 @@ public static class Builder private CloseableExecutorProvider asyncExecutorProvider; private String compressorName; private String emulatorHost = System.getenv("SPANNER_EMULATOR_HOST"); - private Dialect dialect = Dialect.GOOGLE_STANDARD_SQL; private Builder() { // Manually set retry and polling settings that work. @@ -748,7 +745,6 @@ private Builder() { this.channelProvider = options.channelProvider; this.channelConfigurator = options.channelConfigurator; this.interceptorProvider = options.interceptorProvider; - this.dialect = options.dialect; } @Override @@ -779,7 +775,6 @@ protected Set getAllowedClientLibTokens() { *

  • {@link #setHost(String)} *
  • {@link #setNumChannels(int)} *
  • {@link #setInterceptorProvider(GrpcInterceptorProvider)} - *
  • {@link #setDialect(Dialect)} *
  • {@link #setHeaderProvider(com.google.api.gax.rpc.HeaderProvider)} * */ @@ -1139,16 +1134,6 @@ public Builder setEmulatorHost(String emulatorHost) { return this; } - /** - * Sets the {@link Dialect} to use with Cloud Spanner. The default is {@link - * Dialect#GOOGLE_STANDARD_SQL}. - */ - public Builder setDialect(Dialect dialect) { - Preconditions.checkNotNull(dialect); - this.dialect = dialect; - return this; - } - @SuppressWarnings("rawtypes") @Override public SpannerOptions build() { @@ -1276,10 +1261,6 @@ public String getCompressorName() { return compressorName; } - public Dialect getDialect() { - return dialect; - } - /** Returns the default query options to use for the specific database. */ public QueryOptions getDefaultQueryOptions(DatabaseId databaseId) { // Use the specific query options for the database if any have been specified. These have diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java index 9cea3810a98..3f3bd218ef2 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java @@ -83,7 +83,7 @@ private LeakedConnectionException() { private volatile LeakedConnectionException leakedException = new LeakedConnectionException(); private final SpannerPool spannerPool; - private final AbstractStatementParser parser; + private AbstractStatementParser statementParser; /** * The {@link ConnectionStatementExecutor} is responsible for translating parsed {@link * ClientSideStatement}s into actual method calls on this {@link ConnectionImpl}. I.e. the {@link @@ -221,7 +221,6 @@ static UnitOfWorkType of(TransactionMode transactionMode) { this.statementExecutor = new StatementExecutor(options.getStatementExecutionInterceptors()); this.spannerPool = SpannerPool.INSTANCE; this.options = options; - this.parser = AbstractStatementParser.getInstance(options.getDialect()); this.spanner = spannerPool.getSpanner(options, this); if (options.isAutoConfigEmulator()) { EmulatorUtil.maybeCreateInstanceAndDatabase(spanner, options.getDatabaseId()); @@ -251,7 +250,6 @@ static UnitOfWorkType of(TransactionMode transactionMode) { this.statementExecutor = new StatementExecutor(Collections.emptyList()); this.spannerPool = spannerPool; this.options = options; - this.parser = AbstractStatementParser.getInstance(options.getDialect()); this.spanner = spannerPool.getSpanner(options, this); this.ddlClient = ddlClient; this.dbClient = dbClient; @@ -269,6 +267,13 @@ private DdlClient createDdlClient() { .build(); } + private AbstractStatementParser getStatementParser() { + if (this.statementParser == null) { + this.statementParser = AbstractStatementParser.getInstance(dbClient.getDialect()); + } + return this.statementParser; + } + @Override public void close() { try { @@ -324,7 +329,7 @@ LeakedConnectionException getLeakedException() { @Override public Dialect getDialect() { - return options.getDialect(); + return dbClient.getDialect(); } @Override @@ -812,7 +817,7 @@ private ApiFuture endCurrentTransactionAsync(EndTransactionMethod endTrans public StatementResult execute(Statement statement) { Preconditions.checkNotNull(statement); ConnectionPreconditions.checkState(!isClosed(), CLOSED_ERROR_MSG); - ParsedStatement parsedStatement = parser.parse(statement, this.queryOptions); + ParsedStatement parsedStatement = getStatementParser().parse(statement, this.queryOptions); switch (parsedStatement.getType()) { case CLIENT_SIDE: return parsedStatement @@ -837,7 +842,7 @@ public StatementResult execute(Statement statement) { public AsyncStatementResult executeAsync(Statement statement) { Preconditions.checkNotNull(statement); ConnectionPreconditions.checkState(!isClosed(), CLOSED_ERROR_MSG); - ParsedStatement parsedStatement = parser.parse(statement, this.queryOptions); + ParsedStatement parsedStatement = getStatementParser().parse(statement, this.queryOptions); switch (parsedStatement.getType()) { case CLIENT_SIDE: return AsyncStatementResultImpl.of( @@ -885,7 +890,7 @@ private ResultSet parseAndExecuteQuery( Preconditions.checkNotNull(query); Preconditions.checkNotNull(analyzeMode); ConnectionPreconditions.checkState(!isClosed(), CLOSED_ERROR_MSG); - ParsedStatement parsedStatement = parser.parse(query, this.queryOptions); + ParsedStatement parsedStatement = getStatementParser().parse(query, this.queryOptions); if (parsedStatement.isQuery()) { switch (parsedStatement.getType()) { case CLIENT_SIDE: @@ -910,7 +915,7 @@ private AsyncResultSet parseAndExecuteQueryAsync( Statement query, AnalyzeMode analyzeMode, QueryOption... options) { Preconditions.checkNotNull(query); ConnectionPreconditions.checkState(!isClosed(), CLOSED_ERROR_MSG); - ParsedStatement parsedStatement = parser.parse(query, this.queryOptions); + ParsedStatement parsedStatement = getStatementParser().parse(query, this.queryOptions); if (parsedStatement.isQuery()) { switch (parsedStatement.getType()) { case CLIENT_SIDE: @@ -938,7 +943,7 @@ private AsyncResultSet parseAndExecuteQueryAsync( public long executeUpdate(Statement update) { Preconditions.checkNotNull(update); ConnectionPreconditions.checkState(!isClosed(), CLOSED_ERROR_MSG); - ParsedStatement parsedStatement = parser.parse(update); + ParsedStatement parsedStatement = getStatementParser().parse(update); if (parsedStatement.isUpdate()) { switch (parsedStatement.getType()) { case UPDATE: @@ -958,7 +963,7 @@ public long executeUpdate(Statement update) { public ApiFuture executeUpdateAsync(Statement update) { Preconditions.checkNotNull(update); ConnectionPreconditions.checkState(!isClosed(), CLOSED_ERROR_MSG); - ParsedStatement parsedStatement = parser.parse(update); + ParsedStatement parsedStatement = getStatementParser().parse(update); if (parsedStatement.isUpdate()) { switch (parsedStatement.getType()) { case UPDATE: @@ -982,7 +987,7 @@ public long[] executeBatchUpdate(Iterable updates) { // Check that there are only DML statements in the input. List parsedStatements = new LinkedList<>(); for (Statement update : updates) { - ParsedStatement parsedStatement = parser.parse(update); + ParsedStatement parsedStatement = getStatementParser().parse(update); switch (parsedStatement.getType()) { case UPDATE: parsedStatements.add(parsedStatement); @@ -1008,7 +1013,7 @@ public ApiFuture executeBatchUpdateAsync(Iterable updates) { // Check that there are only DML statements in the input. List parsedStatements = new LinkedList<>(); for (Statement update : updates) { - ParsedStatement parsedStatement = parser.parse(update); + ParsedStatement parsedStatement = getStatementParser().parse(update); switch (parsedStatement.getType()) { case UPDATE: parsedStatements.add(parsedStatement); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java index eb6a430f6d0..e4f3cfdbee2 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java @@ -25,7 +25,6 @@ import com.google.cloud.NoCredentials; import com.google.cloud.ServiceOptions; import com.google.cloud.spanner.DatabaseId; -import com.google.cloud.spanner.Dialect; import com.google.cloud.spanner.ErrorCode; import com.google.cloud.spanner.Options.RpcPriority; import com.google.cloud.spanner.SessionPoolOptions; @@ -166,7 +165,6 @@ public String[] getValidValues() { private static final RpcPriority DEFAULT_RPC_PRIORITY = null; private static final boolean DEFAULT_RETURN_COMMIT_STATS = false; private static final boolean DEFAULT_LENIENT = false; - private static final String DEFAULT_DIALECT = Dialect.GOOGLE_STANDARD_SQL.name(); private static final String PLAIN_TEXT_PROTOCOL = "http:"; private static final String HOST_PROTOCOL = "https:"; @@ -519,7 +517,6 @@ public static Builder newBuilder() { private final Integer maxSessions; private final String userAgent; private final QueryOptions queryOptions; - private final Dialect dialect; private final boolean returnCommitStats; private final boolean autoConfigEmulator; private final RpcPriority rpcPriority; @@ -563,14 +560,6 @@ private ConnectionOptions(Builder builder) { queryOptionsBuilder.setOptimizerStatisticsPackage(parseOptimizerStatisticsPackage(this.uri)); this.queryOptions = queryOptionsBuilder.build(); this.returnCommitStats = parseReturnCommitStats(this.uri); - - try { - this.dialect = Dialect.fromName(parseDialect(uri)); - } catch (IllegalArgumentException e) { - throw SpannerExceptionFactory.newSpannerException( - ErrorCode.INVALID_ARGUMENT, e.getMessage(), e); - } - this.autoConfigEmulator = parseAutoConfigEmulator(this.uri); this.usePlainText = this.autoConfigEmulator || parseUsePlainText(this.uri); this.host = determineHost(matcher, autoConfigEmulator, usePlainText); @@ -622,6 +611,7 @@ private ConnectionOptions(Builder builder) { builder.sessionPoolOptions == null ? SessionPoolOptions.newBuilder() : builder.sessionPoolOptions.toBuilder(); + sessionPoolOptionsBuilder.setAutoDetectDialect(true); if (this.minSessions != null) { sessionPoolOptionsBuilder.setMinSessions(this.minSessions); } @@ -629,8 +619,10 @@ private ConnectionOptions(Builder builder) { sessionPoolOptionsBuilder.setMaxSessions(this.maxSessions); } this.sessionPoolOptions = sessionPoolOptionsBuilder.build(); - } else { + } else if (builder.sessionPoolOptions != null) { this.sessionPoolOptions = builder.sessionPoolOptions; + } else { + this.sessionPoolOptions = SessionPoolOptions.newBuilder().setAutoDetectDialect(true).build(); } } @@ -780,12 +772,6 @@ static RpcPriority parseRPCPriority(String uri) { return value != null ? RpcPriority.valueOf(value) : DEFAULT_RPC_PRIORITY; } - @VisibleForTesting - static String parseDialect(String uri) { - String value = parseUriProperty(uri, DIALECT_PROPERTY_NAME); - return value != null ? value.toUpperCase() : DEFAULT_DIALECT; - } - @VisibleForTesting static String parseUriProperty(String uri, String property) { Pattern pattern = Pattern.compile(String.format("(?is)(?:;|\\?)%s=(.*?)(?:;|$)", property)); @@ -970,11 +956,6 @@ public String getWarnings() { return warnings; } - /** The dialect to use for connections created by this {@link ConnectionOptions}. */ - public Dialect getDialect() { - return dialect; - } - /** Use http instead of https. Only valid for (local) test servers. */ boolean isUsePlainText() { return usePlainText; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerPool.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerPool.java index 479b10df2e5..2712143da7a 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerPool.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerPool.java @@ -17,7 +17,6 @@ package com.google.cloud.spanner.connection; import com.google.cloud.NoCredentials; -import com.google.cloud.spanner.Dialect; import com.google.cloud.spanner.ErrorCode; import com.google.cloud.spanner.SessionPoolOptions; import com.google.cloud.spanner.Spanner; @@ -153,7 +152,6 @@ static class SpannerPoolKey { private final Integer numChannels; private final boolean usePlainText; private final String userAgent; - private final Dialect dialect; @VisibleForTesting static SpannerPoolKey of(ConnectionOptions options) { @@ -171,7 +169,6 @@ private SpannerPoolKey(ConnectionOptions options) { this.numChannels = options.getNumChannels(); this.usePlainText = options.isUsePlainText(); this.userAgent = options.getUserAgent(); - this.dialect = options.getDialect(); } @Override @@ -186,8 +183,7 @@ public boolean equals(Object o) { && Objects.equals(this.sessionPoolOptions, other.sessionPoolOptions) && Objects.equals(this.numChannels, other.numChannels) && Objects.equals(this.usePlainText, other.usePlainText) - && Objects.equals(this.userAgent, other.userAgent) - && Objects.equals(this.dialect, other.dialect); + && Objects.equals(this.userAgent, other.userAgent); } @Override @@ -199,8 +195,7 @@ public int hashCode() { this.sessionPoolOptions, this.numChannels, this.usePlainText, - this.userAgent, - this.dialect); + this.userAgent); } } @@ -326,8 +321,7 @@ Spanner createSpanner(SpannerPoolKey key, ConnectionOptions options) { .setClientLibToken(MoreObjects.firstNonNull(key.userAgent, CONNECTION_API_CLIENT_LIB_TOKEN)) .setHost(key.host) .setProjectId(key.projectId) - .setCredentials(options.getCredentials()) - .setDialect(options.getDialect()); + .setCredentials(options.getCredentials()); builder.setSessionPoolOption(key.sessionPoolOptions); if (key.numChannels != null) { builder.setNumChannels(key.numChannels); 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 21c9d3eaa77..c0e1e606402 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 @@ -2204,4 +2204,60 @@ public Long apply(TransactionContext transactionContext) { () -> client.readWriteTransaction().run(tx -> function.apply(tx))); assertTrue(exception.getMessage().contains("Context has been closed")); } + + @Test + public void testGetDialectDefault() { + DatabaseClient client = + spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + assertEquals(Dialect.GOOGLE_STANDARD_SQL, client.getDialect()); + } + + @Test + public void testGetDialectDefaultPreloaded() { + 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)); + assertEquals(Dialect.GOOGLE_STANDARD_SQL, client.getDialect()); + } + } + + @Test + public void testGetDialectPostgreSQL() { + mockSpanner.putStatementResult(StatementResult.detectDialectResult(Dialect.POSTGRESQL)); + try { + DatabaseClient client = + spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + assertEquals(Dialect.POSTGRESQL, client.getDialect()); + } finally { + mockSpanner.putStatementResult( + StatementResult.detectDialectResult(Dialect.GOOGLE_STANDARD_SQL)); + } + } + + @Test + public void testGetDialectPostgreSQLPreloaded() { + mockSpanner.putStatementResult(StatementResult.detectDialectResult(Dialect.POSTGRESQL)); + 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)); + assertEquals(Dialect.POSTGRESQL, client.getDialect()); + } finally { + mockSpanner.putStatementResult( + StatementResult.detectDialectResult(Dialect.GOOGLE_STANDARD_SQL)); + } + } } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/MockSpannerServiceImpl.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/MockSpannerServiceImpl.java index a307e23db22..feb4e680a50 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/MockSpannerServiceImpl.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/MockSpannerServiceImpl.java @@ -33,6 +33,7 @@ import com.google.protobuf.ByteString; import com.google.protobuf.Duration; import com.google.protobuf.Empty; +import com.google.protobuf.ListValue; import com.google.protobuf.Timestamp; import com.google.protobuf.Value.KindCase; import com.google.rpc.Code; @@ -271,6 +272,32 @@ public static StatementResult exception(Statement statement, StatusRuntimeExcept return new StatementResult(statement, exception); } + /** Creates a result for the query that detects the dialect that is used for the database. */ + public static StatementResult detectDialectResult(Dialect resultDialect) { + return StatementResult.query( + SessionPool.DETERMINE_DIALECT_STATEMENT, + ResultSet.newBuilder() + .setMetadata( + ResultSetMetadata.newBuilder() + .setRowType( + StructType.newBuilder() + .addFields( + Field.newBuilder() + .setName("DIALECT") + .setType(Type.newBuilder().setCode(TypeCode.STRING).build()) + .build()) + .build()) + .build()) + .addRows( + ListValue.newBuilder() + .addValues( + com.google.protobuf.Value.newBuilder() + .setStringValue(resultDialect.toString()) + .build()) + .build()) + .build()); + } + private static class KeepLastElementDeque extends LinkedList { private static KeepLastElementDeque singleton(E item) { return new KeepLastElementDeque<>(Collections.singleton(item)); @@ -525,6 +552,12 @@ private static void checkStreamException( private final Random random = new Random(); private double abortProbability = 0.0010D; + /** + * Flip this switch to true if you want the {@link SessionPool#DETERMINE_DIALECT_STATEMENT} + * statement to be included in the recorded requests on the mock server. It is ignored by default + * to prevent tests that do not expect this request to suddenly start failing. + */ + private boolean includeDetermineDialectStatementInRequests = false; private final Object lock = new Object(); private Deque requests = new ConcurrentLinkedDeque<>(); @@ -568,6 +601,10 @@ private static void checkStreamException( private SimulatedExecutionTime rollbackExecutionTime = NO_EXECUTION_TIME; private SimulatedExecutionTime streamingReadExecutionTime = NO_EXECUTION_TIME; + public MockSpannerServiceImpl() { + putStatementResult(StatementResult.detectDialectResult(Dialect.GOOGLE_STANDARD_SQL)); + } + private String generateSessionName(String database) { return String.format("%s/sessions/%s", database, UUID.randomUUID().toString()); } @@ -663,6 +700,15 @@ public void setAbortProbability(double probability) { this.abortProbability = probability; } + /** + * Set this to true if you want the {@link SessionPool#DETERMINE_DIALECT_STATEMENT} statement to + * be included in the recorded requests on the mock server. It is ignored by default to prevent + * tests that do not expect this request to suddenly start failing. + */ + public void setIncludeDetermineDialectStatementInRequests(boolean include) { + this.includeDetermineDialectStatementInRequests = include; + } + /** * Instruct the mock server to abort the specified transaction. Use this method to test handling * of {@link AbortedException} in your code. @@ -1097,7 +1143,10 @@ public void executeBatchDml( @Override public void executeStreamingSql( ExecuteSqlRequest request, StreamObserver responseObserver) { - requests.add(request); + if (includeDetermineDialectStatementInRequests + || !request.getSql().equals(SessionPool.DETERMINE_DIALECT_STATEMENT.getSql())) { + requests.add(request); + } Preconditions.checkNotNull(request.getSession()); Session session = sessions.get(request.getSession()); if (session == null) { diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpanTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpanTest.java index dec7e59c4b1..84074787655 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpanTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpanTest.java @@ -106,6 +106,17 @@ public static void startStaticServer() throws Exception { "This test is only supported on JDK11 and lower", JavaVersionUtil.getJavaMajorVersion() < 12); + // Use a little reflection to set the test tracer. + // This is not possible in Java 12 and later. + java.lang.reflect.Field field = Tracing.class.getDeclaredField("traceComponent"); + field.setAccessible(true); + java.lang.reflect.Field modifiersField = + java.lang.reflect.Field.class.getDeclaredField("modifiers"); + modifiersField.setAccessible(true); + // Remove the final modifier from the 'traceComponent' field. + modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL); + field.set(null, failOnOverkillTraceComponent); + mockSpanner = new MockSpannerServiceImpl(); mockSpanner.setAbortProbability(0.0D); // We don't want any unpredictable aborted transactions. mockSpanner.putStatementResult(StatementResult.update(UPDATE_STATEMENT, UPDATE_COUNT)); @@ -124,17 +135,6 @@ public static void startStaticServer() throws Exception { .build() .start(); channelProvider = LocalChannelProvider.create(uniqueName); - - // Use a little reflection to set the test tracer. - // This is not possible in Java 12 and later. - java.lang.reflect.Field field = Tracing.class.getDeclaredField("traceComponent"); - field.setAccessible(true); - java.lang.reflect.Field modifiersField = - java.lang.reflect.Field.class.getDeclaredField("modifiers"); - modifiersField.setAccessible(true); - // Remove the final modifier from the 'traceComponent' field. - modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL); - field.set(null, failOnOverkillTraceComponent); } @AfterClass diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index 80a542c5305..fc82954ad40 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -81,7 +81,6 @@ public void defaultBuilder() { } assertThat(options.getPrefetchChunks()).isEqualTo(4); assertThat(options.getSessionLabels()).isNull(); - assertThat(options.getDialect()).isEqualTo(Dialect.GOOGLE_STANDARD_SQL); } @Test @@ -96,13 +95,11 @@ public void builder() { .setProjectId(projectId) .setPrefetchChunks(2) .setSessionLabels(labels) - .setDialect(Dialect.POSTGRESQL) .build(); assertThat(options.getHost()).isEqualTo(host); assertThat(options.getProjectId()).isEqualTo(projectId); assertThat(options.getPrefetchChunks()).isEqualTo(2); assertThat(options.getSessionLabels()).containsExactlyEntriesIn(labels); - assertThat(options.getDialect()).isEqualTo(Dialect.POSTGRESQL); } @Test diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AutocommitDmlModeTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AutocommitDmlModeTest.java index 78834f69c89..2acdf6f3d15 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AutocommitDmlModeTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AutocommitDmlModeTest.java @@ -26,6 +26,7 @@ import com.google.cloud.NoCredentials; import com.google.cloud.spanner.DatabaseClient; +import com.google.cloud.spanner.Dialect; import com.google.cloud.spanner.Spanner; import com.google.cloud.spanner.Statement; import com.google.cloud.spanner.TransactionContext; @@ -48,6 +49,7 @@ public class AutocommitDmlModeTest { @SuppressWarnings("unchecked") private ConnectionImpl createConnection(ConnectionOptions options) { dbClient = mock(DatabaseClient.class); + when(dbClient.getDialect()).thenReturn(Dialect.GOOGLE_STANDARD_SQL); txContext = mock(TransactionContext.class); Spanner spanner = mock(Spanner.class); SpannerPool spannerPool = mock(SpannerPool.class); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionAsyncApiAbortedTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionAsyncApiAbortedTest.java index 8aac69acdf8..97d745edd7f 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionAsyncApiAbortedTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionAsyncApiAbortedTest.java @@ -50,6 +50,7 @@ import java.util.concurrent.atomic.AtomicInteger; import org.junit.After; import org.junit.AfterClass; +import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; @@ -107,6 +108,13 @@ public static void stopExecutor() { multiThreadedExecutor.shutdown(); } + @Before + public void setup() { + try (Connection connection = createConnection()) { + connection.getDialect(); + } + } + @After public void reset() { mockSpanner.removeAllExecutionTimes(); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionAsyncApiTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionAsyncApiTest.java index ff409e88c63..21fe086d4f8 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionAsyncApiTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionAsyncApiTest.java @@ -54,6 +54,7 @@ import java.util.concurrent.atomic.AtomicInteger; import org.junit.After; import org.junit.AfterClass; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -78,6 +79,13 @@ public static void stopExecutor() { executor.shutdown(); } + @Before + public void setup() { + try (Connection connection = createConnection()) { + connection.getDialect(); + } + } + @After public void reset() { mockSpanner.removeAllExecutionTimes(); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionImplTest.java index f6afee6da2d..6d04caa8c26 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionImplTest.java @@ -227,6 +227,7 @@ public static ConnectionImpl createConnection(final ConnectionOptions options) { .thenReturn(spanner); DdlClient ddlClient = createDefaultMockDdlClient(); DatabaseClient dbClient = mock(DatabaseClient.class); + when(dbClient.getDialect()).thenReturn(Dialect.GOOGLE_STANDARD_SQL); ReadOnlyTransaction singleUseReadOnlyTx = mock(ReadOnlyTransaction.class); ResultSet mockResultSetWithStats = createSelect1MockResultSet(); @@ -1362,10 +1363,10 @@ public void testAddRemoveTransactionRetryListener() { @Test public void testMergeQueryOptions() { ConnectionOptions connectionOptions = mock(ConnectionOptions.class); - when(connectionOptions.getDialect()).thenReturn(Dialect.GOOGLE_STANDARD_SQL); SpannerPool spannerPool = mock(SpannerPool.class); DdlClient ddlClient = mock(DdlClient.class); DatabaseClient dbClient = mock(DatabaseClient.class); + when(dbClient.getDialect()).thenReturn(Dialect.GOOGLE_STANDARD_SQL); final UnitOfWork unitOfWork = mock(UnitOfWork.class); when(unitOfWork.executeQueryAsync( any(ParsedStatement.class), any(AnalyzeMode.class), Mockito.any())) @@ -1466,10 +1467,10 @@ UnitOfWork getCurrentUnitOfWorkOrStartNewUnitOfWork() { public void testStatementTagAlwaysAllowed() { ConnectionOptions connectionOptions = mock(ConnectionOptions.class); when(connectionOptions.isAutocommit()).thenReturn(true); - when(connectionOptions.getDialect()).thenReturn(Dialect.GOOGLE_STANDARD_SQL); SpannerPool spannerPool = mock(SpannerPool.class); DdlClient ddlClient = mock(DdlClient.class); DatabaseClient dbClient = mock(DatabaseClient.class); + when(dbClient.getDialect()).thenReturn(Dialect.GOOGLE_STANDARD_SQL); final UnitOfWork unitOfWork = mock(UnitOfWork.class); when(unitOfWork.executeQueryAsync( any(ParsedStatement.class), any(AnalyzeMode.class), Mockito.any())) @@ -1509,10 +1510,10 @@ UnitOfWork getCurrentUnitOfWorkOrStartNewUnitOfWork() { public void testTransactionTagAllowedInTransaction() { ConnectionOptions connectionOptions = mock(ConnectionOptions.class); when(connectionOptions.isAutocommit()).thenReturn(false); - when(connectionOptions.getDialect()).thenReturn(Dialect.GOOGLE_STANDARD_SQL); SpannerPool spannerPool = mock(SpannerPool.class); DdlClient ddlClient = mock(DdlClient.class); DatabaseClient dbClient = mock(DatabaseClient.class); + when(dbClient.getDialect()).thenReturn(Dialect.GOOGLE_STANDARD_SQL); try (ConnectionImpl connection = new ConnectionImpl(connectionOptions, spannerPool, ddlClient, dbClient)) { assertFalse(connection.isAutocommit()); @@ -1550,10 +1551,10 @@ public void testTransactionTagAllowedInTransaction() { public void testTransactionTagNotAllowedWithoutTransaction() { ConnectionOptions connectionOptions = mock(ConnectionOptions.class); when(connectionOptions.isAutocommit()).thenReturn(true); - when(connectionOptions.getDialect()).thenReturn(Dialect.GOOGLE_STANDARD_SQL); SpannerPool spannerPool = mock(SpannerPool.class); DdlClient ddlClient = mock(DdlClient.class); DatabaseClient dbClient = mock(DatabaseClient.class); + when(dbClient.getDialect()).thenReturn(Dialect.GOOGLE_STANDARD_SQL); try (ConnectionImpl connection = new ConnectionImpl(connectionOptions, spannerPool, ddlClient, dbClient)) { assertTrue(connection.isAutocommit()); @@ -1571,10 +1572,10 @@ public void testTransactionTagNotAllowedWithoutTransaction() { public void testTransactionTagNotAllowedAfterTransactionStarted() { ConnectionOptions connectionOptions = mock(ConnectionOptions.class); when(connectionOptions.isAutocommit()).thenReturn(false); - when(connectionOptions.getDialect()).thenReturn(Dialect.GOOGLE_STANDARD_SQL); SpannerPool spannerPool = mock(SpannerPool.class); DdlClient ddlClient = mock(DdlClient.class); DatabaseClient dbClient = mock(DatabaseClient.class); + when(dbClient.getDialect()).thenReturn(Dialect.GOOGLE_STANDARD_SQL); final UnitOfWork unitOfWork = mock(UnitOfWork.class); // Indicate that a transaction has been started. when(unitOfWork.getState()).thenReturn(UnitOfWorkState.STARTED); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java index e8758c36d03..837783ee18b 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java @@ -27,7 +27,6 @@ import com.google.auth.oauth2.GoogleCredentials; import com.google.auth.oauth2.ServiceAccountCredentials; import com.google.cloud.NoCredentials; -import com.google.cloud.spanner.Dialect; import com.google.cloud.spanner.ErrorCode; import com.google.cloud.spanner.SpannerException; import com.google.cloud.spanner.SpannerOptions; @@ -577,62 +576,6 @@ public void testSetCredentialsAndEncodedCredentials() throws Exception { "Cannot specify both a credentials URL and encoded credentials. Only set one of the properties."); } - @Test - public void testSetDialect() { - ConnectionOptions options = - ConnectionOptions.newBuilder() - .setCredentials(NoCredentials.getInstance()) - .setUri( - "cloudspanner:/projects/test-project-123/instances/test-instance/databases/test-database") - .build(); - assertThat(options.getDialect()).isEqualTo(Dialect.GOOGLE_STANDARD_SQL); - - ConnectionOptions optionsSpanner = - ConnectionOptions.newBuilder() - .setCredentials(NoCredentials.getInstance()) - .setUri( - "cloudspanner:/projects/test-project-123/instances/test-instance/databases/test-database?dialect=GOOGLE_STANDARD_SQL") - .build(); - assertThat(optionsSpanner.getDialect()).isEqualTo(Dialect.GOOGLE_STANDARD_SQL); - - ConnectionOptions optionsPostgreSQL = - ConnectionOptions.newBuilder() - .setCredentials(NoCredentials.getInstance()) - .setUri( - "cloudspanner:/projects/test-project-123/instances/test-instance/databases/test-database?dialect=POSTGRESQL") - .build(); - assertThat(optionsPostgreSQL.getDialect()).isEqualTo(Dialect.POSTGRESQL); - - ConnectionOptions optionsSpannerLC = - ConnectionOptions.newBuilder() - .setCredentials(NoCredentials.getInstance()) - .setUri( - "cloudspanner:/projects/test-project-123/instances/test-instance/databases/test-database?dialect=google_standard_sql") - .build(); - assertThat(optionsSpannerLC.getDialect()).isEqualTo(Dialect.GOOGLE_STANDARD_SQL); - - ConnectionOptions optionsPostgreSQLLC = - ConnectionOptions.newBuilder() - .setCredentials(NoCredentials.getInstance()) - .setUri( - "cloudspanner:/projects/test-project-123/instances/test-instance/databases/test-database?dialect=postgresql") - .build(); - assertThat(optionsPostgreSQLLC.getDialect()).isEqualTo(Dialect.POSTGRESQL); - - try { - ConnectionOptions.newBuilder() - .setCredentials(NoCredentials.getInstance()) - .setUri( - "cloudspanner:/projects/test-project-123/instances/test-instance/databases/test-database?dialect=POSTGRES") - .build(); - fail("missing expected exception"); - } catch (SpannerException e) { - assertThat(e.getErrorCode()).isEqualTo(ErrorCode.INVALID_ARGUMENT); - assertThat(e.getMessage()) - .contains("Dialect must be one of [GOOGLE_STANDARD_SQL, POSTGRESQL]"); - } - } - @Test public void testExternalChannelProvider() { String baseUri = diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ReadOnlyStalenessTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ReadOnlyStalenessTest.java index 51722b00d40..fa155ea33bc 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ReadOnlyStalenessTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ReadOnlyStalenessTest.java @@ -27,6 +27,7 @@ import com.google.cloud.NoCredentials; import com.google.cloud.Timestamp; import com.google.cloud.spanner.DatabaseClient; +import com.google.cloud.spanner.Dialect; import com.google.cloud.spanner.ReadOnlyTransaction; import com.google.cloud.spanner.ResultSet; import com.google.cloud.spanner.Spanner; @@ -46,6 +47,7 @@ public class ReadOnlyStalenessTest { private final DatabaseClient dbClient = mock(DatabaseClient.class); private ConnectionImpl createConnection(ConnectionOptions options) { + when(dbClient.getDialect()).thenReturn(Dialect.GOOGLE_STANDARD_SQL); Spanner spanner = mock(Spanner.class); SpannerPool spannerPool = mock(SpannerPool.class); when(spannerPool.getSpanner(any(ConnectionOptions.class), any(ConnectionImpl.class))) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SpannerPoolTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SpannerPoolTest.java index ab5a345909f..59c9065e41e 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SpannerPoolTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SpannerPoolTest.java @@ -27,6 +27,8 @@ import com.google.auth.Credentials; import com.google.cloud.NoCredentials; +import com.google.cloud.spanner.DatabaseClient; +import com.google.cloud.spanner.Dialect; import com.google.cloud.spanner.ErrorCode; import com.google.cloud.spanner.SessionPoolOptions; import com.google.cloud.spanner.Spanner; @@ -271,14 +273,22 @@ public void testCloseSpanner() { @Test public void testLeakedConnection() { + attachLogCapturer(); ConnectionOptions options = ConnectionOptions.newBuilder() .setCredentials(NoCredentials.getInstance()) - .setSessionPoolOptions(SessionPoolOptions.newBuilder().setMinSessions(0).build()) + .setSessionPoolOptions( + SessionPoolOptions.newBuilder() + .setMinSessions(0) + .setAutoDetectDialect(false) + .build()) .setUri(URI) .build(); + DdlClient ddlClient = mock(DdlClient.class); + DatabaseClient dbClient = mock(DatabaseClient.class); + when(dbClient.getDialect()).thenReturn(Dialect.GOOGLE_STANDARD_SQL); // create an actual connection object but not in a try-with-resources block - Connection connection = options.getConnection(); + Connection connection = new ConnectionImpl(options, SpannerPool.INSTANCE, ddlClient, dbClient); // try to close the application which should fail try { ConnectionOptions.closeSpanner(); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/StatementTimeoutTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/StatementTimeoutTest.java index 08f47c0c5d4..af6190a8fd6 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/StatementTimeoutTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/StatementTimeoutTest.java @@ -54,6 +54,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import org.junit.After; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -105,6 +106,17 @@ ITConnection createConnection() { return createITConnection(options); } + @Before + public void setup() { + // Set up a connection and get the dialect to ensure that the auto-detect-dialect query has + // already been executed when any of the test cases start. This is necessary to ensure that any + // errors or slow execution times that are used by the different test cases are not applied to + // the query that is used for automatic dialect detection. + try (Connection connection = createConnection()) { + connection.getDialect(); + } + } + @After public void clearExecutionTimes() { mockSpanner.removeAllExecutionTimes(); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java index 4ed6c32c7bf..ab11e141cb1 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java @@ -400,7 +400,6 @@ private SpannerOptions createSpannerOptions() { String endpoint = address.getHostString() + ":" + server.getPort(); return SpannerOptions.newBuilder() .setProjectId("[PROJECT]") - .setDialect(dialect) // Set a custom channel configurator to allow http instead of https. .setChannelConfigurator( input -> {