refactor!: expand SqlParams members into PartitionQueryParams (#1263)

PartitionQueryParams corresponds to PartitionQueryRequest
(https://github.com/googleapis/googleapis/blob/2c17ac33b226194041155bb5340c3f34733f1b3a/google/spanner/v1/spanner.proto#L672),
and SqlParams corresponds to ExecuteSqlRequest
(https://github.com/googleapis/googleapis/blob/2c17ac33b226194041155bb5340c3f34733f1b3a/google/spanner/v1/spanner.proto#L439).
The former does not "include" the latter, so there may be options or
fields in SqlParams that don't belong in PartitionQueryParams.
Therefore, we should break this relationship now, while we can.

Author: devjgm

ci/test-api/spanner_client.expected.abi.dump.gz

@@ -132,7 +132,7 @@ StatusOr<std::vector<QueryPartition>> Client::PartitionQuery(
     Transaction transaction, SqlStatement statement,
     PartitionOptions const& partition_options) {
   return conn_->PartitionQuery(
-      {{std::move(transaction), std::move(statement), {}}, partition_options});
+      {std::move(transaction), std::move(statement), partition_options});
 }
 
 StatusOr<DmlResult> Client::ExecuteDml(Transaction transaction,

google/cloud/spanner/client.cc

@@ -97,7 +97,8 @@ class Connection {
 
   /// Wrap the arguments to `PartitionQuery()`.
   struct PartitionQueryParams {
-    SqlParams sql_params;
+    Transaction transaction;
+    SqlStatement statement;
     PartitionOptions partition_options;
   };
 

google/cloud/spanner/connection.h

@@ -187,11 +187,10 @@ StatusOr<PartitionedDmlResult> ConnectionImpl::ExecutePartitionedDml(
 StatusOr<std::vector<QueryPartition>> ConnectionImpl::PartitionQuery(
     PartitionQueryParams params) {
   return internal::Visit(
-      std::move(params.sql_params.transaction),
+      std::move(params.transaction),
       [this, &params](SessionHolder& session,
                       spanner_proto::TransactionSelector& s, std::int64_t) {
-        return PartitionQueryImpl(session, s, params.sql_params,
-                                  params.partition_options);
+        return PartitionQueryImpl(session, s, std::move(params));
       });
 }
 
@@ -582,7 +581,8 @@ StatusOr<ExecutionPlan> ConnectionImpl::AnalyzeSqlImpl(
 
 StatusOr<std::vector<QueryPartition>> ConnectionImpl::PartitionQueryImpl(
     SessionHolder& session, spanner_proto::TransactionSelector& s,
-    SqlParams const& params, PartitionOptions const& partition_options) {
+    PartitionQueryParams
+        params) {  // NOLINT(performance-unnecessary-value-param)
   // Since the session may be sent to other machines, it should not be returned
   // to the pool when the Transaction is destroyed.
   auto prepare_status = PrepareSession(session, /*dissociate_from_pool=*/true);
@@ -598,7 +598,8 @@ StatusOr<std::vector<QueryPartition>> ConnectionImpl::PartitionQueryImpl(
   *request.mutable_params() = std::move(*sql_statement.mutable_params());
   *request.mutable_param_types() =
       std::move(*sql_statement.mutable_param_types());
-  *request.mutable_partition_options() = internal::ToProto(partition_options);
+  *request.mutable_partition_options() =
+      internal::ToProto(std::move(params.partition_options));
 
   auto stub = session_pool_->GetStub(*session);
   auto response = internal::RetryLoop(
@@ -623,7 +624,7 @@ StatusOr<std::vector<QueryPartition>> ConnectionImpl::PartitionQueryImpl(
   for (auto& partition : response->partitions()) {
     query_partitions.push_back(internal::MakeQueryPartition(
         response->transaction().id(), session->session_name(),
-        partition.partition_token(), params.statement));
+        partition.partition_token(), std::move(params.statement)));
   }
 
   return query_partitions;

google/cloud/spanner/internal/connection_impl.cc

@@ -131,7 +131,7 @@ class ConnectionImpl : public Connection {
 
   StatusOr<std::vector<QueryPartition>> PartitionQueryImpl(
       SessionHolder& session, google::spanner::v1::TransactionSelector& s,
-      SqlParams const& params, PartitionOptions const& partition_options);
+      PartitionQueryParams params);
 
   StatusOr<BatchDmlResult> ExecuteBatchDmlImpl(
       SessionHolder& session, google::spanner::v1::TransactionSelector& s,

google/cloud/spanner/internal/connection_impl.h

@@ -1850,9 +1850,7 @@ TEST(ConnectionImplTest, PartitionQuerySuccess) {
 
   SqlStatement sql_statement("select * from table");
   StatusOr<std::vector<QueryPartition>> result = conn->PartitionQuery(
-      {{MakeReadOnlyTransaction(Transaction::ReadOnlyOptions()),
-        sql_statement,
-        {}},
+      {MakeReadOnlyTransaction(Transaction::ReadOnlyOptions()), sql_statement,
        PartitionOptions()});
   ASSERT_STATUS_OK(result);
 
@@ -1883,10 +1881,8 @@ TEST(ConnectionImplTest, PartitionQueryPermanentFailure) {
       .WillOnce(Return(failed_status));
 
   StatusOr<std::vector<QueryPartition>> result = conn->PartitionQuery(
-      {{MakeReadOnlyTransaction(Transaction::ReadOnlyOptions()),
-        SqlStatement("select * from table"),
-        {}},
-       PartitionOptions()});
+      {MakeReadOnlyTransaction(Transaction::ReadOnlyOptions()),
+       SqlStatement("select * from table"), PartitionOptions()});
   EXPECT_EQ(StatusCode::kPermissionDenied, result.status().code());
   EXPECT_THAT(result.status().message(), HasSubstr(failed_status.message()));
 }
@@ -1910,10 +1906,8 @@ TEST(ConnectionImplTest, PartitionQueryTooManyTransientFailures) {
       .WillRepeatedly(Return(failed_status));
 
   StatusOr<std::vector<QueryPartition>> result = conn->PartitionQuery(
-      {{MakeReadOnlyTransaction(Transaction::ReadOnlyOptions()),
-        SqlStatement("select * from table"),
-        {}},
-       PartitionOptions()});
+      {MakeReadOnlyTransaction(Transaction::ReadOnlyOptions()),
+       SqlStatement("select * from table"), PartitionOptions()});
   EXPECT_EQ(StatusCode::kUnavailable, result.status().code());
   EXPECT_THAT(result.status().message(), HasSubstr(failed_status.message()));
 }
@@ -2316,8 +2310,7 @@ TEST(ConnectionImplTest, PartitionQuerySessionNotFound) {
   auto conn = MakeLimitedRetryConnection(db, mock);
   auto txn = MakeReadWriteTransaction();
   SetTransactionId(txn, "test-txn-id");
-  auto sql_params = Connection::SqlParams{txn, SqlStatement(), {}};
-  auto response = conn->PartitionQuery({sql_params, {}});
+  auto response = conn->PartitionQuery({txn, {}, {}});
   EXPECT_FALSE(response.ok());
   auto status = response.status();
   EXPECT_TRUE(IsSessionNotFound(status)) << status;