From db17d8d1bbc649a22401e77d494558eb38bdc219 Mon Sep 17 00:00:00 2001 From: Rahul Yadav Date: Fri, 14 Nov 2025 18:59:29 +0530 Subject: [PATCH 1/3] fix: allow DML THEN RETURN with retryAbortsInternally=false --- .../spanner/connection/ReadWriteTransaction.java | 14 +++++++++++++- .../connection/ReadWriteTransactionTest.java | 16 ++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ReadWriteTransaction.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ReadWriteTransaction.java index 5b52214d11e..c0e464ee5e6 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ReadWriteTransaction.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ReadWriteTransaction.java @@ -689,7 +689,19 @@ public ApiFuture executeQueryAsync( InterceptorsUsage.IGNORE_INTERCEPTORS, ImmutableList.of(SpannerGrpc.getExecuteStreamingSqlMethod())); } else { - res = super.executeQueryAsync(callType, statement, analyzeMode, options); + // Handle both SELECT queries and DML with THEN RETURN without delegating to the base class, + // which rejects non-SELECT statements. + res = + executeStatementAsync( + callType, + statement, + () -> { + checkTimedOut(); + checkAborted(); + return DirectExecuteResultSet.ofResultSet( + internalExecuteQuery(statement, analyzeMode, options)); + }, + SpannerGrpc.getExecuteStreamingSqlMethod()); } ApiFutures.addCallback(res, new StatementResultCallback<>(), MoreExecutors.directExecutor()); return res; diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ReadWriteTransactionTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ReadWriteTransactionTest.java index 947fe9d33cf..e3a7550e5fb 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ReadWriteTransactionTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ReadWriteTransactionTest.java @@ -287,6 +287,22 @@ public void testExecuteUpdate() { assertThat(get(transaction.executeUpdateAsync(CallType.SYNC, parsedStatement)), is(1L)); } + @Test + public void testExecuteQueryWithDmlReturningWithoutRetry() { + ParsedStatement parsedStatement = mock(ParsedStatement.class); + when(parsedStatement.getType()).thenReturn(StatementType.UPDATE); + when(parsedStatement.isUpdate()).thenReturn(true); + when(parsedStatement.hasReturningClause()).thenReturn(true); + Statement statement = + Statement.of("INSERT INTO TEST (ID, NAME) VALUES (1, 'x') THEN RETURN *"); + when(parsedStatement.getStatement()).thenReturn(statement); + + ReadWriteTransaction transaction = createSubject(/* commitBehavior= */ CommitBehavior.SUCCEED); + ResultSet rs = + get(transaction.executeQueryAsync(CallType.SYNC, parsedStatement, AnalyzeMode.NONE)); + assertThat(rs, is(notNullValue())); + } + @Test public void testGetCommitTimestampBeforeCommit() { ParsedStatement parsedStatement = mock(ParsedStatement.class); From 20c3f5068ef4137120e69154d437a08a79d8296e Mon Sep 17 00:00:00 2001 From: cloud-java-bot Date: Fri, 14 Nov 2025 13:32:56 +0000 Subject: [PATCH 2/3] chore: generate libraries at Fri Nov 14 13:30:19 UTC 2025 --- README.md | 4 ++-- .../cloud/spanner/connection/ReadWriteTransactionTest.java | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 73d8f221816..aa140bda1db 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.70.0 + 26.71.0 pom import @@ -41,7 +41,7 @@ If you are using Maven without the BOM, add this to your dependencies: com.google.cloud google-cloud-spanner - 6.102.0 + 6.102.1 ``` diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ReadWriteTransactionTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ReadWriteTransactionTest.java index e3a7550e5fb..7d0fa94c9b0 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ReadWriteTransactionTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ReadWriteTransactionTest.java @@ -293,8 +293,7 @@ public void testExecuteQueryWithDmlReturningWithoutRetry() { when(parsedStatement.getType()).thenReturn(StatementType.UPDATE); when(parsedStatement.isUpdate()).thenReturn(true); when(parsedStatement.hasReturningClause()).thenReturn(true); - Statement statement = - Statement.of("INSERT INTO TEST (ID, NAME) VALUES (1, 'x') THEN RETURN *"); + Statement statement = Statement.of("INSERT INTO TEST (ID, NAME) VALUES (1, 'x') THEN RETURN *"); when(parsedStatement.getStatement()).thenReturn(statement); ReadWriteTransaction transaction = createSubject(/* commitBehavior= */ CommitBehavior.SUCCEED); From f4f82fed14c15c4ceeb0419c56987c892d2f28d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Fri, 14 Nov 2025 14:59:39 +0100 Subject: [PATCH 3/3] test: add more tests --- .../connection/AbstractMockServerTest.java | 6 ++- .../AutoDmlBatchMockServerTest.java | 4 ++ .../connection/TransactionMockServerTest.java | 43 ++++++++++++++++++- 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AbstractMockServerTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AbstractMockServerTest.java index cf546e95888..a78df0471e3 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AbstractMockServerTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AbstractMockServerTest.java @@ -110,7 +110,7 @@ public abstract class AbstractMockServerTest { .setMetadata(SINGLE_COL_INT64_RESULTSET_METADATA) .build(); public static final com.google.spanner.v1.ResultSet UPDATE_RETURNING_RESULTSET = - com.google.spanner.v1.ResultSet.newBuilder() + ResultSet.newBuilder() .setStats(ResultSetStats.newBuilder().setRowCountExact(1)) .setMetadata( ResultSetMetadata.newBuilder() @@ -121,6 +121,10 @@ public abstract class AbstractMockServerTest { .setName("col") .setType(Type.newBuilder().setCodeValue(TypeCode.INT64_VALUE)) .build()))) + .addRows( + ListValue.newBuilder() + .addValues(Value.newBuilder().setStringValue("1").build()) + .build()) .build(); protected static final ResultSet SELECT1_RESULTSET = diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AutoDmlBatchMockServerTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AutoDmlBatchMockServerTest.java index a7467997c6a..b359bcb60cc 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AutoDmlBatchMockServerTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AutoDmlBatchMockServerTest.java @@ -97,6 +97,8 @@ public void testDmlWithReturningAfterDml() { // DML with a THEN RETURN clause cannot be batched. This therefore flushes the batch and // executes the INSERT ... THEN RETURN statement as a separate ExecuteSqlRequest. try (ResultSet resultSet = connection.executeQuery(INSERT_RETURNING_STATEMENT)) { + assertTrue(resultSet.next()); + assertEquals(1L, resultSet.getLong(0)); assertFalse(resultSet.next()); } @@ -123,6 +125,8 @@ public void testDmlWithReturningAfterDml_usingExecute() { StatementResult result = connection.execute(INSERT_RETURNING_STATEMENT); assertEquals(ResultType.RESULT_SET, result.getResultType()); try (ResultSet resultSet = result.getResultSet()) { + assertTrue(resultSet.next()); + assertEquals(1L, resultSet.getLong(0)); assertFalse(resultSet.next()); } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/TransactionMockServerTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/TransactionMockServerTest.java index 31cc1acd19e..45f68b11a5b 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/TransactionMockServerTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/TransactionMockServerTest.java @@ -31,6 +31,7 @@ import com.google.cloud.spanner.SpannerException; import com.google.cloud.spanner.Statement; import com.google.cloud.spanner.connection.ITAbstractSpannerTest.ITConnection; +import com.google.cloud.spanner.connection.StatementResult.ResultType; import com.google.spanner.v1.BeginTransactionRequest; import com.google.spanner.v1.CommitRequest; import com.google.spanner.v1.ExecuteBatchDmlRequest; @@ -357,5 +358,45 @@ public long nanoTime() { } @Test - public void testTransactionTimeoutInAutoCommit() {} + public void testCanUseAllMethodsWithInternalRetriesDisabled() { + // Verify that all query/update methods work as expected when internal retries have been + // disabled. + try (Connection connection = createConnection()) { + connection.setAutocommit(false); + connection.setRetryAbortsInternally(false); + + try (ResultSet result = connection.executeQuery(SELECT1_STATEMENT)) { + assertTrue(result.next()); + assertEquals(1L, result.getLong(0)); + assertFalse(result.next()); + } + assertEquals(1, connection.executeUpdate(INSERT_STATEMENT)); + try (ResultSet result = connection.executeQuery(INSERT_RETURNING_STATEMENT)) { + assertTrue(result.next()); + assertEquals(1L, result.getLong(0)); + assertFalse(result.next()); + } + + StatementResult statementResult = connection.execute(SELECT1_STATEMENT); + assertEquals(ResultType.RESULT_SET, statementResult.getResultType()); + try (ResultSet result = statementResult.getResultSet()) { + assertTrue(result.next()); + assertEquals(1L, result.getLong(0)); + assertFalse(result.next()); + } + + statementResult = connection.execute(INSERT_STATEMENT); + assertEquals(ResultType.UPDATE_COUNT, statementResult.getResultType()); + assertEquals(1L, statementResult.getUpdateCount().longValue()); + + statementResult = connection.execute(INSERT_RETURNING_STATEMENT); + assertEquals(ResultType.RESULT_SET, statementResult.getResultType()); + try (ResultSet result = statementResult.getResultSet()) { + assertTrue(result.next()); + assertEquals(1L, result.getLong(0)); + assertFalse(result.next()); + } + connection.commit(); + } + } }