Skip to content

Commit 81b496f

Browse files
mpeddada1mpeddada1
authored
test(bigtable): add conformance test for InvalidTypes (#15787)
* test(bigtable): add conformance test for InvalidTypes
1 parent 8e3250e commit 81b496f

File tree

4 files changed

+156
-15
lines changed

4 files changed

+156
-15
lines changed

‎google/cloud/bigtable/ci/run_conformance_tests_proxy_bazel.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ exit_status=$?
5151
# unimplemented features or issues with the tests themselves.
5252
go test -v \
5353
-run "TestExecuteQuery" \
54-
-skip "CloseClient|TestExecuteQuery_FailsOnInvalidType|TestExecuteQuery_PlanRefresh_RespectsDeadline" \
54+
-skip "CloseClient|TestExecuteQuery_PlanRefresh_RespectsDeadline" \
5555
-proxy_addr=:9999
5656
exit_status=$?
5757

‎google/cloud/bigtable/internal/data_connection_impl.cc‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -759,6 +759,19 @@ StatusOr<bigtable::PreparedQuery> DataConnectionImpl::PrepareQuery(
759759
if (!response) {
760760
return std::move(response).status();
761761
}
762+
auto const& proto_schema = response->metadata().proto_schema();
763+
auto const columns_size = proto_schema.columns_size();
764+
if (columns_size == 0) {
765+
return internal::InternalError("ResultSetMetadata columns cannot be empty",
766+
GCP_ERROR_INFO());
767+
}
768+
for (auto const& column : proto_schema.columns()) {
769+
if (!column.has_type() ||
770+
column.type().kind_case() == google::bigtable::v2::Type::KIND_NOT_SET) {
771+
return internal::InternalError("Column type cannot be empty",
772+
GCP_ERROR_INFO());
773+
}
774+
}
762775
auto const* func = __func__;
763776
auto refresh_fn = [this, request, func]() mutable {
764777
auto current = google::cloud::internal::SaveCurrentOptions();
@@ -842,7 +855,20 @@ future<StatusOr<bigtable::PreparedQuery>> DataConnectionImpl::AsyncPrepareQuery(
842855
if (!response) {
843856
return std::move(response).status();
844857
}
845-
858+
auto const& proto_schema = response->metadata().proto_schema();
859+
auto const columns_size = proto_schema.columns_size();
860+
if (columns_size == 0) {
861+
return internal::InternalError(
862+
"ResultSetMetadata columns cannot be empty", GCP_ERROR_INFO());
863+
}
864+
for (auto const& column : proto_schema.columns()) {
865+
if (!column.has_type() ||
866+
column.type().kind_case() ==
867+
google::bigtable::v2::Type::KIND_NOT_SET) {
868+
return StatusOr<bigtable::PreparedQuery>(internal::InternalError(
869+
"Column type cannot be empty", GCP_ERROR_INFO()));
870+
}
871+
}
846872
auto refresh_fn = [this, request, func]() mutable {
847873
auto current = google::cloud::internal::SaveCurrentOptions();
848874
auto retry = query_plan_refresh_function_retry_policy(*current);

‎google/cloud/bigtable/internal/data_connection_impl_test.cc‎

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

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

29362966
auto fake_cq_impl = std::make_shared<FakeCompletionQueueImpl>();
@@ -3758,6 +3788,96 @@ TEST_F(DataConnectionTest, ExecuteQueryFailureWithSchemaChange) {
37583788
fake_cq_impl->SimulateCompletion(false);
37593789
}
37603790

3791+
TEST_F(DataConnectionTest, PrepareQueryFailsOnInvalidType) {
3792+
auto mock = std::make_shared<MockBigtableStub>();
3793+
auto fake_cq_impl = std::make_shared<FakeCompletionQueueImpl>();
3794+
auto mock_bg = std::make_unique<MockBackgroundThreads>();
3795+
EXPECT_CALL(*mock_bg, cq).WillRepeatedly([&]() {
3796+
return CompletionQueue{fake_cq_impl};
3797+
});
3798+
3799+
v2::PrepareQueryResponse pq_response;
3800+
pq_response.set_prepared_query("test-pq-id-54321");
3801+
auto constexpr kResultMetadataText = R"pb(
3802+
proto_schema {
3803+
columns {
3804+
name: "test"
3805+
type { string_type {} }
3806+
}
3807+
columns {
3808+
name: "invalid"
3809+
type {}
3810+
}
3811+
}
3812+
)pb";
3813+
ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(
3814+
kResultMetadataText, pq_response.mutable_metadata()));
3815+
*pq_response.mutable_valid_until() = internal::ToProtoTimestamp(
3816+
std::chrono::system_clock::now() + std::chrono::seconds(3600));
3817+
3818+
EXPECT_CALL(*mock, PrepareQuery)
3819+
.WillOnce([&](grpc::ClientContext&, Options const&,
3820+
v2::PrepareQueryRequest const&) { return pq_response; });
3821+
3822+
auto conn = TestConnection(std::move(mock));
3823+
internal::OptionsSpan span(CallOptions());
3824+
auto params = bigtable::PrepareQueryParams{
3825+
bigtable::InstanceResource(google::cloud::Project("the-project"),
3826+
"the-instance"),
3827+
bigtable::SqlStatement("SELECT * FROM the-table")};
3828+
auto prepared_query = conn->PrepareQuery(params);
3829+
EXPECT_THAT(prepared_query,
3830+
StatusIs(StatusCode::kInternal, "Column type cannot be empty"));
3831+
3832+
fake_cq_impl->SimulateCompletion(false);
3833+
}
3834+
3835+
TEST_F(DataConnectionTest, AsyncPrepareQueryFailsOnInvalidType) {
3836+
auto mock = std::make_shared<MockBigtableStub>();
3837+
auto fake_cq_impl = std::make_shared<FakeCompletionQueueImpl>();
3838+
auto mock_bg = std::make_unique<MockBackgroundThreads>();
3839+
EXPECT_CALL(*mock_bg, cq).WillRepeatedly([&]() {
3840+
return CompletionQueue{fake_cq_impl};
3841+
});
3842+
3843+
v2::PrepareQueryResponse pq_response;
3844+
pq_response.set_prepared_query("test-pq-id-54321");
3845+
auto constexpr kResultMetadataText = R"pb(
3846+
proto_schema {
3847+
columns {
3848+
name: "test"
3849+
type { string_type {} }
3850+
}
3851+
columns {
3852+
name: "invalid"
3853+
type {}
3854+
}
3855+
}
3856+
)pb";
3857+
ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(
3858+
kResultMetadataText, pq_response.mutable_metadata()));
3859+
*pq_response.mutable_valid_until() = internal::ToProtoTimestamp(
3860+
std::chrono::system_clock::now() + std::chrono::seconds(3600));
3861+
3862+
EXPECT_CALL(*mock, AsyncPrepareQuery)
3863+
.WillOnce([&](CompletionQueue const&, auto, auto,
3864+
v2::PrepareQueryRequest const&) {
3865+
return make_ready_future(make_status_or(pq_response));
3866+
});
3867+
3868+
auto conn = TestConnection(std::move(mock));
3869+
internal::OptionsSpan span(CallOptions());
3870+
auto params = bigtable::PrepareQueryParams{
3871+
bigtable::InstanceResource(google::cloud::Project("the-project"),
3872+
"the-instance"),
3873+
bigtable::SqlStatement("SELECT * FROM the-table")};
3874+
auto prepared_query = conn->AsyncPrepareQuery(params).get();
3875+
EXPECT_THAT(prepared_query,
3876+
StatusIs(StatusCode::kInternal, "Column type cannot be empty"));
3877+
3878+
fake_cq_impl->SimulateCompletion(false);
3879+
}
3880+
37613881
} // namespace
37623882
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
37633883
} // namespace bigtable_internal

‎google/cloud/bigtable/internal/partial_result_set_source.cc‎

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

219-
if (columns_size == 0 && !proto_values.empty()) {
220-
return internal::InternalError(
221-
"ProtoRows has values but ResultSetMetadata columns cannot be empty",
222-
GCP_ERROR_INFO());
223-
}
224219
if (proto_values.size() % columns_size != 0) {
225220
state_ = State::kFinished;
226221
return internal::InternalError(

0 commit comments

Comments
 (0)