From 7c5e3da4c128cb9220213db8b3e2291e33566715 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 16 Nov 2022 12:47:03 +0100 Subject: [PATCH] feat: analyze update returns param types (#2156) * feat: analyzeStatement for both DML and queries Add support for `analyzeStatement` that works for both DML and queries. * feat: analyze update returns param types Add the method analyzeUpdateStatement that returns the undeclared parameters in an update statement. This allows connection based APIs to return more metadata for a statement than is currently possible: 1. JDBC should return the parameter data types when PreparedStatement#getMetaData() is called. 2. PGAdapter should return the parameter types when a DescribeStatement message is received. * chore: add clirr differences * chore: address review comments --- .../clirr-ignored-differences.xml | 15 ++ .../cloud/spanner/AbstractResultSet.java | 9 +- .../cloud/spanner/AsyncResultSetImpl.java | 6 + .../cloud/spanner/ForwardingResultSet.java | 6 + .../google/cloud/spanner/NoRowsResultSet.java | 63 ++++++++ .../com/google/cloud/spanner/ResultSet.java | 9 ++ .../com/google/cloud/spanner/ResultSets.java | 7 + .../com/google/cloud/spanner/SessionPool.java | 8 +- .../cloud/spanner/TransactionContext.java | 19 +++ .../cloud/spanner/TransactionRunnerImpl.java | 20 ++- .../cloud/spanner/connection/Connection.java | 20 +++ .../spanner/connection/ConnectionImpl.java | 26 ++- .../cloud/spanner/connection/DdlBatch.java | 3 +- .../connection/DirectExecuteResultSet.java | 6 + .../cloud/spanner/connection/DmlBatch.java | 3 +- .../connection/ReadOnlyTransaction.java | 4 +- .../connection/ReadWriteTransaction.java | 68 +++++--- .../ReplaceableForwardingResultSet.java | 6 + .../connection/SingleUseTransaction.java | 29 ++-- .../cloud/spanner/connection/UnitOfWork.java | 10 +- .../cloud/spanner/DatabaseClientImplTest.java | 54 +++++++ .../cloud/spanner/TransactionContextTest.java | 6 + .../connection/AnalyzeStatementsTest.java | 153 +++++++++++++++++- .../DirectExecuteResultSetTest.java | 3 + .../ReplaceableForwardingResultSetTest.java | 5 +- 25 files changed, 496 insertions(+), 62 deletions(-) create mode 100644 google-cloud-spanner/src/main/java/com/google/cloud/spanner/NoRowsResultSet.java diff --git a/google-cloud-spanner/clirr-ignored-differences.xml b/google-cloud-spanner/clirr-ignored-differences.xml index adda4c3c85..637ca06280 100644 --- a/google-cloud-spanner/clirr-ignored-differences.xml +++ b/google-cloud-spanner/clirr-ignored-differences.xml @@ -207,4 +207,19 @@ com/google/cloud/spanner/connection/AbstractStatementParser boolean checkReturningClauseInternal(java.lang.String) + + 7012 + com/google/cloud/spanner/ResultSet + com.google.spanner.v1.ResultSetMetadata getMetadata() + + + 7012 + com/google/cloud/spanner/TransactionContext + com.google.cloud.spanner.ResultSet analyzeUpdateStatement(com.google.cloud.spanner.Statement, com.google.cloud.spanner.ReadContext$QueryAnalyzeMode, com.google.cloud.spanner.Options$UpdateOption[]) + + + 7012 + com/google/cloud/spanner/connection/Connection + com.google.cloud.spanner.ResultSet analyzeUpdateStatement(com.google.cloud.spanner.Statement, com.google.cloud.spanner.ReadContext$QueryAnalyzeMode, com.google.cloud.spanner.Options$UpdateOption[]) + diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java index 6ccb28900f..fdbea3335b 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java @@ -93,6 +93,7 @@ void onTransactionMetadata(Transaction transaction, boolean shouldIncludeId) static class GrpcResultSet extends AbstractResultSet> { private final GrpcValueIterator iterator; private final Listener listener; + private ResultSetMetadata metadata; private GrpcStruct currRow; private SpannerException error; private ResultSetStats statistics; @@ -117,7 +118,7 @@ public boolean next() throws SpannerException { } try { if (currRow == null) { - ResultSetMetadata metadata = iterator.getMetadata(); + metadata = iterator.getMetadata(); if (metadata.hasTransaction()) { listener.onTransactionMetadata( metadata.getTransaction(), iterator.isWithBeginTransaction()); @@ -146,6 +147,12 @@ public ResultSetStats getStats() { return statistics; } + @Override + public ResultSetMetadata getMetadata() { + checkState(metadata != null, "next() call required"); + return metadata; + } + @Override public void close() { listener.onDone(iterator.isWithBeginTransaction()); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AsyncResultSetImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AsyncResultSetImpl.java index d2c63bb78e..fa7cc158c1 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AsyncResultSetImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AsyncResultSetImpl.java @@ -29,6 +29,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.common.util.concurrent.MoreExecutors; +import com.google.spanner.v1.ResultSetMetadata; import com.google.spanner.v1.ResultSetStats; import java.util.Collection; import java.util.LinkedList; @@ -572,6 +573,11 @@ public ResultSetStats getStats() { return delegateResultSet.get().getStats(); } + @Override + public ResultSetMetadata getMetadata() { + return delegateResultSet.get().getMetadata(); + } + @Override protected void checkValidState() { synchronized (monitor) { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ForwardingResultSet.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ForwardingResultSet.java index 7d97fad162..c29282879e 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ForwardingResultSet.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ForwardingResultSet.java @@ -19,6 +19,7 @@ import com.google.common.base.Preconditions; import com.google.common.base.Supplier; import com.google.common.base.Suppliers; +import com.google.spanner.v1.ResultSetMetadata; import com.google.spanner.v1.ResultSetStats; /** Forwarding implementation of ResultSet that forwards all calls to a delegate. */ @@ -76,4 +77,9 @@ public void close() { public ResultSetStats getStats() { return delegate.get().getStats(); } + + @Override + public ResultSetMetadata getMetadata() { + return delegate.get().getMetadata(); + } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/NoRowsResultSet.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/NoRowsResultSet.java new file mode 100644 index 0000000000..d78a157ca4 --- /dev/null +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/NoRowsResultSet.java @@ -0,0 +1,63 @@ +/* + * Copyright 2022 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; + +import com.google.spanner.v1.ResultSet; +import com.google.spanner.v1.ResultSetMetadata; +import com.google.spanner.v1.ResultSetStats; +import java.util.List; +import javax.annotation.Nullable; + +class NoRowsResultSet extends AbstractResultSet> { + private final ResultSetStats stats; + private final ResultSetMetadata metadata; + + NoRowsResultSet(ResultSet resultSet) { + this.stats = resultSet.getStats(); + this.metadata = resultSet.getMetadata(); + } + + @Override + protected GrpcStruct currRow() { + throw SpannerExceptionFactory.newSpannerException( + ErrorCode.FAILED_PRECONDITION, "This result set has no rows"); + } + + @Override + public boolean next() throws SpannerException { + return false; + } + + @Override + public void close() {} + + @Nullable + @Override + public ResultSetStats getStats() { + return stats; + } + + @Override + public ResultSetMetadata getMetadata() { + return metadata; + } + + @Override + public Type getType() { + return Type.struct(); + } +} diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ResultSet.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ResultSet.java index c2d268231a..cd6fa10b99 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ResultSet.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ResultSet.java @@ -17,6 +17,7 @@ package com.google.cloud.spanner; import com.google.cloud.spanner.Options.QueryOption; +import com.google.spanner.v1.ResultSetMetadata; import com.google.spanner.v1.ResultSetStats; import javax.annotation.Nullable; @@ -73,4 +74,12 @@ public interface ResultSet extends AutoCloseable, StructReader { */ @Nullable ResultSetStats getStats(); + + /** + * Returns the {@link ResultSetMetadata} for this {@link ResultSet}. This is method may only be + * called after calling {@link ResultSet#next()} at least once. + */ + default ResultSetMetadata getMetadata() { + throw new UnsupportedOperationException("Method should be overridden"); + } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ResultSets.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ResultSets.java index 6eacd3208e..fa054ba0cd 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ResultSets.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ResultSets.java @@ -29,6 +29,7 @@ import com.google.common.base.Supplier; import com.google.common.collect.Lists; import com.google.common.util.concurrent.ThreadFactoryBuilder; +import com.google.spanner.v1.ResultSetMetadata; import com.google.spanner.v1.ResultSetStats; import java.math.BigDecimal; import java.util.List; @@ -158,6 +159,12 @@ public ResultSetStats getStats() { "ResultSetStats are available only for results returned from analyzeQuery() calls"); } + @Override + public ResultSetMetadata getMetadata() { + throw new UnsupportedOperationException( + "ResultSetMetadata are available only for results that were returned from Cloud Spanner"); + } + @Override public int getColumnCount() { return getType().getStructFields().size(); 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 5502648b59..ff11df9116 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 @@ -717,8 +717,14 @@ public ApiFuture bufferAsync(Iterable mutations) { @Override public ResultSetStats analyzeUpdate( Statement statement, QueryAnalyzeMode analyzeMode, UpdateOption... options) { + return analyzeUpdateStatement(statement, analyzeMode, options).getStats(); + } + + @Override + public ResultSet analyzeUpdateStatement( + Statement statement, QueryAnalyzeMode analyzeMode, UpdateOption... options) { try { - return delegate.analyzeUpdate(statement, analyzeMode, options); + return delegate.analyzeUpdateStatement(statement, analyzeMode, options); } catch (SessionNotFoundException e) { throw handler.handleSessionNotFound(e); } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionContext.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionContext.java index 65498b2f31..b858616c13 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionContext.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionContext.java @@ -135,12 +135,31 @@ default ApiFuture bufferAsync(Iterable mutations) { * the statement. {@link com.google.cloud.spanner.ReadContext.QueryAnalyzeMode#PROFILE} executes * the DML statement, returns the modified row count and execution statistics, and the effects of * the DML statement will be visible to subsequent operations in the transaction. + * + * @deprecated Use {@link #analyzeUpdateStatement(Statement, QueryAnalyzeMode, UpdateOption...)} + * instead to get both statement plan and parameter metadata */ + @Deprecated default ResultSetStats analyzeUpdate( Statement statement, QueryAnalyzeMode analyzeMode, UpdateOption... options) { throw new UnsupportedOperationException("method should be overwritten"); } + /** + * Analyzes a DML statement and returns query plan and statement parameter metadata and optionally + * execution statistics information. + * + *

{@link com.google.cloud.spanner.ReadContext.QueryAnalyzeMode#PLAN} only returns the plan and + * parameter metadata for the statement. {@link + * com.google.cloud.spanner.ReadContext.QueryAnalyzeMode#PROFILE} executes the DML statement, + * returns the modified row count and execution statistics, and the effects of the DML statement + * will be visible to subsequent operations in the transaction. + */ + default ResultSet analyzeUpdateStatement( + Statement statement, QueryAnalyzeMode analyzeMode, UpdateOption... options) { + throw new UnsupportedOperationException("method should be overwritten"); + } + /** * Executes a list of DML statements (which can include simple DML statements or DML statements * with returning clause) in a single request. The statements will be executed in order and the diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java index e18f63e97f..4557064b5e 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java @@ -43,6 +43,7 @@ import com.google.spanner.v1.ExecuteSqlRequest; import com.google.spanner.v1.ExecuteSqlRequest.QueryMode; import com.google.spanner.v1.RequestOptions; +import com.google.spanner.v1.ResultSet; import com.google.spanner.v1.ResultSetStats; import com.google.spanner.v1.RollbackRequest; import com.google.spanner.v1.Transaction; @@ -673,6 +674,17 @@ public ApiFuture bufferAsync(Iterable mutations) { @Override public ResultSetStats analyzeUpdate( Statement statement, QueryAnalyzeMode analyzeMode, UpdateOption... options) { + return internalAnalyzeStatement(statement, analyzeMode, options).getStats(); + } + + @Override + public com.google.cloud.spanner.ResultSet analyzeUpdateStatement( + Statement statement, QueryAnalyzeMode analyzeMode, UpdateOption... options) { + return new NoRowsResultSet(internalAnalyzeStatement(statement, analyzeMode, options)); + } + + private ResultSet internalAnalyzeStatement( + Statement statement, QueryAnalyzeMode analyzeMode, UpdateOption... options) { Preconditions.checkNotNull(analyzeMode); QueryMode queryMode; switch (analyzeMode) { @@ -691,12 +703,12 @@ public ResultSetStats analyzeUpdate( @Override public long executeUpdate(Statement statement, UpdateOption... options) { - ResultSetStats resultSetStats = internalExecuteUpdate(statement, QueryMode.NORMAL, options); + ResultSet resultSet = internalExecuteUpdate(statement, QueryMode.NORMAL, options); // For standard DML, using the exact row count. - return resultSetStats.getRowCountExact(); + return resultSet.getStats().getRowCountExact(); } - private ResultSetStats internalExecuteUpdate( + private ResultSet internalExecuteUpdate( Statement statement, QueryMode queryMode, UpdateOption... options) { beforeReadOrQuery(); final ExecuteSqlRequest.Builder builder = @@ -716,7 +728,7 @@ private ResultSetStats internalExecuteUpdate( throw new IllegalArgumentException( "DML response missing stats possibly due to non-DML statement as input"); } - return resultSet.getStats(); + return resultSet; } catch (Throwable t) { throw onError( SpannerExceptionFactory.asSpannerException(t), builder.getTransaction().hasBegin()); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/Connection.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/Connection.java index afb3723c14..beb36f1b56 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/Connection.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/Connection.java @@ -29,6 +29,7 @@ import com.google.cloud.spanner.Mutation; import com.google.cloud.spanner.Options.QueryOption; import com.google.cloud.spanner.Options.RpcPriority; +import com.google.cloud.spanner.Options.UpdateOption; import com.google.cloud.spanner.ReadContext.QueryAnalyzeMode; import com.google.cloud.spanner.ResultSet; import com.google.cloud.spanner.SpannerBatchUpdateException; @@ -968,11 +969,30 @@ default RpcPriority getRPCPriority() { * the statement. {@link com.google.cloud.spanner.ReadContext.QueryAnalyzeMode#PROFILE} executes * the DML statement, returns the modified row count and execution statistics, and the effects of * the DML statement will be visible to subsequent operations in the transaction. + * + * @deprecated Use {@link #analyzeUpdateStatement(Statement, QueryAnalyzeMode, UpdateOption...)} + * instead */ + @Deprecated default ResultSetStats analyzeUpdate(Statement update, QueryAnalyzeMode analyzeMode) { throw new UnsupportedOperationException("Not implemented"); } + /** + * Analyzes a DML statement and returns execution plan, undeclared parameters and optionally + * execution statistics information. + * + *

{@link com.google.cloud.spanner.ReadContext.QueryAnalyzeMode#PLAN} only returns the plan and + * undeclared parameters for the statement. {@link + * com.google.cloud.spanner.ReadContext.QueryAnalyzeMode#PROFILE} also executes the DML statement, + * returns the modified row count and execution statistics, and the effects of the DML statement + * will be visible to subsequent operations in the transaction. + */ + default ResultSet analyzeUpdateStatement( + Statement statement, QueryAnalyzeMode analyzeMode, UpdateOption... options) { + throw new UnsupportedOperationException("Not implemented"); + } + /** * Executes the given statement asynchronously as a simple DML statement. If the statement does * not contain a simple DML statement, the method will throw a {@link SpannerException}. A DML diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java index e935a607a8..ad52cc5da6 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java @@ -1065,7 +1065,8 @@ public ResultSetStats analyzeUpdate(Statement update, QueryAnalyzeMode analyzeMo if (parsedStatement.isUpdate()) { switch (parsedStatement.getType()) { case UPDATE: - return get(internalAnalyzeUpdateAsync(parsedStatement, AnalyzeMode.of(analyzeMode))); + return get(internalAnalyzeUpdateAsync(parsedStatement, AnalyzeMode.of(analyzeMode))) + .getStats(); case CLIENT_SIDE: case QUERY: case DDL: @@ -1078,6 +1079,27 @@ public ResultSetStats analyzeUpdate(Statement update, QueryAnalyzeMode analyzeMo "Statement is not an update statement: " + parsedStatement.getSqlWithoutComments()); } + @Override + public ResultSet analyzeUpdateStatement( + Statement statement, QueryAnalyzeMode analyzeMode, UpdateOption... options) { + Preconditions.checkNotNull(statement); + ConnectionPreconditions.checkState(!isClosed(), CLOSED_ERROR_MSG); + ParsedStatement parsedStatement = getStatementParser().parse(statement); + switch (parsedStatement.getType()) { + case UPDATE: + return get( + internalAnalyzeUpdateAsync(parsedStatement, AnalyzeMode.of(analyzeMode), options)); + case QUERY: + case CLIENT_SIDE: + case DDL: + case UNKNOWN: + default: + } + throw SpannerExceptionFactory.newSpannerException( + ErrorCode.INVALID_ARGUMENT, + "Statement is not an update statement: " + parsedStatement.getSqlWithoutComments()); + } + @Override public long[] executeBatchUpdate(Iterable updates) { Preconditions.checkNotNull(updates); @@ -1224,7 +1246,7 @@ private ApiFuture internalExecuteUpdateAsync( update, mergeUpdateRequestOptions(mergeUpdateStatementTag(options))); } - private ApiFuture internalAnalyzeUpdateAsync( + private ApiFuture internalAnalyzeUpdateAsync( final ParsedStatement update, AnalyzeMode analyzeMode, UpdateOption... options) { Preconditions.checkArgument( update.getType() == StatementType.UPDATE, "Statement must be an update"); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DdlBatch.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DdlBatch.java index 31a07f0573..dded72844d 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DdlBatch.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DdlBatch.java @@ -38,7 +38,6 @@ import com.google.common.base.Preconditions; import com.google.spanner.admin.database.v1.DatabaseAdminGrpc; import com.google.spanner.admin.database.v1.UpdateDatabaseDdlMetadata; -import com.google.spanner.v1.ResultSetStats; import com.google.spanner.v1.SpannerGrpc; import java.util.ArrayList; import java.util.Arrays; @@ -203,7 +202,7 @@ public ApiFuture executeUpdateAsync(ParsedStatement update, UpdateOption.. } @Override - public ApiFuture analyzeUpdateAsync( + public ApiFuture analyzeUpdateAsync( ParsedStatement update, AnalyzeMode analyzeMode, UpdateOption... options) { throw SpannerExceptionFactory.newSpannerException( ErrorCode.FAILED_PRECONDITION, "Analyzing updates is not allowed for DDL batches."); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DirectExecuteResultSet.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DirectExecuteResultSet.java index 8fb0bbe440..8690e154f4 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DirectExecuteResultSet.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DirectExecuteResultSet.java @@ -25,6 +25,7 @@ import com.google.cloud.spanner.Type; import com.google.cloud.spanner.Value; import com.google.common.base.Preconditions; +import com.google.spanner.v1.ResultSetMetadata; import com.google.spanner.v1.ResultSetStats; import java.math.BigDecimal; import java.util.List; @@ -94,6 +95,11 @@ public ResultSetStats getStats() { return null; } + @Override + public ResultSetMetadata getMetadata() { + return delegate.getMetadata(); + } + @Override public Type getType() { return delegate.getType(); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DmlBatch.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DmlBatch.java index 960f95b44d..b9384006d4 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DmlBatch.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DmlBatch.java @@ -33,7 +33,6 @@ import com.google.cloud.spanner.connection.AbstractStatementParser.StatementType; import com.google.common.base.Preconditions; import com.google.common.util.concurrent.MoreExecutors; -import com.google.spanner.v1.ResultSetStats; import java.util.ArrayList; import java.util.List; @@ -163,7 +162,7 @@ public ApiFuture executeUpdateAsync(ParsedStatement update, UpdateOption.. } @Override - public ApiFuture analyzeUpdateAsync( + public ApiFuture analyzeUpdateAsync( ParsedStatement update, AnalyzeMode analyzeMode, UpdateOption... options) { throw SpannerExceptionFactory.newSpannerException( ErrorCode.FAILED_PRECONDITION, "Analyzing updates is not allowed for DML batches."); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ReadOnlyTransaction.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ReadOnlyTransaction.java index dd14ad5bee..e391819c42 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ReadOnlyTransaction.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ReadOnlyTransaction.java @@ -25,12 +25,12 @@ import com.google.cloud.spanner.Mutation; import com.google.cloud.spanner.Options.UpdateOption; import com.google.cloud.spanner.ReadContext; +import com.google.cloud.spanner.ResultSet; import com.google.cloud.spanner.SpannerException; import com.google.cloud.spanner.SpannerExceptionFactory; import com.google.cloud.spanner.TimestampBound; import com.google.cloud.spanner.connection.AbstractStatementParser.ParsedStatement; import com.google.common.base.Preconditions; -import com.google.spanner.v1.ResultSetStats; /** * Transaction that is used when a {@link Connection} is in read-only mode or when the transaction @@ -165,7 +165,7 @@ public ApiFuture executeUpdateAsync(ParsedStatement update, UpdateOption.. } @Override - public ApiFuture analyzeUpdateAsync( + public ApiFuture analyzeUpdateAsync( ParsedStatement update, AnalyzeMode analyzeMode, UpdateOption... options) { throw SpannerExceptionFactory.newSpannerException( ErrorCode.FAILED_PRECONDITION, 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 80dc97cde0..169ca1e529 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 @@ -28,6 +28,7 @@ import com.google.api.core.ApiFutures; import com.google.api.core.SettableApiFuture; import com.google.cloud.Timestamp; +import com.google.cloud.Tuple; import com.google.cloud.spanner.AbortedDueToConcurrentModificationException; import com.google.cloud.spanner.AbortedException; import com.google.cloud.spanner.CommitResponse; @@ -52,11 +53,11 @@ import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.util.concurrent.MoreExecutors; -import com.google.spanner.v1.ResultSetStats; import com.google.spanner.v1.SpannerGrpc; import java.util.ArrayList; import java.util.LinkedList; import java.util.List; +import java.util.Objects; import java.util.concurrent.Callable; import java.util.concurrent.atomic.AtomicLong; import java.util.logging.Level; @@ -396,9 +397,12 @@ public void onSuccess(ResultSet result) {} } @Override - public ApiFuture analyzeUpdateAsync( + public ApiFuture analyzeUpdateAsync( ParsedStatement update, AnalyzeMode analyzeMode, UpdateOption... options) { - return internalExecuteUpdateAsync(update, analyzeMode, options); + return ApiFutures.transform( + internalExecuteUpdateAsync(update, analyzeMode, options), + Tuple::y, + MoreExecutors.directExecutor()); } @Override @@ -406,16 +410,28 @@ public ApiFuture executeUpdateAsync( final ParsedStatement update, final UpdateOption... options) { return ApiFutures.transform( internalExecuteUpdateAsync(update, AnalyzeMode.NONE, options), - ResultSetStats::getRowCountExact, + Tuple::x, MoreExecutors.directExecutor()); } - private ApiFuture internalExecuteUpdateAsync( + /** + * Executes the given update statement using the specified query planning mode and with the given + * options and returns the result as a {@link Tuple}. The tuple contains either a {@link + * ResultSet} with the query plan and execution statistics, or a {@link Long} that contains the + * update count that was returned for the update statement. Only one of the elements in the tuple + * will be set, and the reason that we are using a {@link Tuple} here is because Java does not + * have a standard implementation for an 'Either' class (i.e. a Tuple where only one element is + * set). An alternative would be to always return a {@link ResultSet} with the update count + * encoded in the execution stats of the result set, but this would mean that we would create + * additional {@link ResultSet} instances every time an update statement is executed in normal + * mode. + */ + private ApiFuture> internalExecuteUpdateAsync( ParsedStatement update, AnalyzeMode analyzeMode, UpdateOption... options) { Preconditions.checkNotNull(update); Preconditions.checkArgument(update.isUpdate(), "The statement is not an update statement"); checkValidTransaction(); - ApiFuture res; + ApiFuture> res; if (retryAbortsInternally) { res = executeStatementAsync( @@ -431,25 +447,25 @@ private ApiFuture internalExecuteUpdateAsync( StatementExecutionStep.EXECUTE_STATEMENT, ReadWriteTransaction.this); - ResultSetStats updateCount; + Tuple result; + long updateCount; if (analyzeMode == AnalyzeMode.NONE) { updateCount = - ResultSetStats.newBuilder() - .setRowCountExact( - get(txContextFuture) - .executeUpdate(update.getStatement(), options)) - .build(); + get(txContextFuture).executeUpdate(update.getStatement(), options); + result = Tuple.of(updateCount, null); } else { - updateCount = + ResultSet resultSet = get(txContextFuture) - .analyzeUpdate( + .analyzeUpdateStatement( update.getStatement(), analyzeMode.getQueryAnalyzeMode(), options); + updateCount = + Objects.requireNonNull(resultSet.getStats()).getRowCountExact(); + result = Tuple.of(null, resultSet); } - createAndAddRetriableUpdate( - update, analyzeMode, updateCount.getRowCountExact(), options); - return updateCount; + createAndAddRetriableUpdate(update, analyzeMode, updateCount, options); + return result; } catch (AbortedException e) { throw e; } catch (SpannerException e) { @@ -469,20 +485,20 @@ private ApiFuture internalExecuteUpdateAsync( checkTimedOut(); checkAborted(); if (analyzeMode == AnalyzeMode.NONE) { - return ResultSetStats.newBuilder() - .setRowCountExact( - get(txContextFuture).executeUpdate(update.getStatement(), options)) - .build(); + return Tuple.of( + get(txContextFuture).executeUpdate(update.getStatement(), options), null); } - return get(txContextFuture) - .analyzeUpdate( - update.getStatement(), analyzeMode.getQueryAnalyzeMode(), options); + ResultSet resultSet = + get(txContextFuture) + .analyzeUpdateStatement( + update.getStatement(), analyzeMode.getQueryAnalyzeMode(), options); + return Tuple.of(null, resultSet); }, SpannerGrpc.getExecuteSqlMethod()); } ApiFutures.addCallback( res, - new ApiFutureCallback() { + new ApiFutureCallback>() { @Override public void onFailure(Throwable t) { if (t instanceof SpannerException) { @@ -491,7 +507,7 @@ public void onFailure(Throwable t) { } @Override - public void onSuccess(ResultSetStats result) {} + public void onSuccess(Tuple result) {} }, MoreExecutors.directExecutor()); return res; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ReplaceableForwardingResultSet.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ReplaceableForwardingResultSet.java index cc9759a487..07e755b2b2 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ReplaceableForwardingResultSet.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ReplaceableForwardingResultSet.java @@ -27,6 +27,7 @@ import com.google.cloud.spanner.Type; import com.google.cloud.spanner.Value; import com.google.common.base.Preconditions; +import com.google.spanner.v1.ResultSetMetadata; import com.google.spanner.v1.ResultSetStats; import java.math.BigDecimal; import java.util.List; @@ -94,6 +95,11 @@ public ResultSetStats getStats() { return delegate.getStats(); } + @Override + public ResultSetMetadata getMetadata() { + return delegate.getMetadata(); + } + @Override public Type getType() { checkClosed(); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SingleUseTransaction.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SingleUseTransaction.java index cbde67393a..9cfd095df3 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SingleUseTransaction.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SingleUseTransaction.java @@ -24,6 +24,7 @@ import com.google.api.core.SettableApiFuture; import com.google.api.gax.longrunning.OperationFuture; import com.google.cloud.Timestamp; +import com.google.cloud.Tuple; import com.google.cloud.spanner.CommitResponse; import com.google.cloud.spanner.DatabaseClient; import com.google.cloud.spanner.ErrorCode; @@ -46,7 +47,6 @@ import com.google.common.collect.Iterables; import com.google.common.util.concurrent.MoreExecutors; import com.google.spanner.admin.database.v1.DatabaseAdminGrpc; -import com.google.spanner.v1.ResultSetStats; import com.google.spanner.v1.SpannerGrpc; import java.util.concurrent.Callable; @@ -341,7 +341,7 @@ public ApiFuture executeUpdateAsync(ParsedStatement update, UpdateOption.. res = ApiFutures.transform( executeTransactionalUpdateAsync(update, AnalyzeMode.NONE, options), - ResultSetStats::getRowCountExact, + Tuple::x, MoreExecutors.directExecutor()); break; case PARTITIONED_NON_ATOMIC: @@ -355,7 +355,7 @@ public ApiFuture executeUpdateAsync(ParsedStatement update, UpdateOption.. } @Override - public ApiFuture analyzeUpdateAsync( + public ApiFuture analyzeUpdateAsync( ParsedStatement update, AnalyzeMode analyzeMode, UpdateOption... options) { Preconditions.checkNotNull(update); Preconditions.checkArgument(update.isUpdate(), "Statement is not an update statement"); @@ -366,7 +366,10 @@ public ApiFuture analyzeUpdateAsync( "Analyzing update statements is not supported for Partitioned DML"); checkAndMarkUsed(); - return executeTransactionalUpdateAsync(update, analyzeMode, options); + return ApiFutures.transform( + executeTransactionalUpdateAsync(update, analyzeMode, options), + Tuple::y, + MoreExecutors.directExecutor()); } @Override @@ -416,23 +419,23 @@ private TransactionRunner createWriteTransaction() { return dbClient.readWriteTransaction(options); } - private ApiFuture executeTransactionalUpdateAsync( + private ApiFuture> executeTransactionalUpdateAsync( final ParsedStatement update, AnalyzeMode analyzeMode, final UpdateOption... options) { - Callable callable = + Callable> callable = () -> { try { writeTransaction = createWriteTransaction(); - ResultSetStats res = + Tuple res = writeTransaction.run( transaction -> { if (analyzeMode == AnalyzeMode.NONE) { - return ResultSetStats.newBuilder() - .setRowCountExact( - transaction.executeUpdate(update.getStatement(), options)) - .build(); + return Tuple.of( + transaction.executeUpdate(update.getStatement(), options), null); } - return transaction.analyzeUpdate( - update.getStatement(), analyzeMode.getQueryAnalyzeMode(), options); + ResultSet resultSet = + transaction.analyzeUpdateStatement( + update.getStatement(), analyzeMode.getQueryAnalyzeMode(), options); + return Tuple.of(null, resultSet); }); state = UnitOfWorkState.COMMITTED; return res; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/UnitOfWork.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/UnitOfWork.java index bb90866c89..42470acf6f 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/UnitOfWork.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/UnitOfWork.java @@ -178,15 +178,15 @@ ApiFuture executeQueryAsync( ApiFuture executeUpdateAsync(ParsedStatement update, UpdateOption... options); /** - * Execute a DML statement on Spanner. + * Execute and/or analyze a DML statement on Spanner. * - * @param update The DML statement to execute. + * @param update The DML statement to analyze/execute. * @param analyzeMode Specifies the query/analyze mode to use for the DML statement. * @param options Update options to apply for the statement. - * @return an {@link ApiFuture} containing the {@link ResultSetStats} that were returned by this - * statement. + * @return an {@link ApiFuture} containing the {@link ResultSet} that were returned by this + * statement. The {@link ResultSet} will not contain any rows. */ - ApiFuture analyzeUpdateAsync( + ApiFuture analyzeUpdateAsync( ParsedStatement update, AnalyzeMode analyzeMode, UpdateOption... options); /** diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java index 69888fa96b..666152eeca 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java @@ -24,6 +24,7 @@ import static com.google.cloud.spanner.SpannerApiFutures.get; 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.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; @@ -61,6 +62,12 @@ import com.google.spanner.v1.ExecuteSqlRequest.QueryOptions; import com.google.spanner.v1.ReadRequest; import com.google.spanner.v1.RequestOptions.Priority; +import com.google.spanner.v1.ResultSetMetadata; +import com.google.spanner.v1.ResultSetStats; +import com.google.spanner.v1.StructType; +import com.google.spanner.v1.StructType.Field; +import com.google.spanner.v1.Type; +import com.google.spanner.v1.TypeCode; import io.grpc.Context; import io.grpc.Server; import io.grpc.Status; @@ -2327,4 +2334,51 @@ public void testGetDatabaseRole() { spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); assertEquals(TEST_DATABASE_ROLE, client.getDatabaseRole()); } + + @Test + public void testAnalyzeUpdateStatement() { + String sql = "update foo set bar=1 where baz=@param"; + mockSpanner.putStatementResult( + StatementResult.query( + Statement.of(sql), + com.google.spanner.v1.ResultSet.newBuilder() + .setMetadata( + ResultSetMetadata.newBuilder() + .setUndeclaredParameters( + StructType.newBuilder() + .addFields( + Field.newBuilder() + .setName("param") + .setType(Type.newBuilder().setCode(TypeCode.STRING).build()) + .build()) + .build()) + .build()) + .setStats(ResultSetStats.newBuilder().setRowCountExact(0L).build()) + .build())); + DatabaseClient client = + spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + client + .readWriteTransaction() + .run( + transaction -> { + try (ResultSet resultSet = + transaction.analyzeUpdateStatement(Statement.of(sql), QueryAnalyzeMode.PLAN)) { + assertFalse(resultSet.next()); + assertNotNull(resultSet.getStats()); + assertEquals(0L, resultSet.getStats().getRowCountExact()); + assertNotNull(resultSet.getMetadata()); + assertEquals(1, resultSet.getMetadata().getUndeclaredParameters().getFieldsCount()); + assertEquals( + "param", + resultSet.getMetadata().getUndeclaredParameters().getFields(0).getName()); + assertEquals( + Type.newBuilder().setCode(TypeCode.STRING).build(), + resultSet.getMetadata().getUndeclaredParameters().getFields(0).getType()); + } + return null; + }); + assertEquals(1, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class)); + ExecuteSqlRequest request = mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).get(0); + assertEquals(QueryMode.PLAN, request.getQueryMode()); + } } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionContextTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionContextTest.java index 045c58d837..24ec4b59d2 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionContextTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionContextTest.java @@ -111,6 +111,12 @@ public ApiFuture executeUpdateAsync(Statement statement, UpdateOption... o return null; } + @Override + public ResultSet analyzeUpdateStatement( + Statement statement, QueryAnalyzeMode analyzeMode, UpdateOption... options) { + return null; + } + @Override public long executeUpdate(Statement statement, UpdateOption... options) { return 0; diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AnalyzeStatementsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AnalyzeStatementsTest.java index 2039eb29b9..3adb50b170 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AnalyzeStatementsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AnalyzeStatementsTest.java @@ -50,8 +50,9 @@ @RunWith(JUnit4.class) public class AnalyzeStatementsTest extends AbstractMockServerTest { private static final Statement PLAN_QUERY = - Statement.of("SELECT * FROM SomeTable ORDER BY Value"); - private static final Statement PLAN_UPDATE = Statement.of("UPDATE SomeTable SET Value=Value+1"); + Statement.of("SELECT * FROM SomeTable WHERE Key LIKE @param ORDER BY Value"); + private static final Statement PLAN_UPDATE = + Statement.of("UPDATE SomeTable SET Value=Value+1 WHERE Key LIKE @param"); @BeforeClass public static void setupAnalyzeResults() { @@ -74,6 +75,14 @@ public static void setupAnalyzeResults() { .setName("Value") .build()) .build()) + .setUndeclaredParameters( + StructType.newBuilder() + .addFields( + Field.newBuilder() + .setType(Type.newBuilder().setCode(TypeCode.STRING).build()) + .setName("param") + .build()) + .build()) .build()) .setStats( ResultSetStats.newBuilder() @@ -88,7 +97,17 @@ public static void setupAnalyzeResults() { MockSpannerServiceImpl.StatementResult.query( PLAN_UPDATE, com.google.spanner.v1.ResultSet.newBuilder() - .setMetadata(ResultSetMetadata.newBuilder().build()) + .setMetadata( + ResultSetMetadata.newBuilder() + .setUndeclaredParameters( + StructType.newBuilder() + .addFields( + Field.newBuilder() + .setType(Type.newBuilder().setCode(TypeCode.STRING).build()) + .setName("param") + .build()) + .build()) + .build()) .setStats( ResultSetStats.newBuilder() .setQueryPlan( @@ -121,6 +140,14 @@ public void testAnalyzeQuery() { assertNotNull(resultSet.getStats()); assertNotNull(resultSet.getStats().getQueryPlan()); + + assertNotNull(resultSet.getMetadata()); + assertEquals(1, resultSet.getMetadata().getUndeclaredParameters().getFieldsCount()); + assertEquals( + Type.newBuilder().setCode(TypeCode.STRING).build(), + resultSet.getMetadata().getUndeclaredParameters().getFields(0).getType()); + assertEquals( + "param", resultSet.getMetadata().getUndeclaredParameters().getFields(0).getName()); } } @@ -186,6 +213,52 @@ public void testAnalyzeUpdate() { } } + @Test + public void testAnalyzeUpdateStatement() { + for (boolean autocommit : new boolean[] {true, false}) { + mockSpanner.clearRequests(); + + try (Connection connection = createConnection()) { + connection.setAutocommit(autocommit); + + try (ResultSet resultSet = + connection.analyzeUpdateStatement(PLAN_UPDATE, QueryAnalyzeMode.PLAN)) { + assertFalse(resultSet.next()); + + ResultSetStats stats = resultSet.getStats(); + assertNotNull(stats); + assertNotNull(stats.getQueryPlan()); + + assertNotNull(resultSet.getMetadata()); + assertEquals(1, resultSet.getMetadata().getUndeclaredParameters().getFieldsCount()); + assertEquals( + Type.newBuilder().setCode(TypeCode.STRING).build(), + resultSet.getMetadata().getUndeclaredParameters().getFields(0).getType()); + assertEquals( + "param", resultSet.getMetadata().getUndeclaredParameters().getFields(0).getName()); + } + } + + List requests = mockSpanner.getRequestsOfType(ExecuteSqlRequest.class); + assertEquals(1, requests.size()); + ExecuteSqlRequest request = requests.get(0); + assertEquals(PLAN_UPDATE.getSql(), request.getSql()); + assertEquals(QueryMode.PLAN, request.getQueryMode()); + + // As it is a DML statement, we should always start a read/write transaction, even though it + // is not executed. This is required by Cloud Spanner. + assertTrue(request.getTransaction().hasBegin()); + assertTrue(request.getTransaction().getBegin().hasReadWrite()); + + if (autocommit) { + // The read/write transaction should automatically be committed in case of autocommit. + assertEquals(1, mockSpanner.countRequestsOfType(CommitRequest.class)); + } else { + assertEquals(0, mockSpanner.countRequestsOfType(CommitRequest.class)); + } + } + } + @Test public void testAnalyzeUpdateReadOnly() { for (boolean autocommit : new boolean[] {true, false}) { @@ -207,6 +280,48 @@ public void testAnalyzeUpdateReadOnly() { } } + @Test + public void testAnalyzeUpdateStatementWithQuery() { + for (boolean autocommit : new boolean[] {true, false}) { + mockSpanner.clearRequests(); + + try (Connection connection = createConnection()) { + connection.setReadOnly(true); + connection.setAutocommit(autocommit); + + SpannerException exception = + assertThrows( + SpannerException.class, + () -> connection.analyzeUpdateStatement(PLAN_QUERY, QueryAnalyzeMode.PLAN)); + assertEquals(ErrorCode.INVALID_ARGUMENT, exception.getErrorCode()); + } + + assertEquals(0, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class)); + assertEquals(0, mockSpanner.countRequestsOfType(CommitRequest.class)); + } + } + + @Test + public void testAnalyzeUpdateStatementReadOnly() { + for (boolean autocommit : new boolean[] {true, false}) { + mockSpanner.clearRequests(); + + try (Connection connection = createConnection()) { + connection.setReadOnly(true); + connection.setAutocommit(autocommit); + + SpannerException exception = + assertThrows( + SpannerException.class, + () -> connection.analyzeUpdateStatement(PLAN_UPDATE, QueryAnalyzeMode.PLAN)); + assertEquals(ErrorCode.FAILED_PRECONDITION, exception.getErrorCode()); + } + + assertEquals(0, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class)); + assertEquals(0, mockSpanner.countRequestsOfType(CommitRequest.class)); + } + } + @Test public void testAnalyzeUpdateDdlBatch() { try (Connection connection = createConnection()) { @@ -223,6 +338,22 @@ public void testAnalyzeUpdateDdlBatch() { assertEquals(0, mockSpanner.countRequestsOfType(CommitRequest.class)); } + @Test + public void testAnalyzeUpdateStatementDdlBatch() { + try (Connection connection = createConnection()) { + connection.startBatchDdl(); + + SpannerException exception = + assertThrows( + SpannerException.class, + () -> connection.analyzeUpdateStatement(PLAN_UPDATE, QueryAnalyzeMode.PLAN)); + assertEquals(ErrorCode.FAILED_PRECONDITION, exception.getErrorCode()); + } + + assertEquals(0, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class)); + assertEquals(0, mockSpanner.countRequestsOfType(CommitRequest.class)); + } + @Test public void testAnalyzeUpdateDmlBatch() { try (Connection connection = createConnection()) { @@ -238,4 +369,20 @@ public void testAnalyzeUpdateDmlBatch() { assertEquals(0, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class)); assertEquals(0, mockSpanner.countRequestsOfType(CommitRequest.class)); } + + @Test + public void testAnalyzeUpdateStatementDmlBatch() { + try (Connection connection = createConnection()) { + connection.startBatchDml(); + + SpannerException exception = + assertThrows( + SpannerException.class, + () -> connection.analyzeUpdateStatement(PLAN_UPDATE, QueryAnalyzeMode.PLAN)); + assertEquals(ErrorCode.FAILED_PRECONDITION, exception.getErrorCode()); + } + + assertEquals(0, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class)); + assertEquals(0, mockSpanner.countRequestsOfType(CommitRequest.class)); + } } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/DirectExecuteResultSetTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/DirectExecuteResultSetTest.java index 094503cfbc..346055060a 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/DirectExecuteResultSetTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/DirectExecuteResultSetTest.java @@ -55,6 +55,7 @@ public void testMethodCallBeforeNext() List excludedMethods = Arrays.asList( "getStats", + "getMetadata", "next", "close", "ofResultSet", @@ -74,6 +75,7 @@ public void testMethodCallAfterClose() List excludedMethods = Arrays.asList( "getStats", + "getMetadata", "next", "close", "getType", @@ -95,6 +97,7 @@ public void testMethodCallAfterNextHasReturnedFalse() List excludedMethods = Arrays.asList( "getStats", + "getMetadata", "next", "close", "getType", diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ReplaceableForwardingResultSetTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ReplaceableForwardingResultSetTest.java index 4d09f0c840..3f69c2171e 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ReplaceableForwardingResultSetTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ReplaceableForwardingResultSetTest.java @@ -98,7 +98,8 @@ public void testReplace() { @Test public void testMethodCallBeforeNext() throws IllegalAccessException, IllegalArgumentException, InvocationTargetException { - List excludedMethods = Arrays.asList("getStats", "next", "close", "equals", "hashCode"); + List excludedMethods = + Arrays.asList("getStats", "getMetadata", "next", "close", "equals", "hashCode"); ReplaceableForwardingResultSet subject = createSubject(); // Test that all methods throw an IllegalStateException except the excluded methods when called // before a call to ResultSet#next(). @@ -111,6 +112,7 @@ public void testMethodCallAfterClose() List excludedMethods = Arrays.asList( "getStats", + "getMetadata", "next", "close", "getType", @@ -134,6 +136,7 @@ public void testMethodCallAfterNextHasReturnedFalse() List excludedMethods = Arrays.asList( "getStats", + "getMetadata", "next", "close", "getType",