Skip to content
This repository was archived by the owner on Dec 8, 2021. It is now read-only.

Commit 8864ec4

Browse files
devjgmdevjgm
authored
removed Row<> from mutations API (#938)
Part of preparing for #387 BREAKING CHANGE: This PR removes the AddRow(Row<Ts...>) member function on the WriteMutation API because the Row<Ts...> type will be removed. In place of this method there is now an AddRow(std::vector<Value>) method. This change enables a new feature, which is that callers can now construct Mutations from Value objects, which they could not previously. Note that some of the refactoring in the integration tests exists as an intermediate step. Once the full "select *" refactoring is finished (#387), much of the integration test code could be cleaned up more.
1 parent 5364314 commit 8864ec4

File tree

3 files changed

+46
-39
lines changed

3 files changed

+46
-39
lines changed

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

Lines changed: 39 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -250,10 +250,10 @@ TEST_F(ClientIntegrationTest, RunTransaction) {
250250
// Insert SingerIds 100, 102, and 199.
251251
auto inserter = [](Client const&, Transaction const&) {
252252
auto isb =
253-
InsertMutationBuilder("Singers", {"SingerId", "FirstName", "LastName"});
254-
isb.AddRow(MakeRow(100, "first-name-100", "last-name-100"));
255-
isb.AddRow(MakeRow(102, "first-name-102", "last-name-102"));
256-
isb.AddRow(MakeRow(199, "first-name-199", "last-name-199"));
253+
InsertMutationBuilder("Singers", {"SingerId", "FirstName", "LastName"})
254+
.EmplaceRow(100, "first-name-100", "last-name-100")
255+
.EmplaceRow(102, "first-name-102", "last-name-102")
256+
.EmplaceRow(199, "first-name-199", "last-name-199");
257257
return Mutations{isb.Build()};
258258
};
259259
auto insert_result = RunTransaction(*client_, {}, inserter);
@@ -373,8 +373,8 @@ void CheckReadWithOptions(
373373
Client client,
374374
std::function<Transaction::SingleUseOptions(CommitResult const&)> const&
375375
options_generator) {
376-
using RowType = Row<std::int64_t, std::string, std::string>;
377-
std::vector<RowType> expected_rows;
376+
using RowValues = std::vector<Value>;
377+
std::vector<RowValues> expected_rows;
378378
auto commit = RunTransaction(
379379
client, Transaction::ReadWriteOptions{},
380380
[&expected_rows](Client const&,
@@ -384,7 +384,8 @@ void CheckReadWithOptions(
384384
{"SingerId", "FirstName", "LastName"});
385385
for (int i = 1; i != 10; ++i) {
386386
auto s = std::to_string(i);
387-
auto row = MakeRow(i, "test-fname-" + s, "test-lname-" + s);
387+
auto row = RowValues{Value(i), Value("test-fname-" + s),
388+
Value("test-lname-" + s)};
388389
insert.AddRow(row);
389390
expected_rows.push_back(row);
390391
}
@@ -397,14 +398,16 @@ void CheckReadWithOptions(
397398
{"SingerId", "FirstName", "LastName"});
398399
ASSERT_STATUS_OK(reader);
399400

400-
std::vector<RowType> actual_rows;
401+
std::vector<RowValues> actual_rows;
401402
if (reader) {
402403
int row_number = 0;
403-
for (auto& row : reader->Rows<RowType>()) {
404+
for (auto& row :
405+
reader->Rows<Row<std::int64_t, std::string, std::string>>()) {
404406
SCOPED_TRACE("Reading row[" + std::to_string(row_number++) + "]");
405407
EXPECT_STATUS_OK(row);
406408
if (!row) break;
407-
actual_rows.push_back(*std::move(row));
409+
auto array = std::move(row)->values();
410+
actual_rows.emplace_back(array.begin(), array.end());
408411
}
409412
}
410413
EXPECT_THAT(actual_rows, UnorderedElementsAreArray(expected_rows));
@@ -454,8 +457,8 @@ void CheckExecuteQueryWithSingleUseOptions(
454457
Client client,
455458
std::function<Transaction::SingleUseOptions(CommitResult const&)> const&
456459
options_generator) {
457-
using RowType = Row<std::int64_t, std::string, std::string>;
458-
std::vector<RowType> expected_rows;
460+
using RowValues = std::vector<Value>;
461+
std::vector<RowValues> expected_rows;
459462
auto commit = RunTransaction(
460463
client, Transaction::ReadWriteOptions{},
461464
[&expected_rows](Client const&,
@@ -464,7 +467,8 @@ void CheckExecuteQueryWithSingleUseOptions(
464467
{"SingerId", "FirstName", "LastName"});
465468
for (int i = 1; i != 10; ++i) {
466469
auto s = std::to_string(i);
467-
auto row = MakeRow(i, "test-fname-" + s, "test-lname-" + s);
470+
auto row = RowValues{Value(i), Value("test-fname-" + s),
471+
Value("test-lname-" + s)};
468472
insert.AddRow(row);
469473
expected_rows.push_back(row);
470474
}
@@ -476,13 +480,14 @@ void CheckExecuteQueryWithSingleUseOptions(
476480
options_generator(*commit),
477481
SqlStatement("SELECT SingerId, FirstName, LastName FROM Singers"));
478482

479-
std::vector<RowType> actual_rows;
483+
std::vector<RowValues> actual_rows;
480484
int row_number = 0;
481-
for (auto& row : reader.Rows<RowType>()) {
485+
for (auto& row : reader.Rows<Row<std::int64_t, std::string, std::string>>()) {
482486
SCOPED_TRACE("Reading row[" + std::to_string(row_number++) + "]");
483487
EXPECT_STATUS_OK(row);
484488
if (!row) break;
485-
actual_rows.push_back(*std::move(row));
489+
auto array = std::move(row)->values();
490+
actual_rows.emplace_back(array.begin(), array.end());
486491
}
487492

488493
EXPECT_THAT(actual_rows, UnorderedElementsAreArray(expected_rows));
@@ -531,9 +536,9 @@ TEST_F(ClientIntegrationTest, ExecuteQuery_ExactStaleness_Duration) {
531536
});
532537
}
533538

534-
StatusOr<std::vector<Row<std::int64_t, std::string, std::string>>>
535-
AddSingerDataToTable(Client const& client) {
536-
std::vector<Row<std::int64_t, std::string, std::string>> expected_rows;
539+
StatusOr<std::vector<std::vector<Value>>> AddSingerDataToTable(
540+
Client const& client) {
541+
std::vector<std::vector<Value>> expected_rows;
537542
auto commit = RunTransaction(
538543
client, Transaction::ReadWriteOptions{},
539544
[&expected_rows](Client const&,
@@ -543,7 +548,8 @@ AddSingerDataToTable(Client const& client) {
543548
{"SingerId", "FirstName", "LastName"});
544549
for (int i = 1; i != 10; ++i) {
545550
auto s = std::to_string(i);
546-
auto row = MakeRow(i, "test-fname-" + s, "test-lname-" + s);
551+
auto row = std::vector<Value>{Value(i), Value("test-fname-" + s),
552+
Value("test-lname-" + s)};
547553
insert.AddRow(row);
548554
expected_rows.push_back(row);
549555
}
@@ -572,21 +578,22 @@ TEST_F(ClientIntegrationTest, PartitionRead) {
572578
serialized_partitions.push_back(*serialized_partition);
573579
}
574580

575-
using RowType = Row<std::int64_t, std::string, std::string>;
576-
std::vector<RowType> actual_rows;
581+
std::vector<std::vector<Value>> actual_rows;
577582
int partition_number = 0;
578583
for (auto& partition : serialized_partitions) {
579584
int row_number = 0;
580585
auto deserialized_partition = DeserializeReadPartition(partition);
581586
ASSERT_STATUS_OK(deserialized_partition);
582-
auto partition_result_set = client_->Read(*deserialized_partition);
583-
ASSERT_STATUS_OK(partition_result_set);
584-
for (auto& row : partition_result_set->Rows<RowType>()) {
587+
auto result_set = client_->Read(*deserialized_partition);
588+
ASSERT_STATUS_OK(result_set);
589+
for (auto& row :
590+
result_set->Rows<Row<std::int64_t, std::string, std::string>>()) {
585591
SCOPED_TRACE("Reading partition[" + std::to_string(partition_number++) +
586592
"] row[" + std::to_string(row_number++) + "]");
587593
EXPECT_STATUS_OK(row);
588594
if (!row) break;
589-
actual_rows.push_back(*std::move(row));
595+
auto array = std::move(row)->values();
596+
actual_rows.emplace_back(array.begin(), array.end());
590597
}
591598
}
592599

@@ -610,20 +617,21 @@ TEST_F(ClientIntegrationTest, PartitionQuery) {
610617
serialized_partitions.push_back(*serialized_partition);
611618
}
612619

613-
using RowType = Row<std::int64_t, std::string, std::string>;
614-
std::vector<RowType> actual_rows;
620+
std::vector<std::vector<Value>> actual_rows;
615621
int partition_number = 0;
616622
for (auto& partition : serialized_partitions) {
617623
int row_number = 0;
618624
auto deserialized_partition = DeserializeQueryPartition(partition);
619625
ASSERT_STATUS_OK(deserialized_partition);
620-
auto partition_result_set = client_->ExecuteQuery(*deserialized_partition);
621-
for (auto& row : partition_result_set.Rows<RowType>()) {
626+
auto result_set = client_->ExecuteQuery(*deserialized_partition);
627+
for (auto& row :
628+
result_set.Rows<Row<std::int64_t, std::string, std::string>>()) {
622629
SCOPED_TRACE("Reading partition[" + std::to_string(partition_number++) +
623630
"] row[" + std::to_string(row_number++) + "]");
624631
EXPECT_STATUS_OK(row);
625632
if (!row) break;
626-
actual_rows.push_back(*std::move(row));
633+
auto array = std::move(row)->values();
634+
actual_rows.emplace_back(array.begin(), array.end());
627635
}
628636
}
629637

‎google/cloud/spanner/mutations.h‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,18 +113,17 @@ class WriteMutationBuilder {
113113
Mutation Build() const& { return Mutation(m_); }
114114
Mutation Build() && { return Mutation(std::move(m_)); }
115115

116-
template <typename... Ts>
117-
WriteMutationBuilder& AddRow(Row<Ts...> row) {
116+
WriteMutationBuilder& AddRow(std::vector<Value> values) {
118117
auto& lv = *Op::mutable_field(m_).add_values();
119-
for (auto& v : std::move(row).values()) {
118+
for (auto& v : values) {
120119
std::tie(std::ignore, *lv.add_values()) = internal::ToProto(std::move(v));
121120
}
122121
return *this;
123122
}
124123

125124
template <typename... Ts>
126125
WriteMutationBuilder& EmplaceRow(Ts&&... values) {
127-
return AddRow(MakeRow(std::forward<Ts>(values)...));
126+
return AddRow({Value(std::forward<Ts>(values))...});
128127
}
129128

130129
private:

‎google/cloud/spanner/mutations_test.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ TEST(MutationsTest, InsertComplex) {
8383
Mutation empty;
8484
auto builder =
8585
InsertMutationBuilder("table-name", {"col1", "col2", "col3"})
86-
.AddRow(MakeRow(std::int64_t(42), std::string("foo"), false))
86+
.AddRow({Value(42), Value("foo"), Value(false)})
8787
.EmplaceRow(optional<std::int64_t>(), "bar", optional<bool>{});
8888
Mutation insert = builder.Build();
8989
Mutation moved = std::move(builder).Build();
@@ -146,7 +146,7 @@ TEST(MutationsTest, UpdateComplex) {
146146
Mutation empty;
147147
auto builder =
148148
UpdateMutationBuilder("table-name", {"col_a", "col_b"})
149-
.AddRow(MakeRow(std::vector<std::string>{}, 7.0))
149+
.AddRow({Value(std::vector<std::string>{}), Value(7.0)})
150150
.EmplaceRow(std::vector<std::string>{"a", "b"}, optional<double>{});
151151
Mutation update = builder.Build();
152152
Mutation moved = std::move(builder).Build();
@@ -210,7 +210,7 @@ TEST(MutationsTest, InsertOrUpdateSimple) {
210210
TEST(MutationsTest, InsertOrUpdateComplex) {
211211
Mutation empty;
212212
auto builder = InsertOrUpdateMutationBuilder("table-name", {"col_a", "col_b"})
213-
.AddRow(MakeRow(std::make_tuple("a", 7.0)))
213+
.AddRow({Value(std::make_tuple("a", 7.0))})
214214
.EmplaceRow(std::make_tuple("b", 8.0));
215215
Mutation update = builder.Build();
216216
Mutation moved = std::move(builder).Build();
@@ -278,7 +278,7 @@ TEST(MutationsTest, ReplaceComplex) {
278278
Mutation empty;
279279
auto builder = ReplaceMutationBuilder("table-name", {"col_a", "col_b"})
280280
.EmplaceRow("a", 7.0)
281-
.AddRow(MakeRow("b", 8.0));
281+
.AddRow({Value("b"), Value(8.0)});
282282
Mutation update = builder.Build();
283283
Mutation moved = std::move(builder).Build();
284284
EXPECT_EQ(update, moved);

0 commit comments

Comments
 (0)