From 765b863c2d461bb58a818f7694c573b556fa708a Mon Sep 17 00:00:00 2001 From: mpeddada1 Date: Tue, 1 Jul 2025 20:02:43 +0000 Subject: [PATCH 1/4] feat(spanner): add support for lock_hint enum --- google/cloud/spanner/CMakeLists.txt | 1 + google/cloud/spanner/client.cc | 18 ++-- google/cloud/spanner/client_test.cc | 9 +- google/cloud/spanner/connection.h | 2 + .../spanner/google_cloud_cpp_spanner.bzl | 1 + .../cloud/spanner/internal/connection_impl.cc | 16 ++++ .../spanner/internal/connection_impl_test.cc | 83 +++++++++++++++++++ google/cloud/spanner/lock_hint.h | 37 +++++++++ google/cloud/spanner/options.h | 10 +++ google/cloud/spanner/read_partition.h | 9 +- google/cloud/spanner/read_partition_test.cc | 3 +- 11 files changed, 176 insertions(+), 13 deletions(-) create mode 100644 google/cloud/spanner/lock_hint.h diff --git a/google/cloud/spanner/CMakeLists.txt b/google/cloud/spanner/CMakeLists.txt index 39c9b305fefed..e0436fd4ef3ab 100644 --- a/google/cloud/spanner/CMakeLists.txt +++ b/google/cloud/spanner/CMakeLists.txt @@ -176,6 +176,7 @@ add_library( json.h keys.cc keys.h + lock_hint.h mutations.cc mutations.h numeric.cc diff --git a/google/cloud/spanner/client.cc b/google/cloud/spanner/client.cc index 542f501a3e732..181d8f3410e99 100644 --- a/google/cloud/spanner/client.cc +++ b/google/cloud/spanner/client.cc @@ -54,11 +54,13 @@ RowStream Client::Read(std::string table, KeySet keys, opts = internal::MergeOptions(std::move(opts), opts_); auto directed_read_option = ExtractOpt(opts); internal::OptionsSpan span(std::move(opts)); + auto lock_hint = ExtractOpt(opts); return conn_->Read({spanner_internal::MakeSingleUseTransaction( Transaction::ReadOnlyOptions()), std::move(table), std::move(keys), std::move(columns), ToReadOptions(internal::CurrentOptions()), absl::nullopt, - false, std::move(directed_read_option)}); + false, std::move(directed_read_option), + std::move(lock_hint)}); } RowStream Client::Read(Transaction::SingleUseOptions transaction_options, @@ -67,30 +69,35 @@ RowStream Client::Read(Transaction::SingleUseOptions transaction_options, opts = internal::MergeOptions(std::move(opts), opts_); auto directed_read_option = ExtractOpt(opts); internal::OptionsSpan span(std::move(opts)); + auto lock_hint = ExtractOpt(opts); return conn_->Read({spanner_internal::MakeSingleUseTransaction( std::move(transaction_options)), std::move(table), std::move(keys), std::move(columns), ToReadOptions(internal::CurrentOptions()), absl::nullopt, - false, std::move(directed_read_option)}); + false, std::move(directed_read_option), + std::move(lock_hint)}); } RowStream Client::Read(Transaction transaction, std::string table, KeySet keys, std::vector columns, Options opts) { opts = internal::MergeOptions(std::move(opts), opts_); auto directed_read_option = ExtractOpt(opts); + auto lock_hint = ExtractOpt(opts); internal::OptionsSpan span(std::move(opts)); return conn_->Read({std::move(transaction), std::move(table), std::move(keys), std::move(columns), ToReadOptions(internal::CurrentOptions()), absl::nullopt, - false, std::move(directed_read_option)}); + false, std::move(directed_read_option), + std::move(lock_hint)}); } RowStream Client::Read(ReadPartition const& read_partition, Options opts) { opts = internal::MergeOptions(std::move(opts), opts_); auto directed_read_option = ExtractOpt(opts); + auto lock_hint = ExtractOpt(opts); internal::OptionsSpan span(std::move(opts)); return conn_->Read(spanner_internal::MakeReadParams( - read_partition, std::move(directed_read_option))); + read_partition, std::move(directed_read_option), std::move(lock_hint))); } StatusOr> Client::PartitionRead( @@ -100,7 +107,8 @@ StatusOr> Client::PartitionRead( return conn_->PartitionRead( {{std::move(transaction), std::move(table), std::move(keys), std::move(columns), ToReadOptions(internal::CurrentOptions()), - absl::nullopt, false, DirectedReadOption::Type{}}, + absl::nullopt, false, DirectedReadOption::Type{}, + LockHint::kLockHintUnspecified}, ToPartitionOptions(internal::CurrentOptions())}); } diff --git a/google/cloud/spanner/client_test.cc b/google/cloud/spanner/client_test.cc index ecd2253cf6fca..82e381c93b9ba 100644 --- a/google/cloud/spanner/client_test.cc +++ b/google/cloud/spanner/client_test.cc @@ -404,7 +404,8 @@ TEST(ClientTest, CommitMutatorSuccess) { auto conn = std::make_shared(); Transaction txn = MakeReadWriteTransaction(); // placeholder - Connection::ReadParams actual_read_params{txn, {}, {}, {}, {}, {}, {}, {}}; + Connection::ReadParams actual_read_params{txn, {}, {}, {}, {}, + {}, {}, {}, {}}; Connection::CommitParams actual_commit_params{txn, {}, {}}; auto source = std::make_unique(); @@ -453,7 +454,8 @@ TEST(ClientTest, CommitMutatorSuccess) { TEST(ClientTest, CommitMutatorRollback) { auto conn = std::make_shared(); Transaction txn = MakeReadWriteTransaction(); // placeholder - Connection::ReadParams actual_read_params{txn, {}, {}, {}, {}, {}, {}, {}}; + Connection::ReadParams actual_read_params{txn, {}, {}, {}, {}, + {}, {}, {}, {}}; auto source = std::make_unique(); auto constexpr kText = R"pb( @@ -495,7 +497,8 @@ TEST(ClientTest, CommitMutatorRollback) { TEST(ClientTest, CommitMutatorRollbackError) { auto conn = std::make_shared(); Transaction txn = MakeReadWriteTransaction(); // placeholder - Connection::ReadParams actual_read_params{txn, {}, {}, {}, {}, {}, {}, {}}; + Connection::ReadParams actual_read_params{txn, {}, {}, {}, {}, + {}, {}, {}, {}}; auto source = std::make_unique(); auto constexpr kText = R"pb( diff --git a/google/cloud/spanner/connection.h b/google/cloud/spanner/connection.h index 766974d94ed69..287d547c37194 100644 --- a/google/cloud/spanner/connection.h +++ b/google/cloud/spanner/connection.h @@ -19,6 +19,7 @@ #include "google/cloud/spanner/commit_options.h" #include "google/cloud/spanner/commit_result.h" #include "google/cloud/spanner/keys.h" +#include "google/cloud/spanner/lock_hint.h" #include "google/cloud/spanner/mutations.h" #include "google/cloud/spanner/options.h" #include "google/cloud/spanner/partition_options.h" @@ -81,6 +82,7 @@ class Connection { absl::optional partition_token; bool partition_data_boost = false; // when partition_token DirectedReadOption::Type directed_read_option; + LockHint lock_hint; }; /// Wrap the arguments to `PartitionRead()`. diff --git a/google/cloud/spanner/google_cloud_cpp_spanner.bzl b/google/cloud/spanner/google_cloud_cpp_spanner.bzl index 82d829d0680fb..9fe3e7265836d 100644 --- a/google/cloud/spanner/google_cloud_cpp_spanner.bzl +++ b/google/cloud/spanner/google_cloud_cpp_spanner.bzl @@ -96,6 +96,7 @@ google_cloud_cpp_spanner_hdrs = [ "interval.h", "json.h", "keys.h", + "lock_hint.h", "mutations.h", "numeric.h", "oid.h", diff --git a/google/cloud/spanner/internal/connection_impl.cc b/google/cloud/spanner/internal/connection_impl.cc index 8c17114eca020..bd4612038db32 100644 --- a/google/cloud/spanner/internal/connection_impl.cc +++ b/google/cloud/spanner/internal/connection_impl.cc @@ -190,6 +190,21 @@ google::spanner::v1::RequestOptions_Priority ProtoRequestPriority( return google::spanner::v1::RequestOptions::PRIORITY_UNSPECIFIED; } +google::spanner::v1::ReadRequest_LockHint ProtoLockHint( + absl::optional const& order_by) { + if (order_by) { + switch (*order_by) { + case spanner::LockHint::kLockHintUnspecified: + return google::spanner::v1::ReadRequest_LockHint_LOCK_HINT_UNSPECIFIED; + case spanner::LockHint::kLockHintShared: + return google::spanner::v1::ReadRequest_LockHint_LOCK_HINT_SHARED; + case spanner::LockHint::kLockHintExclusive: + return google::spanner::v1::ReadRequest_LockHint_LOCK_HINT_EXCLUSIVE; + } + } + return google::spanner::v1::ReadRequest_LockHint_LOCK_HINT_UNSPECIFIED; +} + // Converts a `google::protobuf::Timestamp` to a `spanner::Timestamp`, but // substitutes the maximal value for any conversion error. This is needed // when, for example, a response commit_timestamp is out of range but the @@ -548,6 +563,7 @@ spanner::RowStream ConnectionImpl::ReadImpl( *request->mutable_transaction() = *s; request->set_table(std::move(params.table)); request->set_index(std::move(params.read_options.index_name)); + request->set_lock_hint(ProtoLockHint(params.lock_hint)); for (auto&& column : params.columns) { request->add_columns(std::move(column)); } diff --git a/google/cloud/spanner/internal/connection_impl_test.cc b/google/cloud/spanner/internal/connection_impl_test.cc index 8dfa066ba46e3..6d8809292d825 100644 --- a/google/cloud/spanner/internal/connection_impl_test.cc +++ b/google/cloud/spanner/internal/connection_impl_test.cc @@ -211,6 +211,10 @@ MATCHER_P(HasReplicaType, type, "has replica type") { return arg.type() == type; } +MATCHER_P(HasLockHint, lock_hint, "has lock_hint") { + return arg.lock_hint() == lock_hint; +} + // Ideally this would be a matcher, but matcher args are `const` and `RowStream` // only has non-const methods. bool ContainsNoRows(spanner::RowStream& rows) { @@ -4091,6 +4095,85 @@ TEST(ConnectionImplTest, RollbackSessionNotFound) { EXPECT_THAT(txn, HasBadSession()); } +TEST(ConnectionImplTest, ReadRequestLockHintParameterUnspecified) { + auto mock = std::make_shared(); + auto db = spanner::Database("project", "instance", "database"); + EXPECT_CALL(*mock, BatchCreateSessions(_, _, HasDatabase(db))) + .WillOnce(Return(MakeSessionsResponse({"test-session-name"}))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); + Sequence s; + EXPECT_CALL( + *mock, + StreamingRead( + _, _, + AllOf(HasSession("test-session-name"), + HasLockHint( + google::spanner::v1::ReadRequest::LOCK_HINT_UNSPECIFIED)))) + .InSequence(s) + .WillOnce(Return(ByMove(MakeReader( + {R"pb(metadata: { transaction: { id: "txn1" } })pb"})))); + + auto conn = MakeConnectionImpl(db, mock); + internal::OptionsSpan span(MakeLimitedTimeOptions()); + + // Scenario 1: No explicit OrderBy (should map to UNSPECIFIED) + spanner::ReadOptions read_options; + spanner::Transaction txn1 = + MakeReadOnlyTransaction(spanner::Transaction::ReadOnlyOptions()); + auto rows1 = conn->Read( + {txn1, "table", spanner::KeySet::All(), {"col"}, read_options}); + for (auto const& row : rows1) { + (void)row; + } + EXPECT_THAT(txn1, + HasSessionAndTransaction("test-session-name", "txn1", false, "")); +} + +TEST(ConnectionImplTest, ReadRequestLockHintShared) { + auto mock = std::make_shared(); + auto db = spanner::Database("project", "instance", "database"); + EXPECT_CALL(*mock, BatchCreateSessions(_, _, HasDatabase(db))) + .WillOnce(Return(MakeSessionsResponse({"test-session-name"}))); + EXPECT_CALL(*mock, + AsyncDeleteSession(_, _, _, HasSessionName("test-session-name"))) + .WillOnce(Return(make_ready_future(Status{}))); + Sequence s; + EXPECT_CALL( + *mock, + StreamingRead( + _, _, + AllOf( + HasSession("test-session-name"), + HasLockHint(google::spanner::v1::ReadRequest::LOCK_HINT_SHARED)))) + .InSequence(s) + .WillOnce(Return(ByMove(MakeReader( + {R"pb(metadata: { transaction: { id: "txn1" } })pb"})))); + + auto conn = MakeConnectionImpl(db, mock); + internal::OptionsSpan span(MakeLimitedTimeOptions()); + spanner::ReadOptions read_options; + spanner::Transaction txn1 = + MakeReadOnlyTransaction(spanner::Transaction::ReadOnlyOptions()); + auto read_params = + spanner::Connection::ReadParams{txn1, + "table", + spanner::KeySet::All(), + {"col"}, + read_options, + absl::nullopt, + false, + spanner::DirectedReadOption::Type{}, + spanner::LockHint::kLockHintShared}; + auto rows1 = conn->Read(read_params); + for (auto const& row : rows1) { + (void)row; + } + EXPECT_THAT(txn1, + HasSessionAndTransaction("test-session-name", "txn1", false, "")); +} + TEST(ConnectionImplTest, OperationsFailOnInvalidatedTransaction) { auto mock = std::make_shared(); auto db = spanner::Database("placeholder_project", "placeholder_instance", diff --git a/google/cloud/spanner/lock_hint.h b/google/cloud/spanner/lock_hint.h new file mode 100644 index 0000000000000..f10d3b09642b3 --- /dev/null +++ b/google/cloud/spanner/lock_hint.h @@ -0,0 +1,37 @@ +// Copyright 2025 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 +// +// https://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. + +#ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_LOCK_HINT_H +#define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_LOCK_HINT_H + +#include "google/cloud/spanner/version.h" + +namespace google { +namespace cloud { +namespace spanner { +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN + +// A lock hint mechanism for reads done within a transaction. +enum class LockHint { + kLockHintUnspecified, + kLockHintShared, + kLockHintExclusive, +}; + +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END +} // namespace spanner +} // namespace cloud +} // namespace google + +#endif // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_LOCK_HINT_H diff --git a/google/cloud/spanner/options.h b/google/cloud/spanner/options.h index a081b244cbb45..a6d9451bd21b4 100644 --- a/google/cloud/spanner/options.h +++ b/google/cloud/spanner/options.h @@ -42,6 +42,7 @@ #include "google/cloud/spanner/directed_read_replicas.h" #include "google/cloud/spanner/internal/session.h" #include "google/cloud/spanner/polling_policy.h" +#include "google/cloud/spanner/lock_hint.h" #include "google/cloud/spanner/request_priority.h" #include "google/cloud/spanner/retry_policy.h" #include "google/cloud/spanner/version.h" @@ -195,6 +196,15 @@ struct SessionPoolActionOnExhaustionOption { using Type = spanner::ActionOnExhaustion; }; +/** + * Option for `google::cloud::Options` to set the lock hint mechanism for reads done within a transaction + * + * @ingroup google-cloud-spanner-options + */ +struct LockHintOption { + using Type = spanner::LockHint; +}; + /** * Option for `google::cloud::Options` to set the interval at which we refresh * sessions so they don't get collected by the backend GC. diff --git a/google/cloud/spanner/read_partition.h b/google/cloud/spanner/read_partition.h index 0801d25f620fc..e2b7e87e64d49 100644 --- a/google/cloud/spanner/read_partition.h +++ b/google/cloud/spanner/read_partition.h @@ -174,7 +174,7 @@ struct ReadPartitionInternals { static spanner::Connection::ReadParams MakeReadParams( spanner::ReadPartition const& read_partition, - spanner::DirectedReadOption::Type directed_read_option) { + spanner::DirectedReadOption::Type directed_read_option, spanner::LockHint lock_hint) { return spanner::Connection::ReadParams{ MakeTransactionFromIds( read_partition.SessionId(), read_partition.TransactionId(), @@ -185,7 +185,8 @@ struct ReadPartitionInternals { read_partition.ReadOptions(), read_partition.PartitionToken(), read_partition.DataBoost(), - std::move(directed_read_option)}; + std::move(directed_read_option), + lock_hint}; } }; @@ -204,9 +205,9 @@ inline spanner::ReadPartition MakeReadPartition( inline spanner::Connection::ReadParams MakeReadParams( spanner::ReadPartition const& read_partition, - spanner::DirectedReadOption::Type directed_read_option) { + spanner::DirectedReadOption::Type directed_read_option, spanner::LockHint lock_hint) { return ReadPartitionInternals::MakeReadParams( - read_partition, std::move(directed_read_option)); + read_partition, std::move(directed_read_option), lock_hint); } GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END diff --git a/google/cloud/spanner/read_partition_test.cc b/google/cloud/spanner/read_partition_test.cc index ddb2d553eb533..917bfc8c82925 100644 --- a/google/cloud/spanner/read_partition_test.cc +++ b/google/cloud/spanner/read_partition_test.cc @@ -206,7 +206,7 @@ TEST(ReadPartitionTest, MakeReadParams) { Connection::ReadParams params = spanner_internal::MakeReadParams( expected_partition.Partition(), IncludeReplicas({ReplicaSelection(ReplicaType::kReadWrite)}, - /*auto_failover_disabled=*/true)); + /*auto_failover_disabled=*/true), google::cloud::spanner::LockHint::kLockHintExclusive); EXPECT_EQ(*params.partition_token, "token"); EXPECT_EQ(params.read_options, read_options); @@ -214,6 +214,7 @@ TEST(ReadPartitionTest, MakeReadParams) { EXPECT_EQ(params.columns, columns); EXPECT_EQ(params.keys, KeySet::All()); EXPECT_EQ(params.table, "Students"); + EXPECT_EQ(params.lock_hint, google::cloud::spanner::LockHint::kLockHintExclusive); EXPECT_THAT(params.transaction, HasSessionAndTransaction("session", "txn-id", true, "tag")); EXPECT_THAT( From 72ee64648eeab1f6c9230753a06edf49cf6ec2fe Mon Sep 17 00:00:00 2001 From: mpeddada1 Date: Tue, 1 Jul 2025 20:12:19 +0000 Subject: [PATCH 2/4] fix formatting --- google/cloud/spanner/options.h | 5 +++-- google/cloud/spanner/read_partition.h | 8 +++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/google/cloud/spanner/options.h b/google/cloud/spanner/options.h index a6d9451bd21b4..6356a717a3805 100644 --- a/google/cloud/spanner/options.h +++ b/google/cloud/spanner/options.h @@ -41,8 +41,8 @@ #include "google/cloud/spanner/backoff_policy.h" #include "google/cloud/spanner/directed_read_replicas.h" #include "google/cloud/spanner/internal/session.h" -#include "google/cloud/spanner/polling_policy.h" #include "google/cloud/spanner/lock_hint.h" +#include "google/cloud/spanner/polling_policy.h" #include "google/cloud/spanner/request_priority.h" #include "google/cloud/spanner/retry_policy.h" #include "google/cloud/spanner/version.h" @@ -197,7 +197,8 @@ struct SessionPoolActionOnExhaustionOption { }; /** - * Option for `google::cloud::Options` to set the lock hint mechanism for reads done within a transaction + * Option for `google::cloud::Options` to set the lock hint mechanism for reads + * done within a transaction. * * @ingroup google-cloud-spanner-options */ diff --git a/google/cloud/spanner/read_partition.h b/google/cloud/spanner/read_partition.h index e2b7e87e64d49..19bbfeb4d4d04 100644 --- a/google/cloud/spanner/read_partition.h +++ b/google/cloud/spanner/read_partition.h @@ -174,7 +174,8 @@ struct ReadPartitionInternals { static spanner::Connection::ReadParams MakeReadParams( spanner::ReadPartition const& read_partition, - spanner::DirectedReadOption::Type directed_read_option, spanner::LockHint lock_hint) { + spanner::DirectedReadOption::Type directed_read_option, + spanner::LockHint lock_hint) { return spanner::Connection::ReadParams{ MakeTransactionFromIds( read_partition.SessionId(), read_partition.TransactionId(), @@ -186,7 +187,7 @@ struct ReadPartitionInternals { read_partition.PartitionToken(), read_partition.DataBoost(), std::move(directed_read_option), - lock_hint}; + lock_hint}; } }; @@ -205,7 +206,8 @@ inline spanner::ReadPartition MakeReadPartition( inline spanner::Connection::ReadParams MakeReadParams( spanner::ReadPartition const& read_partition, - spanner::DirectedReadOption::Type directed_read_option, spanner::LockHint lock_hint) { + spanner::DirectedReadOption::Type directed_read_option, + spanner::LockHint lock_hint) { return ReadPartitionInternals::MakeReadParams( read_partition, std::move(directed_read_option), lock_hint); } From d29256d386cfeba0122a7f6eb395e50443e159a8 Mon Sep 17 00:00:00 2001 From: mpeddada1 Date: Tue, 1 Jul 2025 21:06:57 +0000 Subject: [PATCH 3/4] fix formatting --- google/cloud/spanner/read_partition_test.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/google/cloud/spanner/read_partition_test.cc b/google/cloud/spanner/read_partition_test.cc index 917bfc8c82925..5cd86b152e823 100644 --- a/google/cloud/spanner/read_partition_test.cc +++ b/google/cloud/spanner/read_partition_test.cc @@ -206,7 +206,8 @@ TEST(ReadPartitionTest, MakeReadParams) { Connection::ReadParams params = spanner_internal::MakeReadParams( expected_partition.Partition(), IncludeReplicas({ReplicaSelection(ReplicaType::kReadWrite)}, - /*auto_failover_disabled=*/true), google::cloud::spanner::LockHint::kLockHintExclusive); + /*auto_failover_disabled=*/true), + google::cloud::spanner::LockHint::kLockHintExclusive); EXPECT_EQ(*params.partition_token, "token"); EXPECT_EQ(params.read_options, read_options); @@ -214,7 +215,8 @@ TEST(ReadPartitionTest, MakeReadParams) { EXPECT_EQ(params.columns, columns); EXPECT_EQ(params.keys, KeySet::All()); EXPECT_EQ(params.table, "Students"); - EXPECT_EQ(params.lock_hint, google::cloud::spanner::LockHint::kLockHintExclusive); + EXPECT_EQ(params.lock_hint, + google::cloud::spanner::LockHint::kLockHintExclusive); EXPECT_THAT(params.transaction, HasSessionAndTransaction("session", "txn-id", true, "tag")); EXPECT_THAT( From 4522807dcb713067e0bb99c7927d4e5133aa2e3b Mon Sep 17 00:00:00 2001 From: mpeddada1 Date: Mon, 14 Jul 2025 19:01:19 +0000 Subject: [PATCH 4/4] fix client.cc and add testing for lock_hint in client_test --- google/cloud/spanner/client.cc | 4 +-- google/cloud/spanner/client_test.cc | 52 +++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/google/cloud/spanner/client.cc b/google/cloud/spanner/client.cc index 181d8f3410e99..aaef48fe15c93 100644 --- a/google/cloud/spanner/client.cc +++ b/google/cloud/spanner/client.cc @@ -53,8 +53,8 @@ RowStream Client::Read(std::string table, KeySet keys, std::vector columns, Options opts) { opts = internal::MergeOptions(std::move(opts), opts_); auto directed_read_option = ExtractOpt(opts); - internal::OptionsSpan span(std::move(opts)); auto lock_hint = ExtractOpt(opts); + internal::OptionsSpan span(std::move(opts)); return conn_->Read({spanner_internal::MakeSingleUseTransaction( Transaction::ReadOnlyOptions()), std::move(table), std::move(keys), std::move(columns), @@ -68,8 +68,8 @@ RowStream Client::Read(Transaction::SingleUseOptions transaction_options, std::vector columns, Options opts) { opts = internal::MergeOptions(std::move(opts), opts_); auto directed_read_option = ExtractOpt(opts); - internal::OptionsSpan span(std::move(opts)); auto lock_hint = ExtractOpt(opts); + internal::OptionsSpan span(std::move(opts)); return conn_->Read({spanner_internal::MakeSingleUseTransaction( std::move(transaction_options)), std::move(table), std::move(keys), std::move(columns), diff --git a/google/cloud/spanner/client_test.cc b/google/cloud/spanner/client_test.cc index 82e381c93b9ba..acd61531c60d5 100644 --- a/google/cloud/spanner/client_test.cc +++ b/google/cloud/spanner/client_test.cc @@ -138,6 +138,58 @@ TEST(ClientTest, ReadSuccess) { IsOkAndHolds(RowType("Ann", 42)))); } +TEST(ClientTest, ReadWithLockHint) { + auto conn = std::make_shared(); + Client client(conn); + + auto constexpr kText = R"pb( + row_type: { + fields: { + name: "Name", + type: { code: INT64 } + } + fields: { + name: "Id", + type: { code: INT64 } + } + } + )pb"; + google::spanner::v1::ResultSetMetadata metadata; + ASSERT_TRUE(TextFormat::ParseFromString(kText, &metadata)); + + EXPECT_CALL(*conn, Read) + .WillOnce([&metadata](Connection::ReadParams const& params) { + EXPECT_THAT( + params.directed_read_option, + VariantWith(AllOf( + Property(&IncludeReplicas::replica_selections, + ElementsAre(ReplicaSelection(ReplicaType::kReadOnly))), + Property(&IncludeReplicas::auto_failover_disabled, true)))); + EXPECT_THAT(params.lock_hint, Eq(LockHint::kLockHintShared)); + auto source = std::make_unique(); + EXPECT_CALL(*source, Metadata()).WillRepeatedly(Return(metadata)); + EXPECT_CALL(*source, NextRow()) + .WillOnce(Return(spanner_mocks::MakeRow("Steve", 12))) + .WillOnce(Return(spanner_mocks::MakeRow("Ann", 42))) + .WillOnce(Return(Row())); + return RowStream(std::move(source)); + }); + + KeySet keys = KeySet::All(); + auto rows = client.Read("table", std::move(keys), {"column1", "column2"}, + Options{} + .set(IncludeReplicas( + {ReplicaSelection(ReplicaType::kReadOnly)}, + /*auto_failover_disabled=*/true)) + .set(LockHint::kLockHintShared)); + + using RowType = std::tuple; + auto stream = StreamOf(rows); + auto actual = std::vector>{stream.begin(), stream.end()}; + EXPECT_THAT(actual, ElementsAre(IsOkAndHolds(RowType("Steve", 12)), + IsOkAndHolds(RowType("Ann", 42)))); +} + TEST(ClientTest, ReadFailure) { auto conn = std::make_shared(); Client client(conn);