fix: use new PreparedQuery constructor

diegomarquezp · diegomarquezp · commit 8333b90cf0a4 · 2025-11-05T21:05:42.000Z
diff --git a/google/cloud/bigtable/bound_query.cc b/google/cloud/bigtable/bound_query.cc
@@ -19,12 +19,14 @@ namespace cloud {
 namespace bigtable {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 
-StatusOr<std::string> BoundQuery::prepared_query() const {
-  return query_plan_->prepared_query();
-}
+std::string BoundQuery::prepared_query() const { return sql_statement_->sql(); }
 
 StatusOr<google::bigtable::v2::ResultSetMetadata> BoundQuery::metadata() const {
-  return query_plan_->metadata();
+  auto response = query_plan_->response();
+  if (response.ok()) {
+    return response->metadata();
+  }
+  return response.status();
 }
 
 std::unordered_map<std::string, Value> const& BoundQuery::parameters() const {
@@ -36,9 +38,9 @@ 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();
+  auto response = query_plan_->response();
+  if (response.ok()) {
+    *result.mutable_prepared_query() = sql_statement_->sql();
   }
 
   google::protobuf::Map<std::string, google::bigtable::v2::Value> parameters;
diff --git a/google/cloud/bigtable/bound_query.h b/google/cloud/bigtable/bound_query.h
@@ -17,6 +17,7 @@
 
 #include "google/cloud/bigtable/instance_resource.h"
 #include "google/cloud/bigtable/internal/query_plan.h"
+#include "google/cloud/bigtable/sql_statement.h"
 #include "google/cloud/bigtable/value.h"
 #include "google/cloud/bigtable/version.h"
 #include "google/cloud/completion_queue.h"
@@ -42,7 +43,7 @@ class BoundQuery {
   BoundQuery& operator=(BoundQuery&&) = default;
 
   // Accessors
-  StatusOr<std::string> prepared_query() const;
+  std::string prepared_query() const;
   StatusOr<google::bigtable::v2::ResultSetMetadata> metadata() const;
   std::unordered_map<std::string, Value> const& parameters() const;
   InstanceResource const& instance() const;
@@ -53,16 +54,19 @@ class BoundQuery {
   friend class PreparedQuery;
   BoundQuery(InstanceResource instance,
              std::shared_ptr<bigtable_internal::QueryPlan> query_plan,
-             std::unordered_map<std::string, Value> parameters)
+             std::unordered_map<std::string, Value> parameters,
+             std::shared_ptr<SqlStatement> sql_statement)
       : instance_(std::move(instance)),
         query_plan_(std::move(query_plan)),
-        parameters_(std::move(parameters)) {}
+        parameters_(std::move(parameters)),
+        sql_statement_(std::move(sql_statement)) {}
 
   InstanceResource instance_;
   // Copy of the query_plan_ contained by the PreparedQuery that created
   // this BoundQuery.
   std::shared_ptr<bigtable_internal::QueryPlan> query_plan_;
   std::unordered_map<std::string, Value> parameters_;
+  std::shared_ptr<SqlStatement> sql_statement_;
 };
 
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
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,13 +44,19 @@ 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(statement_contents, bq.prepared_query());
   EXPECT_EQ(parameters, bq.parameters());
   EXPECT_STATUS_OK(bq.metadata());
   EXPECT_TRUE(bq.metadata().value().has_proto_schema());
@@ -72,11 +75,15 @@ 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());
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));
@@ -62,14 +72,23 @@ TEST(Client, AsyncPrepareQuery) {
   auto conn_mock = std::make_shared<bigtable_mocks::MockDataConnection>();
   InstanceResource instance(Project("the-project"), "the-instance");
   SqlStatement sql("SELECT * FROM the-table");
-  EXPECT_CALL(*conn_mock, AsyncPrepareQuery)
+  EXPECT_CALL(*conn_mock, PrepareQuery)
       .WillOnce([&](PrepareQueryParams const& params) {
         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));
+        return bigtable::PreparedQuery(instance, sql, std::move(query_plan));
       });
   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
@@ -22,12 +22,12 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 
 BoundQuery PreparedQuery::BindParameters(
     std::unordered_map<std::string, Value> params) const {
-  return BoundQuery(instance_, query_plan_, std::move(params));
+  return BoundQuery(instance_, query_plan_, std::move(params), sql_statement_);
 }
 
 InstanceResource const& PreparedQuery::instance() const { return instance_; }
 SqlStatement const& PreparedQuery::sql_statement() const {
-  return sql_statement_;
+  return *sql_statement_;
 }
 
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
diff --git a/google/cloud/bigtable/prepared_query.h b/google/cloud/bigtable/prepared_query.h
@@ -44,36 +44,16 @@ 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)),
-        sql_statement_(std::move(sql_statement)),
+        sql_statement_(
+            std::make_shared<SqlStatement>(std::move(sql_statement))),
         query_plan_(std::move(query_plan)) {}
 
  private:
   InstanceResource instance_;
-  SqlStatement sql_statement_;
+  std::shared_ptr<SqlStatement> sql_statement_;
   std::shared_ptr<bigtable_internal::QueryPlan> query_plan_;
 };
 
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
`@@ -22,12 +22,12 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN`
`22`	`22`
`23`	`23`	`BoundQuery PreparedQuery::BindParameters(`
`24`	`24`	`std::unordered_map<std::string, Value> params) const {`
`25`		`- return BoundQuery(instance_, query_plan_, std::move(params));`
	`25`	`+ return BoundQuery(instance_, query_plan_, std::move(params), sql_statement_);`
`26`	`26`	`}`
`27`	`27`
`28`	`28`	`InstanceResource const& PreparedQuery::instance() const { return instance_; }`
`29`	`29`	`SqlStatement const& PreparedQuery::sql_statement() const {`
`30`		`- return sql_statement_;`
	`30`	`+ return *sql_statement_;`
`31`	`31`	`}`
`32`	`32`
`33`	`33`	`GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END`