diff --git a/.kokoro/presubmit/graalvm-native-17.cfg b/.kokoro/presubmit/graalvm-native-17.cfg index 56db68092d2..c2a88196e84 100644 --- a/.kokoro/presubmit/graalvm-native-17.cfg +++ b/.kokoro/presubmit/graalvm-native-17.cfg @@ -3,7 +3,7 @@ # Configure the docker image for kokoro-trampoline. env_vars: { key: "TRAMPOLINE_IMAGE" - value: "gcr.io/cloud-devrel-public-resources/graalvm_sdk_platform_b:3.27.0" + value: "gcr.io/cloud-devrel-public-resources/graalvm_sdk_platform_b:3.28.1" } env_vars: { diff --git a/.kokoro/presubmit/graalvm-native.cfg b/.kokoro/presubmit/graalvm-native.cfg index 6283184778e..94e00cbaa0a 100644 --- a/.kokoro/presubmit/graalvm-native.cfg +++ b/.kokoro/presubmit/graalvm-native.cfg @@ -3,7 +3,7 @@ # Configure the docker image for kokoro-trampoline. env_vars: { key: "TRAMPOLINE_IMAGE" - value: "gcr.io/cloud-devrel-public-resources/graalvm_sdk_platform_a:3.27.0" + value: "gcr.io/cloud-devrel-public-resources/graalvm_sdk_platform_a:3.28.1" } env_vars: { diff --git a/CHANGELOG.md b/CHANGELOG.md index c2b269ee2ef..cf6dc252e4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,36 @@ # Changelog +## [6.62.0](https://github.com/googleapis/java-spanner/compare/v6.61.0...v6.62.0) (2024-03-19) + + +### Features + +* Allow attempt direct path xds via env var ([#2950](https://github.com/googleapis/java-spanner/issues/2950)) ([247a15f](https://github.com/googleapis/java-spanner/commit/247a15f2b8b858143bc906e0619f95a017ffe5c3)) +* Next release from main branch is 6.56.0 ([#2929](https://github.com/googleapis/java-spanner/issues/2929)) ([66374b1](https://github.com/googleapis/java-spanner/commit/66374b1c4ed88e01ff60fb8e1b7409e5dbbcb811)) + + +### Bug Fixes + +* Return type of max commit delay option. ([#2953](https://github.com/googleapis/java-spanner/issues/2953)) ([6e937ab](https://github.com/googleapis/java-spanner/commit/6e937ab16d130e72d633979c1a76bf7b3edbe7b6)) + + +### Performance Improvements + +* Keep comments when searching for params ([#2951](https://github.com/googleapis/java-spanner/issues/2951)) ([b782725](https://github.com/googleapis/java-spanner/commit/b782725b92a2662c42ad35647b23009ad95a99a5)) + + +### Dependencies + +* Update dependency com.google.cloud:google-cloud-monitoring to v3.38.0 ([#2942](https://github.com/googleapis/java-spanner/issues/2942)) ([ba665bd](https://github.com/googleapis/java-spanner/commit/ba665bd483ba70f09770d92028355ad499003fed)) +* Update dependency com.google.cloud:google-cloud-trace to v2.37.0 ([#2944](https://github.com/googleapis/java-spanner/issues/2944)) ([b5e608e](https://github.com/googleapis/java-spanner/commit/b5e608ef001473ab5575f1619804b351053c57f2)) +* Update dependency com.google.cloud:sdk-platform-java-config to v3.28.1 ([#2952](https://github.com/googleapis/java-spanner/issues/2952)) ([1e45237](https://github.com/googleapis/java-spanner/commit/1e45237dd235484a6a279f71ae7e126727382f9c)) +* Update opentelemetry.version to v1.36.0 ([#2945](https://github.com/googleapis/java-spanner/issues/2945)) ([e70b035](https://github.com/googleapis/java-spanner/commit/e70b0357543d38b6e9265e04444cec494ebd6885)) + + +### Documentation + +* **samples:** Add tag to statement timeout sample ([#2931](https://github.com/googleapis/java-spanner/issues/2931)) ([2392afe](https://github.com/googleapis/java-spanner/commit/2392afed0d25266294e0ce11c6ae32d7307e6830)) + ## [6.61.0](https://github.com/googleapis/java-spanner/compare/v6.60.1...v6.61.0) (2024-03-04) diff --git a/README.md b/README.md index 1eb566d981b..4718890c54f 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ If you are using Maven with [BOM][libraries-bom], add this to your pom.xml file: com.google.cloud libraries-bom - 26.33.0 + 26.34.0 pom import @@ -50,7 +50,7 @@ If you are using Maven without the BOM, add this to your dependencies: If you are using Gradle 5.x or later, add this to your dependencies: ```Groovy -implementation platform('com.google.cloud:libraries-bom:26.33.0') +implementation platform('com.google.cloud:libraries-bom:26.34.0') implementation 'com.google.cloud:google-cloud-spanner' ``` diff --git a/google-cloud-spanner-bom/pom.xml b/google-cloud-spanner-bom/pom.xml index e60c1748158..552c09656aa 100644 --- a/google-cloud-spanner-bom/pom.xml +++ b/google-cloud-spanner-bom/pom.xml @@ -3,12 +3,12 @@ 4.0.0 com.google.cloud google-cloud-spanner-bom - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT pom com.google.cloud sdk-platform-java-config - 3.27.0 + 3.28.1 Google Cloud Spanner BOM @@ -53,43 +53,43 @@ com.google.cloud google-cloud-spanner - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT com.google.cloud google-cloud-spanner test-jar - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT com.google.api.grpc grpc-google-cloud-spanner-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT com.google.api.grpc grpc-google-cloud-spanner-admin-instance-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT com.google.api.grpc grpc-google-cloud-spanner-admin-database-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT com.google.api.grpc proto-google-cloud-spanner-admin-instance-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT com.google.api.grpc proto-google-cloud-spanner-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT com.google.api.grpc proto-google-cloud-spanner-admin-database-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT @@ -100,7 +100,7 @@ org.apache.maven.plugins maven-compiler-plugin - 3.12.1 + 3.13.0 1.8 1.8 diff --git a/google-cloud-spanner-executor/pom.xml b/google-cloud-spanner-executor/pom.xml index 46b9d92ecf1..17fbe99ec0c 100644 --- a/google-cloud-spanner-executor/pom.xml +++ b/google-cloud-spanner-executor/pom.xml @@ -5,14 +5,14 @@ 4.0.0 com.google.cloud google-cloud-spanner-executor - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT jar Google Cloud Spanner Executor com.google.cloud google-cloud-spanner-parent - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT @@ -164,7 +164,7 @@ maven-assembly-plugin - 3.6.0 + 3.7.1 assembly-descriptor.xml diff --git a/google-cloud-spanner/clirr-ignored-differences.xml b/google-cloud-spanner/clirr-ignored-differences.xml index eaf7637b0b9..5d3beb2e7b2 100644 --- a/google-cloud-spanner/clirr-ignored-differences.xml +++ b/google-cloud-spanner/clirr-ignored-differences.xml @@ -605,6 +605,14 @@ com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings getInstanceAdminStubSettings() + + + 7006 + com/google/cloud/spanner/Options + com.google.cloud.spanner.Options$ReadQueryUpdateTransactionOption maxCommitDelay(java.time.Duration) + com.google.cloud.spanner.Options$TransactionOption + + 7005 com/google/cloud/spanner/PartitionedDmlTransaction diff --git a/google-cloud-spanner/pom.xml b/google-cloud-spanner/pom.xml index a2a8c8e3d1e..d393523667c 100644 --- a/google-cloud-spanner/pom.xml +++ b/google-cloud-spanner/pom.xml @@ -3,7 +3,7 @@ 4.0.0 com.google.cloud google-cloud-spanner - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT jar Google Cloud Spanner https://github.com/googleapis/java-spanner @@ -11,12 +11,12 @@ com.google.cloud google-cloud-spanner-parent - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT google-cloud-spanner 0.31.1 - 1.35.0 + 1.36.0 com.google.cloud.spanner.GceTestEnvConfig projects/gcloud-devel/instances/spanner-testing-east1 gcloud-devel diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java index 4819e8bb1f7..3dbd0c1cda3 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java @@ -155,7 +155,7 @@ public static ReadQueryUpdateTransactionOption priority(RpcPriority priority) { return new PriorityOption(priority); } - public static ReadQueryUpdateTransactionOption maxCommitDelay(Duration maxCommitDelay) { + public static TransactionOption maxCommitDelay(Duration maxCommitDelay) { Preconditions.checkArgument(!maxCommitDelay.isNegative(), "maxCommitDelay should be positive"); return new MaxCommitDelayOption(maxCommitDelay); } @@ -272,8 +272,7 @@ void appendToOptions(Options options) { static final CommitStatsOption COMMIT_STATS_OPTION = new CommitStatsOption(); /** Option to request {@link MaxCommitDelayOption} for read/write transactions. */ - static final class MaxCommitDelayOption extends InternalOption - implements ReadQueryUpdateTransactionOption { + static final class MaxCommitDelayOption extends InternalOption implements TransactionOption { final Duration maxCommitDelay; MaxCommitDelayOption(Duration maxCommitDelay) { 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 cc24dd2ba0f..8058802a8fc 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 @@ -49,6 +49,7 @@ import com.google.api.gax.core.ExecutorProvider; import com.google.api.gax.rpc.ServerStream; import com.google.cloud.Timestamp; +import com.google.cloud.Tuple; import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.grpc.GrpcTransportOptions.ExecutorFactory; import com.google.cloud.spanner.Options.QueryOption; @@ -1750,6 +1751,13 @@ final class PoolMaintainer { */ @VisibleForTesting Instant lastExecutionTime; + /** + * The previous numSessionsAcquired seen by the maintainer. This is used to calculate the + * transactions per second, which again is used to determine whether to randomize the order of + * the session pool. + */ + private long prevNumSessionsAcquired; + boolean closed = false; @GuardedBy("lock") @@ -1793,6 +1801,12 @@ void maintainPool() { return; } running = true; + if (loopFrequency >= 1000L) { + SessionPool.this.transactionsPerSecond = + (SessionPool.this.numSessionsAcquired - prevNumSessionsAcquired) + / (loopFrequency / 1000L); + } + this.prevNumSessionsAcquired = SessionPool.this.numSessionsAcquired; } Instant currTime = clock.instant(); removeIdleSessions(currTime); @@ -1854,7 +1868,7 @@ private void keepAliveSessions(Instant currTime) { // Keep chugging till there is no session that needs to be kept alive. while (numSessionsToKeepAlive > 0) { - PooledSession sessionToKeepAlive = null; + Tuple sessionToKeepAlive; synchronized (lock) { sessionToKeepAlive = findSessionToKeepAlive(sessions, keepAliveThreshold, 0); } @@ -1862,10 +1876,10 @@ private void keepAliveSessions(Instant currTime) { break; } try { - logger.log(Level.FINE, "Keeping alive session " + sessionToKeepAlive.getName()); + logger.log(Level.FINE, "Keeping alive session " + sessionToKeepAlive.x().getName()); numSessionsToKeepAlive--; - sessionToKeepAlive.keepAlive(); - releaseSession(sessionToKeepAlive, false); + sessionToKeepAlive.x().keepAlive(); + releaseSession(sessionToKeepAlive); } catch (SpannerException e) { handleException(e, sessionToKeepAlive); } @@ -1994,6 +2008,7 @@ enum Position { private final SettableFuture dialect = SettableFuture.create(); private final String databaseRole; private final SessionClient sessionClient; + private final int numChannels; private final ScheduledExecutorService executor; private final ExecutorFactory executorFactory; @@ -2053,6 +2068,9 @@ enum Position { @GuardedBy("lock") private long numIdleSessionsRemoved = 0; + @GuardedBy("lock") + private long transactionsPerSecond = 0L; + @GuardedBy("lock") private long numLeakedSessionsRemoved = 0; @@ -2189,6 +2207,7 @@ private SessionPool( this.executorFactory = executorFactory; this.executor = executor; this.sessionClient = sessionClient; + this.numChannels = sessionClient.getSpanner().getOptions().getNumChannels(); this.clock = clock; this.initialReleasePosition = initialReleasePosition; this.poolMaintainer = new PoolMaintainer(); @@ -2314,11 +2333,11 @@ private boolean isClosed() { } } - private void handleException(SpannerException e, PooledSession session) { + private void handleException(SpannerException e, Tuple session) { if (isSessionNotFound(e)) { - invalidateSession(session); + invalidateSession(session.x()); } else { - releaseSession(session, false); + releaseSession(session); } } @@ -2342,7 +2361,7 @@ private void invalidateSession(PooledSession session) { } } - private PooledSession findSessionToKeepAlive( + private Tuple findSessionToKeepAlive( Queue queue, Instant keepAliveThreshold, int numAlreadyChecked) { int numChecked = 0; Iterator iterator = queue.iterator(); @@ -2352,7 +2371,7 @@ private PooledSession findSessionToKeepAlive( PooledSession session = iterator.next(); if (session.delegate.getLastUseTime().isBefore(keepAliveThreshold)) { iterator.remove(); - return session; + return Tuple.of(session, numChecked); } numChecked++; } @@ -2476,18 +2495,29 @@ private void maybeCreateSession() { } } - /** Releases a session back to the pool. This might cause one of the waiters to be unblocked. */ + private void releaseSession(Tuple sessionWithPosition) { + releaseSession(sessionWithPosition.x(), false, sessionWithPosition.y()); + } + private void releaseSession(PooledSession session, boolean isNewSession) { + releaseSession(session, isNewSession, null); + } + + /** Releases a session back to the pool. This might cause one of the waiters to be unblocked. */ + private void releaseSession( + PooledSession session, boolean isNewSession, @Nullable Integer position) { Preconditions.checkNotNull(session); synchronized (lock) { if (closureFuture != null) { return; } - if (waiters.size() == 0) { + if (waiters.isEmpty()) { // There are no pending waiters. - // Add to a random position if the head of the session pool already contains many sessions - // with the same channel as this one. - if (session.releaseToPosition == Position.FIRST && isUnbalanced(session)) { + // Add to a random position if the transactions per second is high or the head of the + // session pool already contains many sessions with the same channel as this one. + if (session.releaseToPosition != Position.RANDOM && shouldRandomize()) { + session.releaseToPosition = Position.RANDOM; + } else if (session.releaseToPosition == Position.FIRST && isUnbalanced(session)) { session.releaseToPosition = Position.RANDOM; } else if (session.releaseToPosition == Position.RANDOM && !isNewSession @@ -2497,7 +2527,12 @@ private void releaseSession(PooledSession session, boolean isNewSession) { // more efficient. session.releaseToPosition = options.getReleaseToPosition(); } - if (session.releaseToPosition == Position.RANDOM && !sessions.isEmpty()) { + if (position != null) { + // Make sure we use a valid position, as the number of sessions could have changed in the + // meantime. + int actualPosition = Math.min(position, sessions.size()); + sessions.add(actualPosition, session); + } else if (session.releaseToPosition == Position.RANDOM && !sessions.isEmpty()) { // A session should only be added at a random position the first time it is added to // the pool or if the pool was deemed unbalanced. All following releases into the pool // should normally happen at the default release position (unless the pool is again deemed @@ -2510,12 +2545,32 @@ private void releaseSession(PooledSession session, boolean isNewSession) { } else { sessions.addFirst(session); } + session.releaseToPosition = options.getReleaseToPosition(); } else { waiters.poll().put(session); } } } + /** + * Returns true if the position where we return the session should be random if: + * + *
    + *
  1. The current TPS is higher than the configured threshold. + *
  2. AND the number of sessions checked out is larger than the number of channels. + *
+ * + * The second check prevents the session pool from being randomized when the application is + * running many small, quick queries using a small number of parallel threads. This can cause a + * high TPS, without actually having a high degree of parallelism. + */ + @VisibleForTesting + boolean shouldRandomize() { + return this.options.getRandomizePositionQPSThreshold() > 0 + && this.transactionsPerSecond >= this.options.getRandomizePositionQPSThreshold() + && this.numSessionsInUse >= this.numChannels; + } + private boolean isUnbalanced(PooledSession session) { int channel = session.getChannel(); int numChannels = sessionClient.getSpanner().getOptions().getNumChannels(); 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 80d53a4d71f..2ebe77e1ba2 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 @@ -64,6 +64,7 @@ public class SessionPoolOptions { private final Duration waitForMinSessions; private final Duration acquireSessionTimeout; private final Position releaseToPosition; + private final long randomizePositionQPSThreshold; /** Property for allowing mocking of session maintenance clock. */ private final Clock poolMaintainerClock; @@ -89,6 +90,7 @@ private SessionPoolOptions(Builder builder) { this.waitForMinSessions = builder.waitForMinSessions; this.acquireSessionTimeout = builder.acquireSessionTimeout; this.releaseToPosition = builder.releaseToPosition; + this.randomizePositionQPSThreshold = builder.randomizePositionQPSThreshold; this.inactiveTransactionRemovalOptions = builder.inactiveTransactionRemovalOptions; this.poolMaintainerClock = builder.poolMaintainerClock; } @@ -118,6 +120,7 @@ public boolean equals(Object o) { && Objects.equals(this.waitForMinSessions, other.waitForMinSessions) && Objects.equals(this.acquireSessionTimeout, other.acquireSessionTimeout) && Objects.equals(this.releaseToPosition, other.releaseToPosition) + && Objects.equals(this.randomizePositionQPSThreshold, other.randomizePositionQPSThreshold) && Objects.equals( this.inactiveTransactionRemovalOptions, other.inactiveTransactionRemovalOptions) && Objects.equals(this.poolMaintainerClock, other.poolMaintainerClock); @@ -143,6 +146,7 @@ public int hashCode() { this.waitForMinSessions, this.acquireSessionTimeout, this.releaseToPosition, + this.randomizePositionQPSThreshold, this.inactiveTransactionRemovalOptions, this.poolMaintainerClock); } @@ -263,6 +267,10 @@ Position getReleaseToPosition() { return releaseToPosition; } + long getRandomizePositionQPSThreshold() { + return randomizePositionQPSThreshold; + } + public static Builder newBuilder() { return new Builder(); } @@ -451,6 +459,13 @@ public static class Builder { private Duration waitForMinSessions = Duration.ZERO; private Duration acquireSessionTimeout = Duration.ofSeconds(60); private Position releaseToPosition = getReleaseToPositionFromSystemProperty(); + /** + * The session pool will randomize the position of a session that is being returned when this + * threshold is exceeded. That is: If the transactions per second exceeds this threshold, then + * the session pool will use a random order for the sessions instead of LIFO. The default is 0, + * which means that the option is disabled. + */ + private long randomizePositionQPSThreshold = 0L; private Clock poolMaintainerClock; @@ -487,6 +502,7 @@ private Builder(SessionPoolOptions options) { this.autoDetectDialect = options.autoDetectDialect; this.waitForMinSessions = options.waitForMinSessions; this.acquireSessionTimeout = options.acquireSessionTimeout; + this.randomizePositionQPSThreshold = options.randomizePositionQPSThreshold; this.inactiveTransactionRemovalOptions = options.inactiveTransactionRemovalOptions; this.poolMaintainerClock = options.poolMaintainerClock; } @@ -764,6 +780,13 @@ Builder setReleaseToPosition(Position releaseToPosition) { return this; } + Builder setRandomizePositionQPSThreshold(long randomizePositionQPSThreshold) { + Preconditions.checkArgument( + randomizePositionQPSThreshold >= 0L, "randomizePositionQPSThreshold must be >= 0"); + this.randomizePositionQPSThreshold = randomizePositionQPSThreshold; + return this; + } + /** Build a SessionPoolOption object */ public SessionPoolOptions build() { validate(); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/AbstractStatementParser.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/AbstractStatementParser.java index 8fc3043791e..9793a50c636 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/AbstractStatementParser.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/AbstractStatementParser.java @@ -40,6 +40,7 @@ import java.util.concurrent.Callable; import java.util.logging.Level; import java.util.logging.Logger; +import javax.annotation.Nullable; /** * Internal class for the Spanner Connection API. @@ -594,6 +595,7 @@ private boolean statementStartsWith(String sql, Iterable checkStatements static final char CLOSE_PARENTHESIS = ')'; static final char COMMA = ','; static final char UNDERSCORE = '_'; + static final char BACKSLASH = '\\'; /** * Removes comments from and trims the given sql statement using the dialect of this parser. @@ -632,36 +634,33 @@ public static class ParametersInfo { /** * Converts all positional parameters (?) in the given sql string into named parameters. The - * parameters are named @p1, @p2, etc. This method is used when converting a JDBC statement that - * uses positional parameters to a Cloud Spanner {@link Statement} instance that requires named - * parameters. The input SQL string may not contain any comments, except for PostgreSQL-dialect - * SQL strings. + * parameters are named @p1, @p2, etc. for GoogleSQL, and $1, $2, etc. for PostgreSQL. This method + * is used when converting a JDBC statement that uses positional parameters to a Cloud Spanner + * {@link Statement} instance that requires named parameters. * - * @param sql The sql string that should be converted - * @return A {@link ParametersInfo} object containing a string with named parameters instead of - * positional parameters and the number of parameters. - * @throws SpannerException If the input sql string contains an unclosed string/byte literal. - */ - @InternalApi - abstract ParametersInfo convertPositionalParametersToNamedParametersInternal( - char paramChar, String sql); - - /** - * Converts all positional parameters (?) in the given sql string into named parameters. The - * parameters are named @p1, @p2, etc. This method is used when converting a JDBC statement that - * uses positional parameters to a Cloud Spanner {@link Statement} instance that requires named - * parameters. The input SQL string may not contain any comments. There is an exception case if - * the statement starts with a GSQL comment which forces it to be interpreted as a GoogleSql - * statement. - * - * @param sql The sql string without comments that should be converted + * @param sql The sql string that should be converted to use named parameters * @return A {@link ParametersInfo} object containing a string with named parameters instead of * positional parameters and the number of parameters. * @throws SpannerException If the input sql string contains an unclosed string/byte literal. */ @InternalApi public ParametersInfo convertPositionalParametersToNamedParameters(char paramChar, String sql) { - return convertPositionalParametersToNamedParametersInternal(paramChar, sql); + Preconditions.checkNotNull(sql); + final String namedParamPrefix = getQueryParameterPrefix(); + StringBuilder named = new StringBuilder(sql.length() + countOccurrencesOf(paramChar, sql)); + int index = 0; + int paramIndex = 1; + while (index < sql.length()) { + char c = sql.charAt(index); + if (c == paramChar) { + named.append(namedParamPrefix).append(paramIndex); + paramIndex++; + index++; + } else { + index = skip(sql, index, named); + } + } + return new ParametersInfo(paramIndex - 1, named.toString()); } /** Convenience method that is used to estimate the number of parameters in a SQL statement. */ @@ -696,4 +695,274 @@ static int countOccurrencesOf(char c, String string) { public boolean checkReturningClause(String sql) { return checkReturningClauseInternal(sql); } + + /** + * <<<<<<< HEAD Returns true if this dialect supports nested comments. ======= <<<<<<< HEAD + * Returns true if this dialect supports nested comments. >>>>>>> main + * + *
    + *
  • This method should return false for dialects that consider this to be a valid comment: + * /* A comment /* still a comment */. + *
  • This method should return true for dialects that require all comment start sequences to + * be balanced with a comment end sequence: + * /* A comment /* still a comment */ Also still a comment */. + *
+ */ + abstract boolean supportsNestedComments(); + + /** + * Returns true for dialects that support dollar-quoted string literals. + * + *

Example: $tag$This is a string$tag$. + */ + abstract boolean supportsDollarQuotedStrings(); + + /** + * Returns true for dialects that support backticks as a quoting character, either for string + * literals or identifiers. + */ + abstract boolean supportsBacktickQuote(); + + /** + * Returns true for dialects that support triple-quoted string literals and identifiers. + * + *

Example: ```This is a triple-quoted string``` + */ + abstract boolean supportsTripleQuotedStrings(); + + /** + * Returns true if the dialect supports escaping a quote character within a literal with the same + * quote as the literal is using. That is: 'foo''bar' means "foo'bar". + */ + abstract boolean supportsEscapeQuoteWithQuote(); + + /** Returns true if the dialect supports starting an escape sequence with a backslash. */ + abstract boolean supportsBackslashEscape(); + + /** + * Returns true if the dialect supports single-line comments that start with a dash. + * + *

Example: # This is a comment + */ + abstract boolean supportsHashSingleLineComments(); + + /** + * Returns true for dialects that allow line-feeds in quoted strings. Note that the return value + * of this is not used for triple-quoted strings. Triple-quoted strings are assumed to always + * support line-feeds. + */ + abstract boolean supportsLineFeedInQuotedString(); + + /** Returns the query parameter prefix that should be used for this dialect. */ + abstract String getQueryParameterPrefix(); + + /** + * Returns true for characters that can be used as the first character in unquoted identifiers. + */ + boolean isValidIdentifierFirstChar(char c) { + return Character.isLetter(c) || c == UNDERSCORE; + } + + /** Returns true for characters that can be used in unquoted identifiers. */ + boolean isValidIdentifierChar(char c) { + return isValidIdentifierFirstChar(c) || Character.isDigit(c) || c == DOLLAR; + } + + /** Reads a dollar-quoted string literal from position index in the given sql string. */ + String parseDollarQuotedString(String sql, int index) { + // Look ahead to the next dollar sign (if any). Everything in between is the quote tag. + StringBuilder tag = new StringBuilder(); + while (index < sql.length()) { + char c = sql.charAt(index); + if (c == DOLLAR) { + return tag.toString(); + } + if (!isValidIdentifierChar(c)) { + break; + } + tag.append(c); + index++; + } + return null; + } + + /** + * Skips the next character, literal, identifier, or comment in the given sql string from the + * given index. The skipped characters are added to result if it is not null. + */ + int skip(String sql, int currentIndex, @Nullable StringBuilder result) { + if (currentIndex >= sql.length()) { + return currentIndex; + } + char currentChar = sql.charAt(currentIndex); + + if (currentChar == SINGLE_QUOTE + || currentChar == DOUBLE_QUOTE + || (supportsBacktickQuote() && currentChar == BACKTICK_QUOTE)) { + appendIfNotNull(result, currentChar); + return skipQuoted(sql, currentIndex, currentChar, result); + } else if (supportsDollarQuotedStrings() && currentChar == DOLLAR) { + String dollarTag = parseDollarQuotedString(sql, currentIndex + 1); + if (dollarTag != null) { + appendIfNotNull(result, currentChar, dollarTag, currentChar); + return skipQuoted( + sql, currentIndex + dollarTag.length() + 1, currentChar, dollarTag, result); + } + } else if (currentChar == HYPHEN + && sql.length() > (currentIndex + 1) + && sql.charAt(currentIndex + 1) == HYPHEN) { + return skipSingleLineComment(sql, currentIndex, result); + } else if (currentChar == DASH && supportsHashSingleLineComments()) { + return skipSingleLineComment(sql, currentIndex, result); + } else if (currentChar == SLASH + && sql.length() > (currentIndex + 1) + && sql.charAt(currentIndex + 1) == ASTERISK) { + return skipMultiLineComment(sql, currentIndex, result); + } + + appendIfNotNull(result, currentChar); + return currentIndex + 1; + } + + /** Skips a single-line comment from startIndex and adds it to result if result is not null. */ + static int skipSingleLineComment(String sql, int startIndex, @Nullable StringBuilder result) { + int endIndex = sql.indexOf('\n', startIndex + 2); + if (endIndex == -1) { + endIndex = sql.length(); + } else { + // Include the newline character. + endIndex++; + } + appendIfNotNull(result, sql.substring(startIndex, endIndex)); + return endIndex; + } + + /** Skips a multi-line comment from startIndex and adds it to result if result is not null. */ + int skipMultiLineComment(String sql, int startIndex, @Nullable StringBuilder result) { + // Current position is start + '/*'.length(). + int pos = startIndex + 2; + // PostgreSQL allows comments to be nested. That is, the following is allowed: + // '/* test /* inner comment */ still a comment */' + int level = 1; + while (pos < sql.length()) { + if (supportsNestedComments() + && sql.charAt(pos) == SLASH + && sql.length() > (pos + 1) + && sql.charAt(pos + 1) == ASTERISK) { + level++; + } + if (sql.charAt(pos) == ASTERISK && sql.length() > (pos + 1) && sql.charAt(pos + 1) == SLASH) { + level--; + if (level == 0) { + pos += 2; + appendIfNotNull(result, sql.substring(startIndex, pos)); + return pos; + } + } + pos++; + } + appendIfNotNull(result, sql.substring(startIndex)); + return sql.length(); + } + + /** Skips a quoted string from startIndex. */ + private int skipQuoted( + String sql, int startIndex, char startQuote, @Nullable StringBuilder result) { + return skipQuoted(sql, startIndex, startQuote, null, result); + } + + /** + * Skips a quoted string from startIndex. The quote character is assumed to be $ if dollarTag is + * not null. + */ + int skipQuoted( + String sql, + int startIndex, + char startQuote, + @Nullable String dollarTag, + @Nullable StringBuilder result) { + boolean isTripleQuoted = + supportsTripleQuotedStrings() + && sql.length() > startIndex + 2 + && sql.charAt(startIndex + 1) == startQuote + && sql.charAt(startIndex + 2) == startQuote; + int currentIndex = startIndex + (isTripleQuoted ? 3 : 1); + if (isTripleQuoted) { + appendIfNotNull(result, startQuote); + appendIfNotNull(result, startQuote); + } + while (currentIndex < sql.length()) { + char currentChar = sql.charAt(currentIndex); + if (currentChar == startQuote) { + if (supportsDollarQuotedStrings() && currentChar == DOLLAR) { + // Check if this is the end of the current dollar quoted string. + String tag = parseDollarQuotedString(sql, currentIndex + 1); + if (tag != null && tag.equals(dollarTag)) { + appendIfNotNull(result, currentChar, dollarTag, currentChar); + return currentIndex + tag.length() + 2; + } + } else if (supportsEscapeQuoteWithQuote() + && sql.length() > currentIndex + 1 + && sql.charAt(currentIndex + 1) == startQuote) { + // This is an escaped quote (e.g. 'foo''bar') + appendIfNotNull(result, currentChar); + appendIfNotNull(result, currentChar); + currentIndex += 2; + continue; + } else if (isTripleQuoted) { + // Check if this is the end of the triple-quoted string. + if (sql.length() > currentIndex + 2 + && sql.charAt(currentIndex + 1) == startQuote + && sql.charAt(currentIndex + 2) == startQuote) { + appendIfNotNull(result, currentChar); + appendIfNotNull(result, currentChar); + appendIfNotNull(result, currentChar); + return currentIndex + 3; + } + } else { + appendIfNotNull(result, currentChar); + return currentIndex + 1; + } + } else if (supportsBackslashEscape() + && currentChar == BACKSLASH + && sql.length() > currentIndex + 1 + && sql.charAt(currentIndex + 1) == startQuote) { + // This is an escaped quote (e.g. 'foo\'bar'). + // Note that in raw strings, the \ officially does not start an escape sequence, but the + // result is still the same, as in a raw string 'both characters are preserved'. + appendIfNotNull(result, currentChar); + appendIfNotNull(result, sql.charAt(currentIndex + 1)); + currentIndex += 2; + continue; + } else if (currentChar == '\n' && !isTripleQuoted && !supportsLineFeedInQuotedString()) { + break; + } + currentIndex++; + appendIfNotNull(result, currentChar); + } + throw SpannerExceptionFactory.newSpannerException( + ErrorCode.INVALID_ARGUMENT, "SQL statement contains an unclosed literal: " + sql); + } + + /** Appends the given character to result if result is not null. */ + private void appendIfNotNull(@Nullable StringBuilder result, char currentChar) { + if (result != null) { + result.append(currentChar); + } + } + + /** Appends the given suffix to result if result is not null. */ + private static void appendIfNotNull(@Nullable StringBuilder result, String suffix) { + if (result != null) { + result.append(suffix); + } + } + + /** Appends the given prefix, tag, and suffix to result if result is not null. */ + private static void appendIfNotNull( + @Nullable StringBuilder result, char prefix, String tag, char suffix) { + if (result != null) { + result.append(prefix).append(tag).append(suffix); + } + } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/PostgreSQLStatementParser.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/PostgreSQLStatementParser.java index 8cb2b7e464a..be4aa9d7f46 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/PostgreSQLStatementParser.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/PostgreSQLStatementParser.java @@ -26,7 +26,6 @@ import java.util.HashSet; import java.util.Set; import java.util.regex.Pattern; -import javax.annotation.Nullable; @InternalApi public class PostgreSQLStatementParser extends AbstractStatementParser { @@ -49,6 +48,51 @@ protected boolean supportsExplain() { return false; } + @Override + boolean supportsNestedComments() { + return true; + } + + @Override + boolean supportsDollarQuotedStrings() { + return true; + } + + @Override + boolean supportsBacktickQuote() { + return false; + } + + @Override + boolean supportsTripleQuotedStrings() { + return false; + } + + @Override + boolean supportsEscapeQuoteWithQuote() { + return true; + } + + @Override + boolean supportsBackslashEscape() { + return false; + } + + @Override + boolean supportsHashSingleLineComments() { + return false; + } + + @Override + boolean supportsLineFeedInQuotedString() { + return true; + } + + @Override + String getQueryParameterPrefix() { + return "$"; + } + /** * Removes comments from and trims the given sql statement. PostgreSQL supports two types of * comments: @@ -136,50 +180,12 @@ String removeCommentsAndTrimInternal(String sql) { return res.toString().trim(); } - String parseDollarQuotedString(String sql, int index) { - // Look ahead to the next dollar sign (if any). Everything in between is the quote tag. - StringBuilder tag = new StringBuilder(); - while (index < sql.length()) { - char c = sql.charAt(index); - if (c == DOLLAR) { - return tag.toString(); - } - if (!isValidIdentifierChar(c)) { - break; - } - tag.append(c); - index++; - } - return null; - } - /** PostgreSQL does not support statement hints. */ @Override String removeStatementHint(String sql) { return sql; } - @InternalApi - @Override - ParametersInfo convertPositionalParametersToNamedParametersInternal(char paramChar, String sql) { - Preconditions.checkNotNull(sql); - final String namedParamPrefix = "$"; - StringBuilder named = new StringBuilder(sql.length() + countOccurrencesOf(paramChar, sql)); - int index = 0; - int paramIndex = 1; - while (index < sql.length()) { - char c = sql.charAt(index); - if (c == paramChar) { - named.append(namedParamPrefix).append(paramIndex); - paramIndex++; - index++; - } else { - index = skip(sql, index, named); - } - } - return new ParametersInfo(paramIndex - 1, named.toString()); - } - /** * Note: This is an internal API and breaking changes can be made without prior notice. * @@ -220,135 +226,6 @@ public Set getQueryParameters(String sql) { return parameters; } - private int skip(String sql, int currentIndex, @Nullable StringBuilder result) { - char currentChar = sql.charAt(currentIndex); - if (currentChar == SINGLE_QUOTE || currentChar == DOUBLE_QUOTE) { - appendIfNotNull(result, currentChar); - return skipQuoted(sql, currentIndex, currentChar, result); - } else if (currentChar == DOLLAR) { - String dollarTag = parseDollarQuotedString(sql, currentIndex + 1); - if (dollarTag != null) { - appendIfNotNull(result, currentChar, dollarTag, currentChar); - return skipQuoted( - sql, currentIndex + dollarTag.length() + 1, currentChar, dollarTag, result); - } - } else if (currentChar == HYPHEN - && sql.length() > (currentIndex + 1) - && sql.charAt(currentIndex + 1) == HYPHEN) { - return skipSingleLineComment(sql, currentIndex, result); - } else if (currentChar == SLASH - && sql.length() > (currentIndex + 1) - && sql.charAt(currentIndex + 1) == ASTERISK) { - return skipMultiLineComment(sql, currentIndex, result); - } - - appendIfNotNull(result, currentChar); - return currentIndex + 1; - } - - static int skipSingleLineComment(String sql, int currentIndex, @Nullable StringBuilder result) { - int endIndex = sql.indexOf('\n', currentIndex + 2); - if (endIndex == -1) { - endIndex = sql.length(); - } else { - // Include the newline character. - endIndex++; - } - appendIfNotNull(result, sql.substring(currentIndex, endIndex)); - return endIndex; - } - - static int skipMultiLineComment(String sql, int startIndex, @Nullable StringBuilder result) { - // Current position is start + '/*'.length(). - int pos = startIndex + 2; - // PostgreSQL allows comments to be nested. That is, the following is allowed: - // '/* test /* inner comment */ still a comment */' - int level = 1; - while (pos < sql.length()) { - if (sql.charAt(pos) == SLASH && sql.length() > (pos + 1) && sql.charAt(pos + 1) == ASTERISK) { - level++; - } - if (sql.charAt(pos) == ASTERISK && sql.length() > (pos + 1) && sql.charAt(pos + 1) == SLASH) { - level--; - if (level == 0) { - pos += 2; - appendIfNotNull(result, sql.substring(startIndex, pos)); - return pos; - } - } - pos++; - } - appendIfNotNull(result, sql.substring(startIndex)); - return sql.length(); - } - - private int skipQuoted( - String sql, int startIndex, char startQuote, @Nullable StringBuilder result) { - return skipQuoted(sql, startIndex, startQuote, null, result); - } - - private int skipQuoted( - String sql, - int startIndex, - char startQuote, - String dollarTag, - @Nullable StringBuilder result) { - int currentIndex = startIndex + 1; - while (currentIndex < sql.length()) { - char currentChar = sql.charAt(currentIndex); - if (currentChar == startQuote) { - if (currentChar == DOLLAR) { - // Check if this is the end of the current dollar quoted string. - String tag = parseDollarQuotedString(sql, currentIndex + 1); - if (tag != null && tag.equals(dollarTag)) { - appendIfNotNull(result, currentChar, dollarTag, currentChar); - return currentIndex + tag.length() + 2; - } - } else if (sql.length() > currentIndex + 1 && sql.charAt(currentIndex + 1) == startQuote) { - // This is an escaped quote (e.g. 'foo''bar') - appendIfNotNull(result, currentChar); - appendIfNotNull(result, currentChar); - currentIndex += 2; - continue; - } else { - appendIfNotNull(result, currentChar); - return currentIndex + 1; - } - } - currentIndex++; - appendIfNotNull(result, currentChar); - } - throw SpannerExceptionFactory.newSpannerException( - ErrorCode.INVALID_ARGUMENT, "SQL statement contains an unclosed literal: " + sql); - } - - private void appendIfNotNull(@Nullable StringBuilder result, char currentChar) { - if (result != null) { - result.append(currentChar); - } - } - - private static void appendIfNotNull(@Nullable StringBuilder result, String suffix) { - if (result != null) { - result.append(suffix); - } - } - - private void appendIfNotNull( - @Nullable StringBuilder result, char prefix, String tag, char suffix) { - if (result != null) { - result.append(prefix).append(tag).append(suffix); - } - } - - private boolean isValidIdentifierFirstChar(char c) { - return Character.isLetter(c) || c == UNDERSCORE; - } - - private boolean isValidIdentifierChar(char c) { - return isValidIdentifierFirstChar(c) || Character.isDigit(c) || c == DOLLAR; - } - private boolean checkCharPrecedingReturning(char ch) { return (ch == SPACE) || (ch == SINGLE_QUOTE) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerStatementParser.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerStatementParser.java index 251c5a2e6ec..892672ad0df 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerStatementParser.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerStatementParser.java @@ -50,6 +50,51 @@ protected boolean supportsExplain() { return true; } + @Override + boolean supportsNestedComments() { + return false; + } + + @Override + boolean supportsDollarQuotedStrings() { + return false; + } + + @Override + boolean supportsBacktickQuote() { + return true; + } + + @Override + boolean supportsTripleQuotedStrings() { + return true; + } + + @Override + boolean supportsEscapeQuoteWithQuote() { + return false; + } + + @Override + boolean supportsBackslashEscape() { + return true; + } + + @Override + boolean supportsHashSingleLineComments() { + return true; + } + + @Override + boolean supportsLineFeedInQuotedString() { + return false; + } + + @Override + String getQueryParameterPrefix() { + return "@p"; + } + /** * Removes comments from and trims the given sql statement. Spanner supports three types of * comments: @@ -210,68 +255,6 @@ String removeStatementHint(String sql) { return sql; } - @InternalApi - @Override - ParametersInfo convertPositionalParametersToNamedParametersInternal(char paramChar, String sql) { - boolean isInQuoted = false; - char startQuote = 0; - boolean lastCharWasEscapeChar = false; - boolean isTripleQuoted = false; - int paramIndex = 1; - StringBuilder named = new StringBuilder(sql.length() + countOccurrencesOf(paramChar, sql)); - for (int index = 0; index < sql.length(); index++) { - char c = sql.charAt(index); - if (isInQuoted) { - if ((c == '\n' || c == '\r') && !isTripleQuoted) { - throw SpannerExceptionFactory.newSpannerException( - ErrorCode.INVALID_ARGUMENT, "SQL statement contains an unclosed literal: " + sql); - } else if (c == startQuote) { - if (lastCharWasEscapeChar) { - lastCharWasEscapeChar = false; - } else if (isTripleQuoted) { - if (sql.length() > index + 2 - && sql.charAt(index + 1) == startQuote - && sql.charAt(index + 2) == startQuote) { - isInQuoted = false; - startQuote = 0; - isTripleQuoted = false; - } - } else { - isInQuoted = false; - startQuote = 0; - } - } else if (c == '\\') { - lastCharWasEscapeChar = true; - } else { - lastCharWasEscapeChar = false; - } - named.append(c); - } else { - if (c == paramChar) { - named.append("@p" + paramIndex); - paramIndex++; - } else { - if (c == SINGLE_QUOTE || c == DOUBLE_QUOTE || c == BACKTICK_QUOTE) { - isInQuoted = true; - startQuote = c; - // check whether it is a triple-quote - if (sql.length() > index + 2 - && sql.charAt(index + 1) == startQuote - && sql.charAt(index + 2) == startQuote) { - isTripleQuoted = true; - } - } - named.append(c); - } - } - } - if (isInQuoted) { - throw SpannerExceptionFactory.newSpannerException( - ErrorCode.INVALID_ARGUMENT, "SQL statement contains an unclosed literal: " + sql); - } - return new ParametersInfo(paramIndex - 1, named.toString()); - } - private boolean isReturning(String sql, int index) { return (index >= 1) && (index + 12 <= sql.length()) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index 0f4b2275717..29de1fe1ff5 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -354,6 +354,11 @@ public GapicSpannerRpc(final SpannerOptions options) { options.isAttemptDirectPath() && !Objects.equals( options.getScopedCredentials(), NoCredentials.getInstance())); + String directPathXdsEnv = System.getenv("GOOGLE_SPANNER_ENABLE_DIRECT_ACCESS"); + boolean isAttemptDirectPathXds = Boolean.parseBoolean(directPathXdsEnv); + if (isAttemptDirectPathXds) { + defaultChannelProviderBuilder.setAttemptDirectPathXds(); + } if (options.isUseVirtualThreads()) { ExecutorService executor = tryCreateVirtualThreadPerTaskExecutor("spanner-virtual-grpc-executor"); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolMaintainerTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolMaintainerTest.java index 127ffd3bff6..b489511b73b 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolMaintainerTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolMaintainerTest.java @@ -17,6 +17,9 @@ package com.google.cloud.spanner; 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.assertTrue; import static org.mockito.Mockito.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; @@ -28,6 +31,7 @@ import com.google.cloud.spanner.SessionPool.PooledSessionFuture; import com.google.cloud.spanner.SessionPool.Position; import com.google.cloud.spanner.SessionPool.SessionConsumerImpl; +import com.google.common.base.Preconditions; import io.opencensus.trace.Tracing; import io.opentelemetry.api.OpenTelemetry; import java.util.ArrayList; @@ -115,6 +119,10 @@ private SessionImpl setupMockSession(final SessionImpl session, final ReadContex } private SessionPool createPool() throws Exception { + return createPool(this.options); + } + + private SessionPool createPool(SessionPoolOptions options) throws Exception { // Allow sessions to be added to the head of the pool in all cases in this test, as it is // otherwise impossible to know which session exactly is getting pinged at what point in time. SessionPool pool = @@ -175,14 +183,20 @@ public void testKeepAlive() throws Exception { Session session3 = pool.getSession(); Session session4 = pool.getSession(); Session session5 = pool.getSession(); - // Note that session2 was now the first session in the pool as it was the last to receive a - // ping. - assertThat(session3.getName()).isEqualTo(session2.getName()); - assertThat(session4.getName()).isEqualTo(session1.getName()); + // Pinging a session will put it at the back of the pool. A session that needed a ping to be + // kept alive is not one that should be preferred for use. This means that session2 is the last + // session in the pool, and session1 the second-to-last. + assertEquals(session1.getName(), session3.getName()); + assertEquals(session2.getName(), session4.getName()); session5.close(); session4.close(); session3.close(); // Advance the clock to force pings for the sessions in the pool and do three maintenance loops. + // This should ping the sessions in the following order: + // 1. session3 (=session1) + // 2. session4 (=session2) + // The pinged sessions already contains: {session1: 1, session2: 1} + // Note that the pool only pings up to MinSessions sessions. clock.currentTimeMillis.addAndGet( TimeUnit.MINUTES.toMillis(options.getKeepAliveIntervalMinutes()) + 1); runMaintenanceLoop(clock, pool, 3); @@ -192,16 +206,18 @@ public void testKeepAlive() throws Exception { // should cause only one session to get a ping. clock.currentTimeMillis.addAndGet( TimeUnit.MINUTES.toMillis(options.getKeepAliveIntervalMinutes()) + 1); - // We are now checking out session2 because + // This will be session1, as all sessions were pinged in the previous 3 maintenance loops, and + // this will have brought session1 back to the front of the pool. Session session6 = pool.getSession(); // The session that was first in the pool now is equal to the initial first session as each full // round of pings will swap the order of the first MinSessions sessions in the pool. assertThat(session6.getName()).isEqualTo(session1.getName()); runMaintenanceLoop(clock, pool, 3); + // Running 3 cycles will only ping the 2 sessions in the pool once. assertThat(pool.totalSessions()).isEqualTo(3); assertThat(pingedSessions).containsExactly(session1.getName(), 2, session2.getName(), 3); // Update the last use date and release the session to the pool and do another maintenance - // cycle. + // cycle. This should not ping any sessions. ((PooledSessionFuture) session6).get().markUsed(); session6.close(); runMaintenanceLoop(clock, pool, 3); @@ -267,10 +283,10 @@ public void testIdleSessions() throws Exception { Session session3 = pool.getSession().get(); Session session4 = pool.getSession().get(); Session session5 = pool.getSession().get(); - // Note that session2 was now the first session in the pool as it was the last to receive a - // ping. - assertThat(session3.getName()).isEqualTo(session2.getName()); - assertThat(session4.getName()).isEqualTo(session1.getName()); + // Note that pinging sessions does not change the order of the pool. This means that session2 + // is still the last session in the pool. + assertThat(session3.getName()).isEqualTo(session1.getName()); + assertThat(session4.getName()).isEqualTo(session2.getName()); session5.close(); session4.close(); session3.close(); @@ -315,4 +331,67 @@ public void testIdleSessions() throws Exception { } assertThat(pool.totalSessions()).isEqualTo(options.getMinSessions()); } + + @Test + public void testRandomizeThreshold() throws Exception { + SessionPool pool = + createPool( + this.options + .toBuilder() + .setMaxSessions(400) + .setLoopFrequency(1000L) + .setRandomizePositionQPSThreshold(4) + .build()); + List sessions; + + // Run a maintenance loop. No sessions have been checked out so far, so the TPS should be 0. + runMaintenanceLoop(clock, pool, 1); + assertFalse(pool.shouldRandomize()); + + // Get and return one session. This means TPS == 1. + returnSessions(1, useSessions(1, pool)); + runMaintenanceLoop(clock, pool, 1); + assertFalse(pool.shouldRandomize()); + + // Get and return four sessions. This means TPS == 4, and that no sessions are checked out. + returnSessions(4, useSessions(4, pool)); + runMaintenanceLoop(clock, pool, 1); + assertFalse(pool.shouldRandomize()); + + // Get four sessions without returning them. + // This means TPS == 4 and that they are all still checked out. + sessions = useSessions(4, pool); + runMaintenanceLoop(clock, pool, 1); + assertTrue(pool.shouldRandomize()); + // Returning one of the sessions reduces the number of checked out sessions enough to stop the + // randomizing. + returnSessions(1, sessions); + runMaintenanceLoop(clock, pool, 1); + assertFalse(pool.shouldRandomize()); + + // Get three more session and run the maintenance loop. + // The TPS is then 3, as we've only gotten 3 sessions since the last maintenance run. + // That means that we should not randomize. + sessions.addAll(useSessions(3, pool)); + runMaintenanceLoop(clock, pool, 1); + assertFalse(pool.shouldRandomize()); + + returnSessions(sessions.size(), sessions); + } + + private List useSessions(int numSessions, SessionPool pool) { + List sessions = new ArrayList<>(numSessions); + for (int i = 0; i < numSessions; i++) { + sessions.add(pool.getSession()); + sessions.get(sessions.size() - 1).singleUse().executeQuery(Statement.of("SELECT 1")).next(); + } + return sessions; + } + + private void returnSessions(int numSessions, List sessions) { + Preconditions.checkArgument(numSessions <= sessions.size()); + for (int i = 0; i < numSessions; i++) { + sessions.remove(0).close(); + } + } } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolOptionsTest.java index 23aa626f393..22d10d92a87 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolOptionsTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -218,4 +219,31 @@ public void verifyDefaultAcquireSessionTimeout() { assertEquals(Duration.ofSeconds(60), sessionPoolOptions.getAcquireSessionTimeout()); } + + @Test + public void testRandomizePositionQPSThreshold() { + assertEquals(0L, SessionPoolOptions.newBuilder().build().getRandomizePositionQPSThreshold()); + assertEquals( + 4L, + SessionPoolOptions.newBuilder() + .setRandomizePositionQPSThreshold(4L) + .build() + .getRandomizePositionQPSThreshold()); + assertEquals( + 10L, + SessionPoolOptions.newBuilder() + .setRandomizePositionQPSThreshold(4L) + .setRandomizePositionQPSThreshold(10L) + .build() + .getRandomizePositionQPSThreshold()); + assertEquals( + 0L, + SessionPoolOptions.newBuilder() + .setRandomizePositionQPSThreshold(0L) + .build() + .getRandomizePositionQPSThreshold()); + assertThrows( + IllegalArgumentException.class, + () -> SessionPoolOptions.newBuilder().setRandomizePositionQPSThreshold(-1L)); + } } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SpannerStatementParserTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SpannerStatementParserTest.java new file mode 100644 index 00000000000..5cec5d838d1 --- /dev/null +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SpannerStatementParserTest.java @@ -0,0 +1,239 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.spanner.connection; + +import static com.google.cloud.spanner.connection.StatementParserTest.assertUnclosedLiteral; +import static org.junit.Assert.assertEquals; + +import com.google.cloud.spanner.Dialect; +import com.google.cloud.spanner.connection.StatementParserTest.CommentInjector; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class SpannerStatementParserTest { + + static String skip(String sql) { + return skip(sql, 0); + } + + static String skip(String sql, int currentIndex) { + int position = + AbstractStatementParser.getInstance(Dialect.GOOGLE_STANDARD_SQL) + .skip(sql, currentIndex, null); + return sql.substring(currentIndex, position); + } + + @Test + public void testSkip() { + assertEquals("", skip("")); + assertEquals("1", skip("1 ")); + assertEquals("1", skip("12 ")); + assertEquals("2", skip("12 ", 1)); + assertEquals("", skip("12", 2)); + + assertEquals("'foo'", skip("'foo' ", 0)); + assertEquals("'foo'", skip("'foo''bar' ", 0)); + assertEquals("'foo'", skip("'foo' 'bar' ", 0)); + assertEquals("'bar'", skip("'foo''bar' ", 5)); + assertEquals("'foo\"bar\"'", skip("'foo\"bar\"' ", 0)); + assertEquals("\"foo'bar'\"", skip("\"foo'bar'\" ", 0)); + assertEquals("`foo'bar'`", skip("`foo'bar'` ", 0)); + + assertEquals("'''foo'bar'''", skip("'''foo'bar''' ", 0)); + assertEquals("'''foo\\'bar'''", skip("'''foo\\'bar''' ", 0)); + assertEquals("'''foo\\'\\'bar'''", skip("'''foo\\'\\'bar''' ", 0)); + assertEquals("'''foo\\'\\'\\'bar'''", skip("'''foo\\'\\'\\'bar''' ", 0)); + assertEquals("\"\"\"foo'bar\"\"\"", skip("\"\"\"foo'bar\"\"\"", 0)); + assertEquals("```foo'bar```", skip("```foo'bar```", 0)); + + assertEquals("-- comment\n", skip("-- comment\nselect * from foo", 0)); + assertEquals("# comment\n", skip("# comment\nselect * from foo", 0)); + assertEquals("/* comment */", skip("/* comment */ select * from foo", 0)); + assertEquals( + "/* comment /* GoogleSQL does not support nested comments */", + skip("/* comment /* GoogleSQL does not support nested comments */ select * from foo", 0)); + // GoogleSQL does not support dollar-quoted strings. + assertEquals("$", skip("$tag$not a string$tag$ select * from foo", 0)); + + assertEquals("/* 'test' */", skip("/* 'test' */ foo")); + assertEquals("-- 'test' \n", skip("-- 'test' \n foo")); + assertEquals("'/* test */'", skip("'/* test */' foo")); + + // Raw strings do not consider '\' as something that starts an escape sequence, but any + // quote character following it is still preserved within the string, as the definition of a + // raw string says that 'both characters are preserved'. + assertEquals("'foo\\''", skip("'foo\\'' ", 0)); + assertEquals("'foo\\''", skip("r'foo\\'' ", 1)); + assertEquals("'''foo\\'\\'\\'bar'''", skip("'''foo\\'\\'\\'bar''' ", 0)); + } + + @Test + public void testConvertPositionalParametersToNamedParameters() { + AbstractStatementParser parser = + AbstractStatementParser.getInstance(Dialect.GOOGLE_STANDARD_SQL); + + for (String comment : + new String[] { + "-- test comment\n", + "/* another test comment */", + "/* comment\nwith\nmultiple\nlines\n */", + "/* comment /* with nested */ comment */" + }) { + for (CommentInjector injector : CommentInjector.values()) { + assertEquals( + injector.inject("select * %sfrom foo where name=@p1", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("select * %sfrom foo where name=?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("@p1%s'?test?\"?test?\"?'@p2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?%s'?test?\"?test?\"?'?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("@p1'?it\\'?s'%s@p2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?'?it\\'?s'%s?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("@p1'?it\\\"?s'%s@p2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?'?it\\\"?s'%s?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("@p1\"?it\\\"?s\"%s@p2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?\"?it\\\"?s\"%s?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("@p1%s'''?it\\''?s'''@p2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?%s'''?it\\''?s'''?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("@p1\"\"\"?it\\\"\"?s\"\"\"%s@p2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?\"\"\"?it\\\"\"?s\"\"\"%s?", comment)) + .sqlWithNamedParameters); + + // GoogleSQL does not support dollar-quoted strings, so these are all ignored. + assertEquals( + injector.inject("@p1$$@p2it$@p3s$$%s@p4", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?$$?it$?s$$%s?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("@p1$tag$@p2it$$@p3s$tag$%s@p4", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?$tag$?it$$?s$tag$%s?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("@p1%s$$@p2it\\'?s \t ?it\\'?s'$$@p3", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?%s$$?it\\'?s \t ?it\\'?s'$$?", comment)) + .sqlWithNamedParameters); + + // Note: GoogleSQL does not allowa a single-quoted string literal to contain line feeds. + assertUnclosedLiteral(parser, injector.inject("?'?it\\''?s \n ?it\\''?s'%s?", comment)); + assertEquals( + "@p1'?it\\''@p2s \n @p3it\\''@p4s@p5", + parser.convertPositionalParametersToNamedParameters('?', "?'?it\\''?s \n ?it\\''?s?") + .sqlWithNamedParameters); + assertEquals( + injector.inject("@p1%s'''?it\\''?s \n ?it\\''?s'''@p2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?%s'''?it\\''?s \n ?it\\''?s'''?", comment)) + .sqlWithNamedParameters); + + assertEquals( + injector.inject( + "select 1, @p1, 'test?test', \"test?test\", %sfoo.* from `foo` where col1=@p2 and col2='test' and col3=@p3 and col4='?' and col5=\"?\" and col6='?''?''?'", + comment), + parser.convertPositionalParametersToNamedParameters( + '?', + injector.inject( + "select 1, ?, 'test?test', \"test?test\", %sfoo.* from `foo` where col1=? and col2='test' and col3=? and col4='?' and col5=\"?\" and col6='?''?''?'", + comment)) + .sqlWithNamedParameters); + + assertEquals( + injector.inject( + "select * " + + "%sfrom foo " + + "where name=@p1 " + + "and col2 like @p2 " + + "and col3 > @p3", + comment), + parser.convertPositionalParametersToNamedParameters( + '?', + injector.inject( + "select * " + + "%sfrom foo " + + "where name=? " + + "and col2 like ? " + + "and col3 > ?", + comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("select * " + "from foo " + "where id between @p1%s and @p2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', + injector.inject( + "select * " + "from foo " + "where id between ?%s and ?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("select * " + "from foo " + "limit @p1 %s offset @p2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', + injector.inject("select * " + "from foo " + "limit ? %s offset ?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject( + "select * " + + "from foo " + + "where col1=@p1 " + + "and col2 like @p2 " + + " %s " + + "and col3 > @p3 " + + "and col4 < @p4 " + + "and col5 != @p5 " + + "and col6 not in (@p6, @p7, @p8) " + + "and col7 in (@p9, @p10, @p11) " + + "and col8 between @p12 and @p13", + comment), + parser.convertPositionalParametersToNamedParameters( + '?', + injector.inject( + "select * " + + "from foo " + + "where col1=? " + + "and col2 like ? " + + " %s " + + "and col3 > ? " + + "and col4 < ? " + + "and col5 != ? " + + "and col6 not in (?, ?, ?) " + + "and col7 in (?, ?, ?) " + + "and col8 between ? and ?", + comment)) + .sqlWithNamedParameters); + } + } + } +} diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/StatementParserTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/StatementParserTest.java index d3438b2b661..3739aa11064 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/StatementParserTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/StatementParserTest.java @@ -17,12 +17,11 @@ package com.google.cloud.spanner.connection; import static com.google.common.truth.Truth.assertThat; -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.junit.Assume.assumeTrue; @@ -1066,85 +1065,86 @@ private void testParseStatementWithOneParame public void testGoogleStandardSQLDialectConvertPositionalParametersToNamedParameters() { assumeTrue(dialect == Dialect.GOOGLE_STANDARD_SQL); - assertThat( - parser.convertPositionalParametersToNamedParameters( - '?', "select * from foo where name=?") - .sqlWithNamedParameters) - .isEqualTo("select * from foo where name=@p1"); - assertThat( - parser.convertPositionalParametersToNamedParameters('?', "?'?test?\"?test?\"?'?") - .sqlWithNamedParameters) - .isEqualTo("@p1'?test?\"?test?\"?'@p2"); - assertThat( - parser.convertPositionalParametersToNamedParameters('?', "?'?it\\'?s'?") - .sqlWithNamedParameters) - .isEqualTo("@p1'?it\\'?s'@p2"); - assertThat( - parser.convertPositionalParametersToNamedParameters('?', "?'?it\\\"?s'?") - .sqlWithNamedParameters) - .isEqualTo("@p1'?it\\\"?s'@p2"); - assertThat( - parser.convertPositionalParametersToNamedParameters('?', "?\"?it\\\"?s\"?") - .sqlWithNamedParameters) - .isEqualTo("@p1\"?it\\\"?s\"@p2"); - assertThat( - parser.convertPositionalParametersToNamedParameters('?', "?'''?it\\'?s'''?") - .sqlWithNamedParameters) - .isEqualTo("@p1'''?it\\'?s'''@p2"); - assertThat( - parser.convertPositionalParametersToNamedParameters('?', "?\"\"\"?it\\\"?s\"\"\"?") - .sqlWithNamedParameters) - .isEqualTo("@p1\"\"\"?it\\\"?s\"\"\"@p2"); + assertEquals( + "select * from foo where name=@p1", + parser.convertPositionalParametersToNamedParameters('?', "select * from foo where name=?") + .sqlWithNamedParameters); + assertEquals( + "@p1'?test?\"?test?\"?'@p2", + parser.convertPositionalParametersToNamedParameters('?', "?'?test?\"?test?\"?'?") + .sqlWithNamedParameters); + assertEquals( + "@p1'?it\\'?s'@p2", + parser.convertPositionalParametersToNamedParameters('?', "?'?it\\'?s'?") + .sqlWithNamedParameters); + assertEquals( + "@p1'?it\\\"?s'@p2", + parser.convertPositionalParametersToNamedParameters('?', "?'?it\\\"?s'?") + .sqlWithNamedParameters); + assertEquals( + "@p1\"?it\\\"?s\"@p2", + parser.convertPositionalParametersToNamedParameters('?', "?\"?it\\\"?s\"?") + .sqlWithNamedParameters); + assertEquals( + "@p1'''?it\\'?s'''@p2", + parser.convertPositionalParametersToNamedParameters('?', "?'''?it\\'?s'''?") + .sqlWithNamedParameters); + assertEquals( + "@p1\"\"\"?it\\\"?s\"\"\"@p2", + parser.convertPositionalParametersToNamedParameters('?', "?\"\"\"?it\\\"?s\"\"\"?") + .sqlWithNamedParameters); - assertThat( - parser.convertPositionalParametersToNamedParameters('?', "?`?it\\`?s`?") - .sqlWithNamedParameters) - .isEqualTo("@p1`?it\\`?s`@p2"); - assertThat( - parser.convertPositionalParametersToNamedParameters('?', "?```?it\\`?s```?") - .sqlWithNamedParameters) - .isEqualTo("@p1```?it\\`?s```@p2"); - assertThat( - parser.convertPositionalParametersToNamedParameters('?', "?'''?it\\'?s \n ?it\\'?s'''?") - .sqlWithNamedParameters) - .isEqualTo("@p1'''?it\\'?s \n ?it\\'?s'''@p2"); + assertEquals( + "@p1`?it\\`?s`@p2", + parser.convertPositionalParametersToNamedParameters('?', "?`?it\\`?s`?") + .sqlWithNamedParameters); + assertEquals( + "@p1```?it\\`?s```@p2", + parser.convertPositionalParametersToNamedParameters('?', "?```?it\\`?s```?") + .sqlWithNamedParameters); + assertEquals( + "@p1'''?it\\'?s \n ?it\\'?s'''@p2", + parser.convertPositionalParametersToNamedParameters('?', "?'''?it\\'?s \n ?it\\'?s'''?") + .sqlWithNamedParameters); - assertUnclosedLiteral("?'?it\\'?s \n ?it\\'?s'?"); - assertUnclosedLiteral("?'?it\\'?s \n ?it\\'?s?"); - assertUnclosedLiteral("?'''?it\\'?s \n ?it\\'?s'?"); + assertUnclosedLiteral(parser, "?'?it\\'?s \n ?it\\'?s'?"); + assertUnclosedLiteral(parser, "?'?it\\'?s \n ?it\\'?s?"); + assertUnclosedLiteral(parser, "?'''?it\\'?s \n ?it\\'?s'?"); - assertThat( + assertEquals( + "select 1, @p1, 'test?test', \"test?test\", foo.* from `foo` where col1=@p2 and col2='test' and col3=@p3 and col4='?' and col5=\"?\" and col6='?''?''?'", parser.convertPositionalParametersToNamedParameters( '?', "select 1, ?, 'test?test', \"test?test\", foo.* from `foo` where col1=? and col2='test' and col3=? and col4='?' and col5=\"?\" and col6='?''?''?'") - .sqlWithNamedParameters, - is( - equalTo( - "select 1, @p1, 'test?test', \"test?test\", foo.* from `foo` where col1=@p2 and col2='test' and col3=@p3 and col4='?' and col5=\"?\" and col6='?''?''?'"))); + .sqlWithNamedParameters); - assertThat( + assertEquals( + "select * " + "from foo " + "where name=@p1 " + "and col2 like @p2 " + "and col3 > @p3", parser.convertPositionalParametersToNamedParameters( '?', "select * " + "from foo " + "where name=? " + "and col2 like ? " + "and col3 > ?") - .sqlWithNamedParameters, - is( - equalTo( - "select * " - + "from foo " - + "where name=@p1 " - + "and col2 like @p2 " - + "and col3 > @p3"))); - assertThat( + .sqlWithNamedParameters); + assertEquals( + "select * " + "from foo " + "where id between @p1 and @p2", parser.convertPositionalParametersToNamedParameters( '?', "select * " + "from foo " + "where id between ? and ?") - .sqlWithNamedParameters, - is(equalTo("select * " + "from foo " + "where id between @p1 and @p2"))); - assertThat( + .sqlWithNamedParameters); + assertEquals( + "select * " + "from foo " + "limit @p1 offset @p2", parser.convertPositionalParametersToNamedParameters( '?', "select * " + "from foo " + "limit ? offset ?") - .sqlWithNamedParameters, - is(equalTo("select * " + "from foo " + "limit @p1 offset @p2"))); - assertThat( + .sqlWithNamedParameters); + assertEquals( + "select * " + + "from foo " + + "where col1=@p1 " + + "and col2 like @p2 " + + "and col3 > @p3 " + + "and col4 < @p4 " + + "and col5 != @p5 " + + "and col6 not in (@p6, @p7, @p8) " + + "and col7 in (@p9, @p10, @p11) " + + "and col8 between @p12 and @p13", parser.convertPositionalParametersToNamedParameters( '?', "select * " @@ -1157,22 +1157,10 @@ public void testGoogleStandardSQLDialectConvertPositionalParametersToNamedParame + "and col6 not in (?, ?, ?) " + "and col7 in (?, ?, ?) " + "and col8 between ? and ?") - .sqlWithNamedParameters, - is( - equalTo( - "select * " - + "from foo " - + "where col1=@p1 " - + "and col2 like @p2 " - + "and col3 > @p3 " - + "and col4 < @p4 " - + "and col5 != @p5 " - + "and col6 not in (@p6, @p7, @p8) " - + "and col7 in (@p9, @p10, @p11) " - + "and col8 between @p12 and @p13"))); + .sqlWithNamedParameters); } - private enum CommentInjector { + enum CommentInjector { NONE { @Override String inject(String sql, String comment) { @@ -1213,57 +1201,57 @@ public void testPostgreSQLDialectDialectConvertPositionalParametersToNamedParame "/* comment /* with nested */ comment */" }) { for (CommentInjector injector : CommentInjector.values()) { - assertThat( - parser.convertPositionalParametersToNamedParameters( - '?', injector.inject("select * %sfrom foo where name=?", comment)) - .sqlWithNamedParameters) - .isEqualTo(injector.inject("select * %sfrom foo where name=$1", comment)); - assertThat( - parser.convertPositionalParametersToNamedParameters( - '?', injector.inject("?%s'?test?\"?test?\"?'?", comment)) - .sqlWithNamedParameters) - .isEqualTo(injector.inject("$1%s'?test?\"?test?\"?'$2", comment)); - assertThat( - parser.convertPositionalParametersToNamedParameters( - '?', injector.inject("?'?it\\''?s'%s?", comment)) - .sqlWithNamedParameters) - .isEqualTo(injector.inject("$1'?it\\''?s'%s$2", comment)); - assertThat( - parser.convertPositionalParametersToNamedParameters( - '?', injector.inject("?'?it\\\"?s'%s?", comment)) - .sqlWithNamedParameters) - .isEqualTo(injector.inject("$1'?it\\\"?s'%s$2", comment)); - assertThat( - parser.convertPositionalParametersToNamedParameters( - '?', injector.inject("?\"?it\\\"\"?s\"%s?", comment)) - .sqlWithNamedParameters) - .isEqualTo(injector.inject("$1\"?it\\\"\"?s\"%s$2", comment)); - assertThat( - parser.convertPositionalParametersToNamedParameters( - '?', injector.inject("?%s'''?it\\''?s'''?", comment)) - .sqlWithNamedParameters) - .isEqualTo(injector.inject("$1%s'''?it\\''?s'''$2", comment)); - assertThat( - parser.convertPositionalParametersToNamedParameters( - '?', injector.inject("?\"\"\"?it\\\"\"?s\"\"\"%s?", comment)) - .sqlWithNamedParameters) - .isEqualTo(injector.inject("$1\"\"\"?it\\\"\"?s\"\"\"%s$2", comment)); - - assertThat( - parser.convertPositionalParametersToNamedParameters( - '?', injector.inject("?$$?it$?s$$%s?", comment)) - .sqlWithNamedParameters) - .isEqualTo(injector.inject("$1$$?it$?s$$%s$2", comment)); - assertThat( - parser.convertPositionalParametersToNamedParameters( - '?', injector.inject("?$tag$?it$$?s$tag$%s?", comment)) - .sqlWithNamedParameters) - .isEqualTo(injector.inject("$1$tag$?it$$?s$tag$%s$2", comment)); - assertThat( - parser.convertPositionalParametersToNamedParameters( - '?', injector.inject("?%s$$?it\\'?s \n ?it\\'?s$$?", comment)) - .sqlWithNamedParameters) - .isEqualTo(injector.inject("$1%s$$?it\\'?s \n ?it\\'?s$$$2", comment)); + assertEquals( + injector.inject("select * %sfrom foo where name=$1", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("select * %sfrom foo where name=?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("$1%s'?test?\"?test?\"?'$2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?%s'?test?\"?test?\"?'?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("$1'?it\\''?s'%s$2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?'?it\\''?s'%s?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("$1'?it\\\"?s'%s$2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?'?it\\\"?s'%s?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("$1\"?it\\\"\"?s\"%s$2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?\"?it\\\"\"?s\"%s?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("$1%s'''?it\\''?s'''$2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?%s'''?it\\''?s'''?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("$1\"\"\"?it\\\"\"?s\"\"\"%s$2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?\"\"\"?it\\\"\"?s\"\"\"%s?", comment)) + .sqlWithNamedParameters); + + assertEquals( + injector.inject("$1$$?it$?s$$%s$2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?$$?it$?s$$%s?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("$1$tag$?it$$?s$tag$%s$2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?$tag$?it$$?s$tag$%s?", comment)) + .sqlWithNamedParameters); + assertEquals( + injector.inject("$1%s$$?it\\'?s \n ?it\\'?s$$$2", comment), + parser.convertPositionalParametersToNamedParameters( + '?', injector.inject("?%s$$?it\\'?s \n ?it\\'?s$$?", comment)) + .sqlWithNamedParameters); // Note: PostgreSQL allows a single-quoted string literal to contain line feeds. assertEquals( @@ -1271,27 +1259,32 @@ public void testPostgreSQLDialectDialectConvertPositionalParametersToNamedParame parser.convertPositionalParametersToNamedParameters( '?', injector.inject("?'?it\\''?s \n ?it\\''?s'%s?", comment)) .sqlWithNamedParameters); - assertUnclosedLiteral("?'?it\\''?s \n ?it\\''?s?"); + assertUnclosedLiteral(parser, "?'?it\\''?s \n ?it\\''?s?"); assertEquals( injector.inject("$1%s'''?it\\''?s \n ?it\\''?s'$2", comment), parser.convertPositionalParametersToNamedParameters( '?', injector.inject("?%s'''?it\\''?s \n ?it\\''?s'?", comment)) .sqlWithNamedParameters); - assertThat( + assertEquals( + injector.inject( + "select 1, $1, 'test?test', \"test?test\", %sfoo.* from `foo` where col1=$2 and col2='test' and col3=$3 and col4='?' and col5=\"?\" and col6='?''?''?'", + comment), parser.convertPositionalParametersToNamedParameters( '?', injector.inject( "select 1, ?, 'test?test', \"test?test\", %sfoo.* from `foo` where col1=? and col2='test' and col3=? and col4='?' and col5=\"?\" and col6='?''?''?'", comment)) - .sqlWithNamedParameters, - is( - equalTo( - injector.inject( - "select 1, $1, 'test?test', \"test?test\", %sfoo.* from `foo` where col1=$2 and col2='test' and col3=$3 and col4='?' and col5=\"?\" and col6='?''?''?'", - comment)))); + .sqlWithNamedParameters); - assertThat( + assertEquals( + injector.inject( + "select * " + + "%sfrom foo " + + "where name=$1 " + + "and col2 like $2 " + + "and col3 > $3", + comment), parser.convertPositionalParametersToNamedParameters( '?', injector.inject( @@ -1301,36 +1294,34 @@ public void testPostgreSQLDialectDialectConvertPositionalParametersToNamedParame + "and col2 like ? " + "and col3 > ?", comment)) - .sqlWithNamedParameters, - is( - equalTo( - injector.inject( - "select * " - + "%sfrom foo " - + "where name=$1 " - + "and col2 like $2 " - + "and col3 > $3", - comment)))); - assertThat( + .sqlWithNamedParameters); + assertEquals( + injector.inject("select * " + "from foo " + "where id between $1%s and $2", comment), parser.convertPositionalParametersToNamedParameters( '?', injector.inject( "select * " + "from foo " + "where id between ?%s and ?", comment)) - .sqlWithNamedParameters, - is( - equalTo( - injector.inject( - "select * " + "from foo " + "where id between $1%s and $2", comment)))); - assertThat( + .sqlWithNamedParameters); + assertEquals( + injector.inject("select * " + "from foo " + "limit $1 %s offset $2", comment), parser.convertPositionalParametersToNamedParameters( '?', injector.inject("select * " + "from foo " + "limit ? %s offset ?", comment)) - .sqlWithNamedParameters, - is( - equalTo( - injector.inject( - "select * " + "from foo " + "limit $1 %s offset $2", comment)))); - assertThat( + .sqlWithNamedParameters); + assertEquals( + injector.inject( + "select * " + + "from foo " + + "where col1=$1 " + + "and col2 like $2 " + + " %s " + + "and col3 > $3 " + + "and col4 < $4 " + + "and col5 != $5 " + + "and col6 not in ($6, $7, $8) " + + "and col7 in ($9, $10, $11) " + + "and col8 between $12 and $13", + comment), parser.convertPositionalParametersToNamedParameters( '?', injector.inject( @@ -1346,22 +1337,7 @@ public void testPostgreSQLDialectDialectConvertPositionalParametersToNamedParame + "and col7 in (?, ?, ?) " + "and col8 between ? and ?", comment)) - .sqlWithNamedParameters, - is( - equalTo( - injector.inject( - "select * " - + "from foo " - + "where col1=$1 " - + "and col2 like $2 " - + " %s " - + "and col3 > $3 " - + "and col4 < $4 " - + "and col5 != $5 " - + "and col6 not in ($6, $7, $8) " - + "and col7 in ($9, $10, $11) " - + "and col8 between $12 and $13", - comment)))); + .sqlWithNamedParameters); } } } @@ -1600,11 +1576,11 @@ public void testPostgreSQLReturningClause() { } int skipSingleLineComment(String sql, int startIndex) { - return PostgreSQLStatementParser.skipSingleLineComment(sql, startIndex, null); + return AbstractStatementParser.skipSingleLineComment(sql, startIndex, null); } int skipMultiLineComment(String sql, int startIndex) { - return PostgreSQLStatementParser.skipMultiLineComment(sql, startIndex, null); + return parser.skipMultiLineComment(sql, startIndex, null); } @Test @@ -1714,18 +1690,20 @@ public void testStatementCache_ParameterizedStatement() { assertEquals(1, stats.hitCount()); } - private void assertUnclosedLiteral(String sql) { - try { - parser.convertPositionalParametersToNamedParameters('?', sql); - fail("missing expected exception"); - } catch (SpannerException e) { - assertThat(e.getErrorCode()).isSameInstanceAs(ErrorCode.INVALID_ARGUMENT); - assertThat(e.getMessage()) - .startsWith( - ErrorCode.INVALID_ARGUMENT.name() - + ": SQL statement contains an unclosed literal: " - + sql); - } + static void assertUnclosedLiteral(AbstractStatementParser parser, String sql) { + SpannerException exception = + assertThrows( + SpannerException.class, + () -> parser.convertPositionalParametersToNamedParameters('?', sql)); + assertEquals(ErrorCode.INVALID_ARGUMENT, exception.getErrorCode()); + assertTrue( + exception.getMessage(), + exception + .getMessage() + .startsWith( + ErrorCode.INVALID_ARGUMENT.name() + + ": SQL statement contains an unclosed literal: " + + sql)); } @SuppressWarnings("unchecked") diff --git a/grpc-google-cloud-spanner-admin-database-v1/pom.xml b/grpc-google-cloud-spanner-admin-database-v1/pom.xml index 2bb8ac86b99..2ca9a97a48e 100644 --- a/grpc-google-cloud-spanner-admin-database-v1/pom.xml +++ b/grpc-google-cloud-spanner-admin-database-v1/pom.xml @@ -4,13 +4,13 @@ 4.0.0 com.google.api.grpc grpc-google-cloud-spanner-admin-database-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT grpc-google-cloud-spanner-admin-database-v1 GRPC library for grpc-google-cloud-spanner-admin-database-v1 com.google.cloud google-cloud-spanner-parent - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT diff --git a/grpc-google-cloud-spanner-admin-instance-v1/pom.xml b/grpc-google-cloud-spanner-admin-instance-v1/pom.xml index c1a962d26fe..8c48c7f332d 100644 --- a/grpc-google-cloud-spanner-admin-instance-v1/pom.xml +++ b/grpc-google-cloud-spanner-admin-instance-v1/pom.xml @@ -4,13 +4,13 @@ 4.0.0 com.google.api.grpc grpc-google-cloud-spanner-admin-instance-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT grpc-google-cloud-spanner-admin-instance-v1 GRPC library for grpc-google-cloud-spanner-admin-instance-v1 com.google.cloud google-cloud-spanner-parent - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT diff --git a/grpc-google-cloud-spanner-executor-v1/pom.xml b/grpc-google-cloud-spanner-executor-v1/pom.xml index b13409b4b71..ee648c3d8c3 100644 --- a/grpc-google-cloud-spanner-executor-v1/pom.xml +++ b/grpc-google-cloud-spanner-executor-v1/pom.xml @@ -4,13 +4,13 @@ 4.0.0 com.google.api.grpc grpc-google-cloud-spanner-executor-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT grpc-google-cloud-spanner-executor-v1 GRPC library for google-cloud-spanner com.google.cloud google-cloud-spanner-parent - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT diff --git a/grpc-google-cloud-spanner-v1/pom.xml b/grpc-google-cloud-spanner-v1/pom.xml index a61e657b209..de603820479 100644 --- a/grpc-google-cloud-spanner-v1/pom.xml +++ b/grpc-google-cloud-spanner-v1/pom.xml @@ -4,13 +4,13 @@ 4.0.0 com.google.api.grpc grpc-google-cloud-spanner-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT grpc-google-cloud-spanner-v1 GRPC library for grpc-google-cloud-spanner-v1 com.google.cloud google-cloud-spanner-parent - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT diff --git a/pom.xml b/pom.xml index 9884574355d..3f727352edc 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ com.google.cloud google-cloud-spanner-parent pom - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT Google Cloud Spanner Parent https://github.com/googleapis/java-spanner @@ -14,7 +14,7 @@ com.google.cloud sdk-platform-java-config - 3.27.0 + 3.28.1 @@ -61,47 +61,47 @@ com.google.api.grpc proto-google-cloud-spanner-admin-instance-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT com.google.api.grpc proto-google-cloud-spanner-executor-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT com.google.api.grpc grpc-google-cloud-spanner-executor-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT com.google.api.grpc proto-google-cloud-spanner-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT com.google.api.grpc proto-google-cloud-spanner-admin-database-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT com.google.api.grpc grpc-google-cloud-spanner-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT com.google.api.grpc grpc-google-cloud-spanner-admin-instance-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT com.google.api.grpc grpc-google-cloud-spanner-admin-database-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT com.google.cloud google-cloud-spanner - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT @@ -153,7 +153,7 @@ org.apache.maven.plugins maven-compiler-plugin - 3.12.1 + 3.13.0 1.8 1.8 diff --git a/proto-google-cloud-spanner-admin-database-v1/pom.xml b/proto-google-cloud-spanner-admin-database-v1/pom.xml index a4d7bc4b1df..056f74268d3 100644 --- a/proto-google-cloud-spanner-admin-database-v1/pom.xml +++ b/proto-google-cloud-spanner-admin-database-v1/pom.xml @@ -4,13 +4,13 @@ 4.0.0 com.google.api.grpc proto-google-cloud-spanner-admin-database-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT proto-google-cloud-spanner-admin-database-v1 PROTO library for proto-google-cloud-spanner-admin-database-v1 com.google.cloud google-cloud-spanner-parent - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT diff --git a/proto-google-cloud-spanner-admin-instance-v1/pom.xml b/proto-google-cloud-spanner-admin-instance-v1/pom.xml index 81d312af696..69e2ace8786 100644 --- a/proto-google-cloud-spanner-admin-instance-v1/pom.xml +++ b/proto-google-cloud-spanner-admin-instance-v1/pom.xml @@ -4,13 +4,13 @@ 4.0.0 com.google.api.grpc proto-google-cloud-spanner-admin-instance-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT proto-google-cloud-spanner-admin-instance-v1 PROTO library for proto-google-cloud-spanner-admin-instance-v1 com.google.cloud google-cloud-spanner-parent - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT diff --git a/proto-google-cloud-spanner-executor-v1/pom.xml b/proto-google-cloud-spanner-executor-v1/pom.xml index 3c1163f2285..91283ecb9e4 100644 --- a/proto-google-cloud-spanner-executor-v1/pom.xml +++ b/proto-google-cloud-spanner-executor-v1/pom.xml @@ -4,13 +4,13 @@ 4.0.0 com.google.api.grpc proto-google-cloud-spanner-executor-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT proto-google-cloud-spanner-executor-v1 Proto library for google-cloud-spanner com.google.cloud google-cloud-spanner-parent - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT diff --git a/proto-google-cloud-spanner-v1/pom.xml b/proto-google-cloud-spanner-v1/pom.xml index fef08e01a18..ceb83bde59e 100644 --- a/proto-google-cloud-spanner-v1/pom.xml +++ b/proto-google-cloud-spanner-v1/pom.xml @@ -4,13 +4,13 @@ 4.0.0 com.google.api.grpc proto-google-cloud-spanner-v1 - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT proto-google-cloud-spanner-v1 PROTO library for proto-google-cloud-spanner-v1 com.google.cloud google-cloud-spanner-parent - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT diff --git a/samples/install-without-bom/pom.xml b/samples/install-without-bom/pom.xml index ffc9caf196f..143fe8d00bc 100644 --- a/samples/install-without-bom/pom.xml +++ b/samples/install-without-bom/pom.xml @@ -23,8 +23,8 @@ 1.8 UTF-8 0.31.1 - 2.36.0 - 3.37.0 + 2.37.0 + 3.38.0 diff --git a/samples/native-image/pom.xml b/samples/native-image/pom.xml index ea90374c400..b009be4637d 100644 --- a/samples/native-image/pom.xml +++ b/samples/native-image/pom.xml @@ -29,7 +29,7 @@ com.google.cloud libraries-bom - 26.33.0 + 26.34.0 pom import diff --git a/samples/snapshot/pom.xml b/samples/snapshot/pom.xml index f32eed55288..11efd20a47c 100644 --- a/samples/snapshot/pom.xml +++ b/samples/snapshot/pom.xml @@ -23,8 +23,8 @@ 1.8 UTF-8 0.31.1 - 2.36.0 - 3.37.0 + 2.37.0 + 3.38.0 @@ -32,7 +32,7 @@ com.google.cloud google-cloud-spanner - 6.61.1-SNAPSHOT + 6.62.1-SNAPSHOT diff --git a/samples/snippets/pom.xml b/samples/snippets/pom.xml index 0d149d8e78f..655c9195864 100644 --- a/samples/snippets/pom.xml +++ b/samples/snippets/pom.xml @@ -34,7 +34,7 @@ com.google.cloud libraries-bom - 26.33.0 + 26.34.0 pom import diff --git a/versions.txt b/versions.txt index 3ebf8d00374..79dd5517204 100644 --- a/versions.txt +++ b/versions.txt @@ -1,13 +1,13 @@ # Format: # module:released-version:current-version -proto-google-cloud-spanner-admin-instance-v1:6.61.0:6.61.1-SNAPSHOT -proto-google-cloud-spanner-v1:6.61.0:6.61.1-SNAPSHOT -proto-google-cloud-spanner-admin-database-v1:6.61.0:6.61.1-SNAPSHOT -grpc-google-cloud-spanner-v1:6.61.0:6.61.1-SNAPSHOT -grpc-google-cloud-spanner-admin-instance-v1:6.61.0:6.61.1-SNAPSHOT -grpc-google-cloud-spanner-admin-database-v1:6.61.0:6.61.1-SNAPSHOT -google-cloud-spanner:6.61.0:6.61.1-SNAPSHOT -google-cloud-spanner-executor:6.61.0:6.61.1-SNAPSHOT -proto-google-cloud-spanner-executor-v1:6.61.0:6.61.1-SNAPSHOT -grpc-google-cloud-spanner-executor-v1:6.61.0:6.61.1-SNAPSHOT +proto-google-cloud-spanner-admin-instance-v1:6.62.0:6.62.1-SNAPSHOT +proto-google-cloud-spanner-v1:6.62.0:6.62.1-SNAPSHOT +proto-google-cloud-spanner-admin-database-v1:6.62.0:6.62.1-SNAPSHOT +grpc-google-cloud-spanner-v1:6.62.0:6.62.1-SNAPSHOT +grpc-google-cloud-spanner-admin-instance-v1:6.62.0:6.62.1-SNAPSHOT +grpc-google-cloud-spanner-admin-database-v1:6.62.0:6.62.1-SNAPSHOT +google-cloud-spanner:6.62.0:6.62.1-SNAPSHOT +google-cloud-spanner-executor:6.62.0:6.62.1-SNAPSHOT +proto-google-cloud-spanner-executor-v1:6.62.0:6.62.1-SNAPSHOT +grpc-google-cloud-spanner-executor-v1:6.62.0:6.62.1-SNAPSHOT