Skip to content

Commit fbc58d6

Browse files
mpeddada1mpeddada1
authored
feat(spanner): add support for order_by enum (#15240)
* feat(spanner): add support for order_by enum
1 parent 9bb18b8 commit fbc58d6

File tree

12 files changed

+200
-11
lines changed

12 files changed

+200
-11
lines changed

‎google/cloud/spanner/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ add_library(
183183
numeric.h
184184
oid.h
185185
options.h
186+
order_by.h
186187
partition_options.cc
187188
partition_options.h
188189
partitioned_dml_result.h

‎google/cloud/spanner/client.cc‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,51 +53,56 @@ RowStream Client::Read(std::string table, KeySet keys,
5353
std::vector<std::string> columns, Options opts) {
5454
opts = internal::MergeOptions(std::move(opts), opts_);
5555
auto directed_read_option = ExtractOpt<DirectedReadOption>(opts);
56+
auto order_by = ExtractOpt<OrderByOption>(opts);
5657
auto lock_hint = ExtractOpt<LockHintOption>(opts);
5758
internal::OptionsSpan span(std::move(opts));
5859
return conn_->Read({spanner_internal::MakeSingleUseTransaction(
5960
Transaction::ReadOnlyOptions()),
6061
std::move(table), std::move(keys), std::move(columns),
6162
ToReadOptions(internal::CurrentOptions()), absl::nullopt,
6263
false, std::move(directed_read_option),
63-
std::move(lock_hint)});
64+
std::move(order_by), std::move(lock_hint)});
6465
}
6566

6667
RowStream Client::Read(Transaction::SingleUseOptions transaction_options,
6768
std::string table, KeySet keys,
6869
std::vector<std::string> columns, Options opts) {
6970
opts = internal::MergeOptions(std::move(opts), opts_);
7071
auto directed_read_option = ExtractOpt<DirectedReadOption>(opts);
72+
auto order_by = ExtractOpt<OrderByOption>(opts);
7173
auto lock_hint = ExtractOpt<LockHintOption>(opts);
7274
internal::OptionsSpan span(std::move(opts));
7375
return conn_->Read({spanner_internal::MakeSingleUseTransaction(
7476
std::move(transaction_options)),
7577
std::move(table), std::move(keys), std::move(columns),
7678
ToReadOptions(internal::CurrentOptions()), absl::nullopt,
7779
false, std::move(directed_read_option),
78-
std::move(lock_hint)});
80+
std::move(order_by), std::move(lock_hint)});
7981
}
8082

8183
RowStream Client::Read(Transaction transaction, std::string table, KeySet keys,
8284
std::vector<std::string> columns, Options opts) {
8385
opts = internal::MergeOptions(std::move(opts), opts_);
8486
auto directed_read_option = ExtractOpt<DirectedReadOption>(opts);
87+
auto order_by = ExtractOpt<OrderByOption>(opts);
8588
auto lock_hint = ExtractOpt<LockHintOption>(opts);
8689
internal::OptionsSpan span(std::move(opts));
8790
return conn_->Read({std::move(transaction), std::move(table), std::move(keys),
8891
std::move(columns),
8992
ToReadOptions(internal::CurrentOptions()), absl::nullopt,
9093
false, std::move(directed_read_option),
91-
std::move(lock_hint)});
94+
std::move(order_by), std::move(lock_hint)});
9295
}
9396

9497
RowStream Client::Read(ReadPartition const& read_partition, Options opts) {
9598
opts = internal::MergeOptions(std::move(opts), opts_);
9699
auto directed_read_option = ExtractOpt<DirectedReadOption>(opts);
100+
auto order_by = ExtractOpt<OrderByOption>(opts);
97101
auto lock_hint = ExtractOpt<LockHintOption>(opts);
98102
internal::OptionsSpan span(std::move(opts));
99103
return conn_->Read(spanner_internal::MakeReadParams(
100-
read_partition, std::move(directed_read_option), std::move(lock_hint)));
104+
read_partition, std::move(directed_read_option), std::move(order_by),
105+
std::move(lock_hint)));
101106
}
102107

103108
StatusOr<std::vector<ReadPartition>> Client::PartitionRead(
@@ -108,7 +113,7 @@ StatusOr<std::vector<ReadPartition>> Client::PartitionRead(
108113
{{std::move(transaction), std::move(table), std::move(keys),
109114
std::move(columns), ToReadOptions(internal::CurrentOptions()),
110115
absl::nullopt, false, DirectedReadOption::Type{},
111-
LockHint::kLockHintUnspecified},
116+
OrderBy::kOrderByUnspecified, LockHint::kLockHintUnspecified},
112117
ToPartitionOptions(internal::CurrentOptions())});
113118
}
114119

‎google/cloud/spanner/client_test.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,7 @@ TEST(ClientTest, CommitMutatorSuccess) {
457457
auto conn = std::make_shared<MockConnection>();
458458
Transaction txn = MakeReadWriteTransaction(); // placeholder
459459
Connection::ReadParams actual_read_params{txn, {}, {}, {}, {},
460-
{}, {}, {}, {}};
460+
{}, {}, {}, {}, {}};
461461
Connection::CommitParams actual_commit_params{txn, {}, {}};
462462

463463
auto source = std::make_unique<MockResultSetSource>();
@@ -507,7 +507,7 @@ TEST(ClientTest, CommitMutatorRollback) {
507507
auto conn = std::make_shared<MockConnection>();
508508
Transaction txn = MakeReadWriteTransaction(); // placeholder
509509
Connection::ReadParams actual_read_params{txn, {}, {}, {}, {},
510-
{}, {}, {}, {}};
510+
{}, {}, {}, {}, {}};
511511

512512
auto source = std::make_unique<MockResultSetSource>();
513513
auto constexpr kText = R"pb(
@@ -550,7 +550,7 @@ TEST(ClientTest, CommitMutatorRollbackError) {
550550
auto conn = std::make_shared<MockConnection>();
551551
Transaction txn = MakeReadWriteTransaction(); // placeholder
552552
Connection::ReadParams actual_read_params{txn, {}, {}, {}, {},
553-
{}, {}, {}, {}};
553+
{}, {}, {}, {}, {}};
554554

555555
auto source = std::make_unique<MockResultSetSource>();
556556
auto constexpr kText = R"pb(

‎google/cloud/spanner/connection.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "google/cloud/spanner/lock_hint.h"
2323
#include "google/cloud/spanner/mutations.h"
2424
#include "google/cloud/spanner/options.h"
25+
#include "google/cloud/spanner/order_by.h"
2526
#include "google/cloud/spanner/partition_options.h"
2627
#include "google/cloud/spanner/partitioned_dml_result.h"
2728
#include "google/cloud/spanner/query_options.h"
@@ -82,6 +83,7 @@ class Connection {
8283
absl::optional<std::string> partition_token;
8384
bool partition_data_boost = false; // when partition_token
8485
DirectedReadOption::Type directed_read_option;
86+
OrderBy order_by;
8587
LockHint lock_hint;
8688
};
8789

‎google/cloud/spanner/google_cloud_cpp_spanner.bzl‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ google_cloud_cpp_spanner_hdrs = [
101101
"numeric.h",
102102
"oid.h",
103103
"options.h",
104+
"order_by.h",
104105
"partition_options.h",
105106
"partitioned_dml_result.h",
106107
"polling_policy.h",

‎google/cloud/spanner/integration_tests/client_integration_test.cc‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,18 @@ class ClientIntegrationTest : public spanner_testing::DatabaseIntegrationTest {
8787
spanner_testing::DatabaseIntegrationTest::TearDownTestSuite();
8888
}
8989

90+
static void InsertUnorderedSingers() {
91+
auto commit_result = client_->Commit(
92+
Mutations{InsertMutationBuilder("Singers",
93+
{"SingerId", "FirstName", "LastName"})
94+
.EmplaceRow(3, "test-fname-3", "test-lname-3")
95+
.EmplaceRow(1, "test-fname-1", "test-lname-1")
96+
.EmplaceRow(2, "test-fname-2", "test-lname-2")
97+
.Build()},
98+
Options{}.set<GrpcCompressionAlgorithmOption>(GRPC_COMPRESS_DEFLATE));
99+
ASSERT_STATUS_OK(commit_result);
100+
}
101+
90102
static std::unique_ptr<Client> client_;
91103
};
92104

@@ -834,6 +846,23 @@ TEST_F(ClientIntegrationTest, DirectedReadWithinReadWriteTransaction) {
834846
"in a read-only transaction")));
835847
}
836848

849+
/// @test Verify that Read() returns rows ordered by primary key.
850+
TEST_F(ClientIntegrationTest, ReadWithOrderByPrimaryKey) {
851+
ASSERT_NO_FATAL_FAILURE(InsertUnorderedSingers());
852+
853+
auto rows = client_->Read(
854+
"Singers", KeySet::All(), {"SingerId", "FirstName", "LastName"},
855+
Options{}.set<OrderByOption>(OrderBy::kOrderByPrimaryKey));
856+
857+
using RowType = std::tuple<std::int64_t, std::string, std::string>;
858+
std::vector<std::int64_t> actual_singer_ids;
859+
for (auto& row : StreamOf<RowType>(rows)) {
860+
if (!row) break;
861+
actual_singer_ids.push_back(std::get<0>(*row));
862+
}
863+
EXPECT_THAT(actual_singer_ids, ::testing::ElementsAre(1, 2, 3));
864+
}
865+
837866
StatusOr<std::vector<std::vector<Value>>> AddSingerDataToTable(Client client) {
838867
std::vector<std::vector<Value>> expected_rows;
839868
auto commit = client.Commit(

‎google/cloud/spanner/internal/connection_impl.cc‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,21 @@ google::spanner::v1::RequestOptions_Priority ProtoRequestPriority(
190190
return google::spanner::v1::RequestOptions::PRIORITY_UNSPECIFIED;
191191
}
192192

193+
google::spanner::v1::ReadRequest_OrderBy ProtoOrderBy(
194+
absl::optional<spanner::OrderBy> const& order_by) {
195+
if (order_by) {
196+
switch (*order_by) {
197+
case spanner::OrderBy::kOrderByUnspecified:
198+
return google::spanner::v1::ReadRequest_OrderBy_ORDER_BY_UNSPECIFIED;
199+
case spanner::OrderBy::kOrderByPrimaryKey:
200+
return google::spanner::v1::ReadRequest_OrderBy_ORDER_BY_PRIMARY_KEY;
201+
case spanner::OrderBy::kOrderByNoOrder:
202+
return google::spanner::v1::ReadRequest_OrderBy_ORDER_BY_NO_ORDER;
203+
}
204+
}
205+
return google::spanner::v1::ReadRequest_OrderBy_ORDER_BY_UNSPECIFIED;
206+
}
207+
193208
google::spanner::v1::ReadRequest_LockHint ProtoLockHint(
194209
absl::optional<spanner::LockHint> const& order_by) {
195210
if (order_by) {
@@ -575,6 +590,7 @@ spanner::RowStream ConnectionImpl::ReadImpl(
575590
*request->mutable_transaction() = *s;
576591
request->set_table(std::move(params.table));
577592
request->set_index(std::move(params.read_options.index_name));
593+
request->set_order_by(ProtoOrderBy(params.order_by));
578594
request->set_lock_hint(ProtoLockHint(params.lock_hint));
579595
for (auto&& column : params.columns) {
580596
request->add_columns(std::move(column));

‎google/cloud/spanner/internal/connection_impl_test.cc‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,10 @@ MATCHER_P(HasReplicaType, type, "has replica type") {
211211
return arg.type() == type;
212212
}
213213

214+
MATCHER_P(HasOrderBy, order_by, "has order_by") {
215+
return arg.order_by() == order_by;
216+
}
217+
214218
MATCHER_P(HasLockHint, lock_hint, "has lock_hint") {
215219
return arg.lock_hint() == lock_hint;
216220
}
@@ -4095,6 +4099,85 @@ TEST(ConnectionImplTest, RollbackSessionNotFound) {
40954099
EXPECT_THAT(txn, HasBadSession());
40964100
}
40974101

4102+
TEST(ConnectionImplTest, ReadRequestOrderByParameterUnspecified) {
4103+
auto mock = std::make_shared<spanner_testing::MockSpannerStub>();
4104+
auto db = spanner::Database("project", "instance", "database");
4105+
EXPECT_CALL(*mock, BatchCreateSessions(_, _, HasDatabase(db)))
4106+
.WillOnce(Return(MakeSessionsResponse({"test-session-name"})));
4107+
EXPECT_CALL(*mock,
4108+
AsyncDeleteSession(_, _, _, HasSessionName("test-session-name")))
4109+
.WillOnce(Return(make_ready_future(Status{})));
4110+
Sequence s;
4111+
EXPECT_CALL(
4112+
*mock,
4113+
StreamingRead(
4114+
_, _,
4115+
AllOf(HasSession("test-session-name"),
4116+
HasOrderBy(
4117+
google::spanner::v1::ReadRequest::ORDER_BY_UNSPECIFIED))))
4118+
.InSequence(s)
4119+
.WillOnce(Return(ByMove(MakeReader<google::spanner::v1::PartialResultSet>(
4120+
{R"pb(metadata: { transaction: { id: "txn1" } })pb"}))));
4121+
4122+
auto conn = MakeConnectionImpl(db, mock);
4123+
internal::OptionsSpan span(MakeLimitedTimeOptions());
4124+
4125+
// Scenario 1: No explicit OrderBy (should map to UNSPECIFIED)
4126+
spanner::ReadOptions read_options;
4127+
spanner::Transaction txn1 =
4128+
MakeReadOnlyTransaction(spanner::Transaction::ReadOnlyOptions());
4129+
auto rows1 = conn->Read(
4130+
{txn1, "table", spanner::KeySet::All(), {"col"}, read_options});
4131+
for (auto const& row : rows1) {
4132+
(void)row;
4133+
}
4134+
EXPECT_THAT(txn1,
4135+
HasSessionAndTransaction("test-session-name", "txn1", false, ""));
4136+
}
4137+
4138+
TEST(ConnectionImplTest, ReadRequestOrderByParameterNoOrder) {
4139+
auto mock = std::make_shared<spanner_testing::MockSpannerStub>();
4140+
auto db = spanner::Database("project", "instance", "database");
4141+
EXPECT_CALL(*mock, BatchCreateSessions(_, _, HasDatabase(db)))
4142+
.WillOnce(Return(MakeSessionsResponse({"test-session-name"})));
4143+
EXPECT_CALL(*mock,
4144+
AsyncDeleteSession(_, _, _, HasSessionName("test-session-name")))
4145+
.WillOnce(Return(make_ready_future(Status{})));
4146+
Sequence s;
4147+
EXPECT_CALL(
4148+
*mock,
4149+
StreamingRead(
4150+
_, _,
4151+
AllOf(
4152+
HasSession("test-session-name"),
4153+
HasOrderBy(google::spanner::v1::ReadRequest::ORDER_BY_NO_ORDER))))
4154+
.InSequence(s)
4155+
.WillOnce(Return(ByMove(MakeReader<google::spanner::v1::PartialResultSet>(
4156+
{R"pb(metadata: { transaction: { id: "txn1" } })pb"}))));
4157+
4158+
auto conn = MakeConnectionImpl(db, mock);
4159+
internal::OptionsSpan span(MakeLimitedTimeOptions());
4160+
spanner::ReadOptions read_options;
4161+
spanner::Transaction txn1 =
4162+
MakeReadOnlyTransaction(spanner::Transaction::ReadOnlyOptions());
4163+
auto read_params =
4164+
spanner::Connection::ReadParams{txn1,
4165+
"table",
4166+
spanner::KeySet::All(),
4167+
{"col"},
4168+
read_options,
4169+
absl::nullopt,
4170+
false,
4171+
spanner::DirectedReadOption::Type{},
4172+
spanner::OrderBy::kOrderByNoOrder};
4173+
auto rows1 = conn->Read(read_params);
4174+
for (auto const& row : rows1) {
4175+
(void)row;
4176+
}
4177+
EXPECT_THAT(txn1,
4178+
HasSessionAndTransaction("test-session-name", "txn1", false, ""));
4179+
}
4180+
40984181
TEST(ConnectionImplTest, ReadRequestLockHintParameterUnspecified) {
40994182
auto mock = std::make_shared<spanner_testing::MockSpannerStub>();
41004183
auto db = spanner::Database("project", "instance", "database");
@@ -4165,6 +4248,7 @@ TEST(ConnectionImplTest, ReadRequestLockHintShared) {
41654248
absl::nullopt,
41664249
false,
41674250
spanner::DirectedReadOption::Type{},
4251+
spanner::OrderBy::kOrderByUnspecified,
41684252
spanner::LockHint::kLockHintShared};
41694253
auto rows1 = conn->Read(read_params);
41704254
for (auto const& row : rows1) {

‎google/cloud/spanner/options.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
#include "google/cloud/spanner/directed_read_replicas.h"
4343
#include "google/cloud/spanner/internal/session.h"
4444
#include "google/cloud/spanner/lock_hint.h"
45+
#include "google/cloud/spanner/order_by.h"
4546
#include "google/cloud/spanner/polling_policy.h"
4647
#include "google/cloud/spanner/request_priority.h"
4748
#include "google/cloud/spanner/retry_policy.h"
@@ -196,6 +197,16 @@ struct SessionPoolActionOnExhaustionOption {
196197
using Type = spanner::ActionOnExhaustion;
197198
};
198199

200+
/**
201+
* Option for `google::cloud::Options` to set the order in which the rows are
202+
* returned from a read request.
203+
*
204+
* @ingroup google-cloud-spanner-options
205+
*/
206+
struct OrderByOption {
207+
using Type = spanner::OrderBy;
208+
};
209+
199210
/**
200211
* Option for `google::cloud::Options` to set the lock hint mechanism for reads
201212
* done within a transaction.

‎google/cloud/spanner/order_by.h‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright 2025 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
#ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_ORDER_BY_H
16+
#define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_ORDER_BY_H
17+
18+
#include "google/cloud/spanner/version.h"
19+
20+
namespace google {
21+
namespace cloud {
22+
namespace spanner {
23+
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
24+
25+
// Controls the order in which rows are returned from a read request.
26+
enum class OrderBy {
27+
kOrderByUnspecified,
28+
kOrderByPrimaryKey,
29+
kOrderByNoOrder,
30+
};
31+
32+
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
33+
} // namespace spanner
34+
} // namespace cloud
35+
} // namespace google
36+
37+
#endif // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_ORDER_BY_H

0 commit comments

Comments
 (0)