Merge branch 'main' into bigtable_execute_query_1

scotthart · web-flow · commit 603dc0077e5e · 2025-11-06T13:42:26.000-05:00
diff --git a/google/cloud/bigtable/bound_query.cc b/google/cloud/bigtable/bound_query.cc
@@ -19,18 +19,11 @@ namespace cloud {
 namespace bigtable {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 
-StatusOr<std::string> BoundQuery::prepared_query() const {
-  return query_plan_->prepared_query();
-}
-
-StatusOr<google::bigtable::v2::PrepareQueryResponse> BoundQuery::response() {
+StatusOr<google::bigtable::v2::PrepareQueryResponse> BoundQuery::response()
+    const {
   return query_plan_->response();
 }
 
-StatusOr<google::bigtable::v2::ResultSetMetadata> BoundQuery::metadata() const {
-  return query_plan_->metadata();
-}
-
 std::unordered_map<std::string, Value> const& BoundQuery::parameters() const {
   return parameters_;
 }
@@ -40,10 +33,6 @@ InstanceResource const& BoundQuery::instance() const { return instance_; }
 google::bigtable::v2::ExecuteQueryRequest BoundQuery::ToRequestProto() const {
   google::bigtable::v2::ExecuteQueryRequest result;
   *result.mutable_instance_name() = instance_.FullName();
-  auto prepared_query = query_plan_->prepared_query();
-  if (prepared_query.ok()) {
-    *result.mutable_prepared_query() = query_plan_->prepared_query().value();
-  }
 
   google::protobuf::Map<std::string, google::bigtable::v2::Value> parameters;
   for (auto const& kv : parameters_) {
diff --git a/google/cloud/bigtable/bound_query.h b/google/cloud/bigtable/bound_query.h
@@ -48,7 +48,7 @@ class BoundQuery {
   BoundQuery& operator=(BoundQuery&&) = default;
 
   // Accessors
-  StatusOr<google::bigtable::v2::PrepareQueryResponse> response();
+  StatusOr<google::bigtable::v2::PrepareQueryResponse> response() const;
   std::unordered_map<std::string, Value> const& parameters() const;
   InstanceResource const& instance() const;
 
diff --git a/google/cloud/bigtable/bound_query_test.cc b/google/cloud/bigtable/bound_query_test.cc
@@ -35,9 +35,6 @@ TEST(BoundQuery, FromPreparedQuery) {
       "SELECT * FROM my_table WHERE col1 = @val1 and col2 = @val2;");
   SqlStatement sql_statement(statement_contents);
   PrepareQueryResponse response;
-  std::unordered_map<std::string, Value> parameters = {{"val1", Value(true)},
-                                                       {"val2", Value(2.0)}};
-
   // The following variables are only meant to confirm the metadata is correctly
   // passed down to the BoundQuery.
   auto metadata = std::make_unique<google::bigtable::v2::ResultSetMetadata>();
@@ -47,18 +44,24 @@ TEST(BoundQuery, FromPreparedQuery) {
   schema->mutable_columns()->Add(std::move(column));
   metadata->set_allocated_proto_schema(schema.release());
   response.set_allocated_metadata(metadata.release());
+  auto refresh_fn = [&response]() {
+    return make_ready_future(StatusOr<PrepareQueryResponse>(response));
+  };
+  auto query_plan = bigtable_internal::QueryPlan::Create(
+      CompletionQueue(fake_cq_impl), std::move(response),
+      std::move(refresh_fn));
+  std::unordered_map<std::string, Value> parameters = {{"val1", Value(true)},
+                                                       {"val2", Value(2.0)}};
 
-  PreparedQuery pq(CompletionQueue(fake_cq_impl), instance, sql_statement,
-                   response);
+  PreparedQuery pq(instance, sql_statement, query_plan);
   auto bq = pq.BindParameters(parameters);
   EXPECT_EQ(instance.FullName(), bq.instance().FullName());
-  EXPECT_STATUS_OK(bq.prepared_query());
-  EXPECT_EQ(statement_contents, bq.prepared_query().value());
   EXPECT_EQ(parameters, bq.parameters());
-  EXPECT_STATUS_OK(bq.metadata());
-  EXPECT_TRUE(bq.metadata().value().has_proto_schema());
-  EXPECT_EQ(1, bq.metadata().value().proto_schema().columns_size());
-  EXPECT_EQ("col1", bq.metadata().value().proto_schema().columns()[0].name());
+  EXPECT_STATUS_OK(bq.response());
+  EXPECT_TRUE(bq.response()->metadata().has_proto_schema());
+  EXPECT_EQ(1, bq.response()->metadata().proto_schema().columns_size());
+  EXPECT_EQ("col1",
+            bq.response()->metadata().proto_schema().columns()[0].name());
 
   // Cancel all pending operations, satisfying any remaining futures.
   fake_cq_impl->SimulateCompletion(false);
@@ -72,15 +75,19 @@ TEST(BoundQuery, ToRequestProto) {
       "SELECT * FROM my_table WHERE col1 = @val1 and col2 = @val2;");
   SqlStatement sql_statement(statement_contents);
   PrepareQueryResponse response;
+  auto refresh_fn = []() {
+    return make_ready_future(StatusOr<PrepareQueryResponse>{});
+  };
+  auto query_plan = bigtable_internal::QueryPlan::Create(
+      CompletionQueue(fake_cq_impl), response, std::move(refresh_fn));
   std::unordered_map<std::string, Value> parameters = {{"val1", Value(true)},
                                                        {"val2", Value(2.0)}};
 
-  PreparedQuery pq(CompletionQueue(fake_cq_impl), instance, sql_statement,
-                   response);
+  PreparedQuery pq(instance, sql_statement, query_plan);
   auto bq = pq.BindParameters(parameters);
   google::bigtable::v2::ExecuteQueryRequest proto = bq.ToRequestProto();
   EXPECT_EQ(instance.FullName(), proto.instance_name());
-  EXPECT_EQ(statement_contents, proto.prepared_query());
+  EXPECT_EQ("", proto.prepared_query());
 
   // Test param contents.
   EXPECT_EQ(parameters.size(), proto.mutable_params()->size());
diff --git a/google/cloud/bigtable/client_test.cc b/google/cloud/bigtable/client_test.cc
@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include "google/cloud/bigtable/client.h"
+#include "google/cloud/bigtable/internal/query_plan.h"
 #include "google/cloud/bigtable/mocks/mock_data_connection.h"
 #include "google/cloud/bigtable/mocks/mock_query_row.h"
 #include "google/cloud/bigtable/mocks/mock_row_reader.h"
@@ -44,9 +45,18 @@ TEST(Client, PrepareQuery) {
         EXPECT_EQ("projects/the-project/instances/the-instance",
                   params.instance.FullName());
         EXPECT_EQ("SELECT * FROM the-table", params.sql_statement.sql());
-        PreparedQuery q(CompletionQueue{fake_cq_impl}, params.instance,
-                        params.sql_statement, PrepareQueryResponse{});
-        return q;
+
+        std::unordered_map<std::string, bigtable::Value> parameters;
+        google::bigtable::v2::PrepareQueryResponse pq_response;
+        auto refresh_fn = [&pq_response]() {
+          return make_ready_future(
+              StatusOr<google::bigtable::v2::PrepareQueryResponse>(
+                  pq_response));
+        };
+        auto query_plan = bigtable_internal::QueryPlan::Create(
+            CompletionQueue(fake_cq_impl), std::move(pq_response),
+            std::move(refresh_fn));
+        return bigtable::PreparedQuery(instance, sql, std::move(query_plan));
       });
 
   Client client(std::move(conn_mock));
@@ -67,9 +77,20 @@ TEST(Client, AsyncPrepareQuery) {
         EXPECT_EQ("projects/the-project/instances/the-instance",
                   params.instance.FullName());
         EXPECT_EQ("SELECT * FROM the-table", params.sql_statement.sql());
-        PreparedQuery q(CompletionQueue{fake_cq_impl}, params.instance,
-                        params.sql_statement, PrepareQueryResponse{});
-        return make_ready_future(make_status_or(std::move(q)));
+
+        std::unordered_map<std::string, bigtable::Value> parameters;
+        google::bigtable::v2::PrepareQueryResponse pq_response;
+        auto refresh_fn = [&pq_response]() {
+          return make_ready_future(
+              StatusOr<google::bigtable::v2::PrepareQueryResponse>(
+                  pq_response));
+        };
+        auto query_plan = bigtable_internal::QueryPlan::Create(
+            CompletionQueue(fake_cq_impl), std::move(pq_response),
+            std::move(refresh_fn));
+        StatusOr<bigtable::PreparedQuery> result =
+            bigtable::PreparedQuery(instance, sql, std::move(query_plan));
+        return make_ready_future(result);
       });
   Client client(std::move(conn_mock));
   auto prepared_query = client.AsyncPrepareQuery(instance, sql);
diff --git a/google/cloud/bigtable/prepared_query.cc b/google/cloud/bigtable/prepared_query.cc
@@ -26,6 +26,7 @@ BoundQuery PreparedQuery::BindParameters(
 }
 
 InstanceResource const& PreparedQuery::instance() const { return instance_; }
+
 SqlStatement const& PreparedQuery::sql_statement() const {
   return sql_statement_;
 }
diff --git a/google/cloud/bigtable/prepared_query.h b/google/cloud/bigtable/prepared_query.h
@@ -44,27 +44,6 @@ class PreparedQuery {
   InstanceResource const& instance() const;
   SqlStatement const& sql_statement() const;
 
-  // While we work on the Bigtable GoogleSQL functionality, we will keep this
-  // constructor as public, but this will be converted to private as
-  // originally intended once other classes that orchestrate PreparedQuery
-  // are implemented.
-  GOOGLE_CLOUD_CPP_DEPRECATED("use the other constructor")
-  PreparedQuery(CompletionQueue cq, InstanceResource instance,
-                SqlStatement sql_statement,
-                google::bigtable::v2::PrepareQueryResponse response)
-      : instance_(std::move(instance)),
-        sql_statement_(std::move(sql_statement)) {
-    *response.mutable_prepared_query() = sql_statement_.sql();
-
-    // For now, the refresh function has no effect, and we simply return a new
-    // prepared query response.
-    query_plan_ = bigtable_internal::QueryPlan::Create(
-        std::move(cq), std::move(response), [] {
-          return make_ready_future(
-              StatusOr<google::bigtable::v2::PrepareQueryResponse>{});
-        });
-  }
-
   PreparedQuery(InstanceResource instance, SqlStatement sql_statement,
                 std::shared_ptr<bigtable_internal::QueryPlan> query_plan)
       : instance_(std::move(instance)),
diff --git a/google/cloud/bigtable/prepared_query_test.cc b/google/cloud/bigtable/prepared_query_test.cc
@@ -33,8 +33,14 @@ TEST(PreparedQuery, DefaultConstructor) {
       "SELECT * FROM my_table WHERE col1 = @val1 and col2 = @val2;");
   SqlStatement sql_statement(statement_contents);
   PrepareQueryResponse response;
-  PreparedQuery q(CompletionQueue(fake_cq_impl), instance, sql_statement,
-                  response);
+  auto refresh_fn = [&response]() {
+    return make_ready_future(
+        StatusOr<google::bigtable::v2::PrepareQueryResponse>(response));
+  };
+  auto query_plan = bigtable_internal::QueryPlan::Create(
+      CompletionQueue(fake_cq_impl), std::move(response),
+      std::move(refresh_fn));
+  PreparedQuery q(instance, sql_statement, query_plan);
   EXPECT_EQ(instance.FullName(), q.instance().FullName());
   EXPECT_EQ(statement_contents, q.sql_statement().sql());
 

Original file line number	Diff line number	Diff line change
`@@ -26,6 +26,7 @@ BoundQuery PreparedQuery::BindParameters(`
`26`	`26`	`}`
`27`	`27`
`28`	`28`	`InstanceResource const& PreparedQuery::instance() const { return instance_; }`
	`29`	`+`
`29`	`30`	`SqlStatement const& PreparedQuery::sql_statement() const {`
`30`	`31`	`return sql_statement_;`
`31`	`32`	`}`