Skip to content

Commit 938c7ce

Browse files
committed
move empty metadata check to prepare query
1 parent 7965c2e commit 938c7ce

File tree

3 files changed

+52
-16
lines changed

3 files changed

+52
-16
lines changed

google/cloud/bigtable/internal/data_connection_impl.cc

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -757,7 +757,13 @@ StatusOr<bigtable::PreparedQuery> DataConnectionImpl::PrepareQuery(
757757
if (!response) {
758758
return std::move(response).status();
759759
}
760-
for (auto const& column : response->metadata().proto_schema().columns()) {
760+
auto const& proto_schema = response->metadata().proto_schema();
761+
auto const columns_size = proto_schema.columns_size();
762+
if (columns_size == 0) {
763+
return internal::InternalError("ResultSetMetadata columns cannot be empty",
764+
GCP_ERROR_INFO());
765+
}
766+
for (auto const& column : proto_schema.columns()) {
761767
if (!column.has_type() ||
762768
column.type().kind_case() == google::bigtable::v2::Type::KIND_NOT_SET) {
763769
return internal::InternalError("Column type cannot be empty",
@@ -847,8 +853,13 @@ future<StatusOr<bigtable::PreparedQuery>> DataConnectionImpl::AsyncPrepareQuery(
847853
if (!response) {
848854
return std::move(response).status();
849855
}
850-
for (auto const& column :
851-
response->metadata().proto_schema().columns()) {
856+
auto const& proto_schema = response->metadata().proto_schema();
857+
auto const columns_size = proto_schema.columns_size();
858+
if (columns_size == 0) {
859+
return internal::InternalError(
860+
"ResultSetMetadata columns cannot be empty", GCP_ERROR_INFO());
861+
}
862+
for (auto const& column : proto_schema.columns()) {
852863
if (!column.has_type() ||
853864
column.type().kind_case() ==
854865
google::bigtable::v2::Type::KIND_NOT_SET) {

google/cloud/bigtable/internal/data_connection_impl_test.cc

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2841,16 +2841,31 @@ TEST_F(DataConnectionTest, PrepareQuerySuccess) {
28412841
auto factory = std::make_unique<SimpleOperationContextFactory>();
28422842
#endif
28432843
auto mock = std::make_shared<MockBigtableStub>();
2844+
v2::PrepareQueryResponse response;
2845+
2846+
auto constexpr kResultMetadataText = R"pb(
2847+
proto_schema {
2848+
columns {
2849+
name: "row_key"
2850+
type { string_type {} }
2851+
}
2852+
columns {
2853+
name: "value"
2854+
type { string_type {} }
2855+
}
2856+
}
2857+
)pb";
2858+
*response.mutable_valid_until() = internal::ToProtoTimestamp(
2859+
std::chrono::system_clock::now() + std::chrono::seconds(3600));
2860+
ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(
2861+
kResultMetadataText, response.mutable_metadata()));
28442862
EXPECT_CALL(*mock, PrepareQuery)
2845-
.WillOnce([](grpc::ClientContext&, Options const&,
2846-
v2::PrepareQueryRequest const& request) {
2863+
.WillOnce([&](grpc::ClientContext&, Options const&,
2864+
v2::PrepareQueryRequest const& request) {
28472865
EXPECT_EQ(kAppProfile, request.app_profile_id());
28482866
EXPECT_EQ("projects/the-project/instances/the-instance",
28492867
request.instance_name());
28502868
EXPECT_EQ("SELECT * FROM the-table", request.query());
2851-
v2::PrepareQueryResponse response;
2852-
*response.mutable_valid_until() = internal::ToProtoTimestamp(
2853-
std::chrono::system_clock::now() + std::chrono::seconds(3600));
28542869
return response;
28552870
});
28562871

@@ -2922,14 +2937,29 @@ TEST_F(DataConnectionTest, AsyncPrepareQuerySuccess) {
29222937
auto factory = std::make_unique<SimpleOperationContextFactory>();
29232938
#endif
29242939
auto mock = std::make_shared<MockBigtableStub>();
2940+
auto constexpr kResultMetadataText = R"pb(
2941+
proto_schema {
2942+
columns {
2943+
name: "row_key"
2944+
type { string_type {} }
2945+
}
2946+
columns {
2947+
name: "value"
2948+
type { string_type {} }
2949+
}
2950+
}
2951+
)pb";
2952+
v2::PrepareQueryResponse response;
2953+
ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(
2954+
kResultMetadataText, response.mutable_metadata()));
29252955
EXPECT_CALL(*mock, AsyncPrepareQuery)
2926-
.WillOnce([](CompletionQueue const&, auto, auto,
2927-
v2::PrepareQueryRequest const& request) {
2956+
.WillOnce([&](CompletionQueue const&, auto, auto,
2957+
v2::PrepareQueryRequest const& request) {
29282958
EXPECT_EQ(kAppProfile, request.app_profile_id());
29292959
EXPECT_EQ("projects/the-project/instances/the-instance",
29302960
request.instance_name());
29312961
EXPECT_EQ("SELECT * FROM the-table", request.query());
2932-
return make_ready_future(make_status_or(v2::PrepareQueryResponse{}));
2962+
return make_ready_future(make_status_or(response));
29332963
});
29342964

29352965
auto fake_cq_impl = std::make_shared<FakeCompletionQueueImpl>();

google/cloud/bigtable/internal/partial_result_set_source.cc

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -210,11 +210,6 @@ Status PartialResultSetSource::BufferProtoRows() {
210210
auto const columns_size = proto_schema.columns_size();
211211
auto const& proto_values = proto_rows_.values();
212212

213-
if (columns_size == 0 && !proto_values.empty()) {
214-
return internal::InternalError(
215-
"ProtoRows has values but ResultSetMetadata columns cannot be empty",
216-
GCP_ERROR_INFO());
217-
}
218213
if (proto_values.size() % columns_size != 0) {
219214
state_ = State::kFinished;
220215
return internal::InternalError(

0 commit comments

Comments
 (0)