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

feat!: Use ReadResult instead of ResultSet; remove ResultSet #935

Merged
merged 1 commit into from
Oct 9, 2019
Merged

feat!: Use ReadResult instead of ResultSet; remove ResultSet #935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions google/cloud/spanner/client.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,32 +27,33 @@ namespace cloud {
namespace spanner {
inline namespace SPANNER_CLIENT_NS {

StatusOr<ResultSet> Client::Read(std::string table, KeySet keys,
std::vector<std::string> columns,
ReadOptions read_options) {
ReadResult Client::Read(std::string table, KeySet keys,
std::vector<std::string> columns,
ReadOptions read_options) {
return conn_->Read(
{internal::MakeSingleUseTransaction(Transaction::ReadOnlyOptions()),
std::move(table), std::move(keys), std::move(columns),
std::move(read_options)});
}

StatusOr<ResultSet> Client::Read(
Transaction::SingleUseOptions transaction_options, std::string table,
KeySet keys, std::vector<std::string> columns, ReadOptions read_options) {
ReadResult Client::Read(Transaction::SingleUseOptions transaction_options,
std::string table, KeySet keys,
std::vector<std::string> columns,
ReadOptions read_options) {
return conn_->Read(
{internal::MakeSingleUseTransaction(std::move(transaction_options)),
std::move(table), std::move(keys), std::move(columns),
std::move(read_options)});
}

StatusOr<ResultSet> Client::Read(Transaction transaction, std::string table,
KeySet keys, std::vector<std::string> columns,
ReadOptions read_options) {
ReadResult Client::Read(Transaction transaction, std::string table, KeySet keys,
std::vector<std::string> columns,
ReadOptions read_options) {
return conn_->Read({std::move(transaction), std::move(table), std::move(keys),
std::move(columns), std::move(read_options)});
}

StatusOr<ResultSet> Client::Read(ReadPartition const& read_partition) {
ReadResult Client::Read(ReadPartition const& read_partition) {
return conn_->Read(internal::MakeReadParams(read_partition));
}

Expand Down
46 changes: 20 additions & 26 deletions google/cloud/spanner/client.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,12 @@ inline namespace SPANNER_CLIENT_NS {
*
* @code
* namespace cs = ::google::cloud::spanner;
* using ::google::cloud::StatusOr;
*
* auto db = cs::Database("my_project", "my_instance", "my_db_id"));
* auto conn = cs::MakeConnection(db);
* auto client = cs::Client(conn);
*
* StatusOr<cs::ResultSet> result = client.Read(...);
* if (!result) {
* return result.status();
* }
* cs::ReadResult result = client.Read(...);
* using RowType = Row<std::int64_t, std::string>;
* for (auto const& row : result.Rows<RowType>()) {
* // ...
Expand Down Expand Up @@ -143,31 +139,30 @@ class Client {
* this request.
* @param read_options `ReadOptions` used for this request.
*
* @return A `StatusOr` containing a `ResultSet` or error status on failure.
* No individual row in the `ResultSet` can exceed 100 MiB, and no column
* value can exceed 10 MiB.
* @note No individual row in the `ReadResult` can exceed 100 MiB, and no
* column value can exceed 10 MiB.
*/
StatusOr<ResultSet> Read(std::string table, KeySet keys,
std::vector<std::string> columns,
ReadOptions read_options = {});
ReadResult Read(std::string table, KeySet keys,
std::vector<std::string> columns,
ReadOptions read_options = {});
/**
* @copydoc Read
*
* @param transaction_options Execute this read in a single-use transaction
* with these options.
*/
StatusOr<ResultSet> Read(Transaction::SingleUseOptions transaction_options,
std::string table, KeySet keys,
std::vector<std::string> columns,
ReadOptions read_options = {});
ReadResult Read(Transaction::SingleUseOptions transaction_options,
std::string table, KeySet keys,
std::vector<std::string> columns,
ReadOptions read_options = {});
/**
* @copydoc Read
*
* @param transaction Execute this read as part of an existing transaction.
*/
StatusOr<ResultSet> Read(Transaction transaction, std::string table,
KeySet keys, std::vector<std::string> columns,
ReadOptions read_options = {});
ReadResult Read(Transaction transaction, std::string table, KeySet keys,
std::vector<std::string> columns,
ReadOptions read_options = {});
//@}

/**
Expand All @@ -177,14 +172,13 @@ class Client {
*
* @param partition A `ReadPartition`, obtained by calling `PartitionRead`.
*
* @return A `StatusOr` containing a `ResultSet` or error status on failure.
* No individual row in the `ResultSet` can exceed 100 MiB, and no column
* value can exceed 10 MiB.
* @note No individual row in the `ReadResult` can exceed 100 MiB, and no
* column value can exceed 10 MiB.
*
* @par Example
* @snippet samples.cc read-read-partition
*/
StatusOr<ResultSet> Read(ReadPartition const& partition);
ReadResult Read(ReadPartition const& partition);

/**
* Creates a set of partitions that can be used to execute a read
Expand Down Expand Up @@ -237,8 +231,8 @@ class Client {
*
* @param statement The SQL statement to execute.
*
* @note No individual row in the `ResultSet` can exceed 100 MiB, and no
* column value can exceed 10 MiB.
* @note No individual row in the `ExecuteQueryResult` can exceed 100 MiB, and
* no column value can exceed 10 MiB.
*/
ExecuteQueryResult ExecuteQuery(SqlStatement statement);

Expand Down Expand Up @@ -266,8 +260,8 @@ class Client {
*
* @param partition A `QueryPartition`, obtained by calling `PartitionRead`.
*
* @note No individual row in the `ResultSet` can exceed 100 MiB, and no
* column value can exceed 10 MiB.
* @note No individual row in the `ExecuteQueryResult` can exceed 100 MiB, and
* no column value can exceed 10 MiB.
*
* @par Example
* @snippet samples.cc execute-sql-query-partition
Expand Down
126 changes: 96 additions & 30 deletions google/cloud/spanner/client_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,29 +96,26 @@ TEST(ClientTest, ReadSuccess) {
)pb",
&metadata));
EXPECT_CALL(*source, Metadata()).WillRepeatedly(Return(metadata));
EXPECT_CALL(*source, Stats())
.WillRepeatedly(Return(optional<spanner_proto::ResultSetStats>()));
EXPECT_CALL(*source, NextValue())
.WillOnce(Return(optional<Value>("Steve")))
.WillOnce(Return(optional<Value>(12)))
.WillOnce(Return(optional<Value>("Ann")))
.WillOnce(Return(optional<Value>(42)))
.WillOnce(Return(optional<Value>()));

ResultSet result_set(std::move(source));
ReadResult result_set(std::move(source));
EXPECT_CALL(*conn, Read(_)).WillOnce(Return(ByMove(std::move(result_set))));

KeySet keys = KeySet::All();
auto result = client.Read("table", std::move(keys), {"column1", "column2"});
EXPECT_STATUS_OK(result);

using RowType = Row<std::string, std::int64_t>;
auto expected = std::vector<RowType>{
RowType("Steve", 12),
RowType("Ann", 42),
};
int row_number = 0;
for (auto& row : result->Rows<RowType>()) {
for (auto& row : result.Rows<RowType>()) {
EXPECT_STATUS_OK(row);
EXPECT_EQ(*row, expected[row_number]);
++row_number;
Expand All @@ -143,21 +140,18 @@ TEST(ClientTest, ReadFailure) {
)pb",
&metadata));
EXPECT_CALL(*source, Metadata()).WillRepeatedly(Return(metadata));
EXPECT_CALL(*source, Stats())
.WillRepeatedly(Return(optional<spanner_proto::ResultSetStats>()));
EXPECT_CALL(*source, NextValue())
.WillOnce(Return(optional<Value>("Steve")))
.WillOnce(Return(optional<Value>("Ann")))
.WillOnce(Return(Status(StatusCode::kDeadlineExceeded, "deadline!")));

ResultSet result_set(std::move(source));
ReadResult result_set(std::move(source));
EXPECT_CALL(*conn, Read(_)).WillOnce(Return(ByMove(std::move(result_set))));

KeySet keys = KeySet::All();
auto result = client.Read("table", std::move(keys), {"column1"});
EXPECT_STATUS_OK(result);

auto rows = result->Rows<Row<std::string>>();
auto rows = result.Rows<Row<std::string>>();
auto iter = rows.begin();
EXPECT_NE(iter, rows.end());
EXPECT_STATUS_OK(*iter);
Expand Down Expand Up @@ -194,8 +188,6 @@ TEST(ClientTest, ExecuteQuerySuccess) {
)pb",
&metadata));
EXPECT_CALL(*source, Metadata()).WillRepeatedly(Return(metadata));
EXPECT_CALL(*source, Stats())
.WillRepeatedly(Return(optional<spanner_proto::ResultSetStats>()));
EXPECT_CALL(*source, NextValue())
.WillOnce(Return(optional<Value>("Steve")))
.WillOnce(Return(optional<Value>(12)))
Expand Down Expand Up @@ -241,8 +233,6 @@ TEST(ClientTest, ExecuteQueryFailure) {
)pb",
&metadata));
EXPECT_CALL(*source, Metadata()).WillRepeatedly(Return(metadata));
EXPECT_CALL(*source, Stats())
.WillRepeatedly(Return(optional<spanner_proto::ResultSetStats>()));
EXPECT_CALL(*source, NextValue())
.WillOnce(Return(optional<Value>("Steve")))
.WillOnce(Return(optional<Value>("Ann")))
Expand Down Expand Up @@ -329,8 +319,6 @@ TEST(ClientTest, ExecutePartitionedDml_Success) {
auto source = make_unique<MockResultSetSource>();
spanner_proto::ResultSetMetadata metadata;
EXPECT_CALL(*source, Metadata()).WillRepeatedly(Return(metadata));
EXPECT_CALL(*source, Stats())
.WillRepeatedly(Return(optional<spanner_proto::ResultSetStats>()));
EXPECT_CALL(*source, NextValue()).WillRepeatedly(Return(optional<Value>()));

std::string const sql_statement = "UPDATE Singers SET MarketingBudget = 1000";
Expand Down Expand Up @@ -418,17 +406,38 @@ TEST(ClientTest, RunTransactionCommit) {
Transaction txn = MakeReadWriteTransaction(); // dummy
Connection::ReadParams actual_read_params{txn, {}, {}, {}, {}};
Connection::CommitParams actual_commit_params{txn, {}};

auto source = make_unique<MockResultSetSource>();
spanner_proto::ResultSetMetadata metadata;
ASSERT_TRUE(TextFormat::ParseFromString(
R"pb(
row_type: {
fields: {
name: "Name",
type: { code: STRING }
}
}
)pb",
&metadata));
EXPECT_CALL(*source, Metadata()).WillRepeatedly(Return(metadata));
EXPECT_CALL(*source, NextValue())
.WillOnce(Return(optional<Value>("Bob")))
.WillOnce(Return(optional<Value>()));
ReadResult result_set(std::move(source));

EXPECT_CALL(*conn, Read(_))
.WillOnce(
DoAll(SaveArg<0>(&actual_read_params), Return(ByMove(ResultSet{}))));
.WillOnce(DoAll(SaveArg<0>(&actual_read_params),
Return(ByMove(std::move(result_set)))));
EXPECT_CALL(*conn, Commit(_))
.WillOnce(DoAll(SaveArg<0>(&actual_commit_params),
Return(CommitResult{*timestamp})));

auto mutation = MakeDeleteMutation("table", KeySet::All());
auto f = [&mutation](Client client, Transaction txn) -> StatusOr<Mutations> {
auto read = client.Read(std::move(txn), "T", KeySet::All(), {"C"});
if (!read) return read.status();
for (auto& row : read.Rows<Row<std::string>>()) {
if (!row) return row.status();
}
return Mutations{mutation};
};

Expand All @@ -447,16 +456,35 @@ TEST(ClientTest, RunTransactionRollback) {
auto conn = std::make_shared<MockConnection>();
Transaction txn = MakeReadWriteTransaction(); // dummy
Connection::ReadParams actual_read_params{txn, {}, {}, {}, {}};

auto source = make_unique<MockResultSetSource>();
spanner_proto::ResultSetMetadata metadata;
ASSERT_TRUE(TextFormat::ParseFromString(
R"pb(
row_type: {
fields: {
name: "Name",
type: { code: INT64 }
}
}
)pb",
&metadata));
EXPECT_CALL(*source, Metadata()).WillRepeatedly(Return(metadata));
EXPECT_CALL(*source, NextValue())
.WillOnce(Return(Status(StatusCode::kInvalidArgument, "blah")));
ReadResult result_set(std::move(source));

EXPECT_CALL(*conn, Read(_))
.WillOnce(
DoAll(SaveArg<0>(&actual_read_params),
Return(ByMove(Status(StatusCode::kInvalidArgument, "blah")))));
.WillOnce(DoAll(SaveArg<0>(&actual_read_params),
Return(ByMove(std::move(result_set)))));
EXPECT_CALL(*conn, Rollback(_)).WillOnce(Return(Status()));

auto mutation = MakeDeleteMutation("table", KeySet::All());
auto f = [&mutation](Client client, Transaction txn) -> StatusOr<Mutations> {
auto read = client.Read(std::move(txn), "T", KeySet::All(), {"C"});
if (!read) return read.status();
for (auto& row : read.Rows<Row<std::string>>()) {
if (!row) return row.status();
}
return Mutations{mutation};
};

Expand All @@ -475,17 +503,36 @@ TEST(ClientTest, RunTransactionRollbackError) {
auto conn = std::make_shared<MockConnection>();
Transaction txn = MakeReadWriteTransaction(); // dummy
Connection::ReadParams actual_read_params{txn, {}, {}, {}, {}};

auto source = make_unique<MockResultSetSource>();
spanner_proto::ResultSetMetadata metadata;
ASSERT_TRUE(TextFormat::ParseFromString(
R"pb(
row_type: {
fields: {
name: "Name",
type: { code: INT64 }
}
}
)pb",
&metadata));
EXPECT_CALL(*source, Metadata()).WillRepeatedly(Return(metadata));
EXPECT_CALL(*source, NextValue())
.WillOnce(Return(Status(StatusCode::kInvalidArgument, "blah")));
ReadResult result_set(std::move(source));

EXPECT_CALL(*conn, Read(_))
.WillOnce(
DoAll(SaveArg<0>(&actual_read_params),
Return(ByMove(Status(StatusCode::kInvalidArgument, "blah")))));
.WillOnce(DoAll(SaveArg<0>(&actual_read_params),
Return(ByMove(std::move(result_set)))));
EXPECT_CALL(*conn, Rollback(_))
.WillOnce(Return(Status(StatusCode::kInternal, "oops")));

auto mutation = MakeDeleteMutation("table", KeySet::All());
auto f = [&mutation](Client client, Transaction txn) -> StatusOr<Mutations> {
auto read = client.Read(std::move(txn), "T", KeySet::All(), {"C"});
if (!read) return read.status();
for (auto& row : read.Rows<Row<std::string>>()) {
if (!row) return row.status();
}
return Mutations{mutation};
};

Expand All @@ -503,14 +550,33 @@ TEST(ClientTest, RunTransactionRollbackError) {
#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
TEST(ClientTest, RunTransactionException) {
auto conn = std::make_shared<MockConnection>();
EXPECT_CALL(*conn, Read(_))
.WillOnce(Return(ByMove(Status(StatusCode::kInvalidArgument, "blah"))));

auto source = make_unique<MockResultSetSource>();
spanner_proto::ResultSetMetadata metadata;
ASSERT_TRUE(TextFormat::ParseFromString(
R"pb(
row_type: {
fields: {
name: "Name",
type: { code: INT64 }
}
}
)pb",
&metadata));
EXPECT_CALL(*source, Metadata()).WillRepeatedly(Return(metadata));
EXPECT_CALL(*source, NextValue())
.WillOnce(Return(Status(StatusCode::kInvalidArgument, "blah")));
ReadResult result_set(std::move(source));

EXPECT_CALL(*conn, Read(_)).WillOnce(Return(ByMove(std::move(result_set))));
EXPECT_CALL(*conn, Rollback(_)).WillOnce(Return(Status()));

auto mutation = MakeDeleteMutation("table", KeySet::All());
auto f = [&mutation](Client client, Transaction txn) -> StatusOr<Mutations> {
auto read = client.Read(std::move(txn), "T", KeySet::All(), {"C"});
if (!read) throw "Read() error";
for (auto& row : read.Rows<Row<std::string>>()) {
if (!row) throw "Read() error";
}
return Mutations{mutation};
};

Expand Down
2 changes: 1 addition & 1 deletion google/cloud/spanner/connection.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ class Connection {
//@}

/// Define the interface for a google.spanner.v1.Spanner.Read RPC
virtual StatusOr<ResultSet> Read(ReadParams) = 0;
virtual ReadResult Read(ReadParams) = 0;

/// Define the interface for a google.spanner.v1.Spanner.PartitionRead RPC
virtual StatusOr<std::vector<ReadPartition>> PartitionRead(
Expand Down
Loading