Merge branch 'main' into schema-change-error

Author: mpeddada1

google/cloud/bigtable/bound_query.cc

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include "google/cloud/bigtable/bound_query.h"
+#include "google/cloud/bigtable/internal/query_plan.h"
 
 namespace google {
 namespace cloud {
@@ -23,12 +24,6 @@ StatusOr<google::bigtable::v2::PrepareQueryResponse> BoundQuery::response() {
   return query_plan_->response();
 }
 
-std::unordered_map<std::string, Value> const& BoundQuery::parameters() const {
-  return parameters_;
-}
-
-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();

google/cloud/bigtable/bound_query.h

@@ -16,7 +16,6 @@
 #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_BIGTABLE_BOUND_QUERY_H
 
 #include "google/cloud/bigtable/instance_resource.h"
-#include "google/cloud/bigtable/internal/query_plan.h"
 #include "google/cloud/bigtable/value.h"
 #include "google/cloud/bigtable/version.h"
 #include "google/cloud/completion_queue.h"
@@ -29,6 +28,7 @@ namespace cloud {
 namespace bigtable_internal {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 class DataConnectionImpl;
+class QueryPlan;
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace bigtable_internal
 
@@ -48,17 +48,14 @@ class BoundQuery {
   BoundQuery& operator=(BoundQuery&&) = default;
 
   // Accessors
+  InstanceResource const& instance() const { return instance_; }
+  std::unordered_map<std::string, Value> const& parameters() const {
+    return parameters_;
+  }
   StatusOr<google::bigtable::v2::PrepareQueryResponse> response();
-  std::unordered_map<std::string, Value> const& parameters() const;
-  InstanceResource const& instance() const;
 
   google::bigtable::v2::ExecuteQueryRequest ToRequestProto() const;
 
-  GOOGLE_CLOUD_CPP_DEPRECATED("use response()")
-  StatusOr<std::string> prepared_query() const;
-  GOOGLE_CLOUD_CPP_DEPRECATED("use response()")
-  StatusOr<google::bigtable::v2::ResultSetMetadata> metadata() const;
-
  private:
   friend class PreparedQuery;
   friend class bigtable_internal::DataConnectionImpl;

google/cloud/bigtable/bound_query_test.cc

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include "google/cloud/bigtable/bound_query.h"
+#include "google/cloud/bigtable/internal/query_plan.h"
 #include "google/cloud/bigtable/prepared_query.h"
 #include "google/cloud/bigtable/sql_statement.h"
 #include "google/cloud/bigtable/value.h"
@@ -27,7 +28,7 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 namespace {
 using ::google::bigtable::v2::PrepareQueryResponse;
 
-TEST(BoundQuery, FromPreparedQuery) {
+TEST(BoundQueryTest, FromPreparedQuery) {
   auto fake_cq_impl = std::make_shared<testing_util::FakeCompletionQueueImpl>();
   Project p("dummy-project");
   InstanceResource instance(p, "dummy-instance");
@@ -67,7 +68,7 @@ TEST(BoundQuery, FromPreparedQuery) {
   fake_cq_impl->SimulateCompletion(false);
 }
 
-TEST(BoundQuery, ToRequestProto) {
+TEST(BoundQueryTest, ToRequestProto) {
   auto fake_cq_impl = std::make_shared<testing_util::FakeCompletionQueueImpl>();
   Project p("dummy-project");
   InstanceResource instance(p, "dummy-instance");

google/cloud/bigtable/internal/data_connection_impl.cc

@@ -24,6 +24,7 @@
 #include "google/cloud/bigtable/internal/partial_result_set_reader.h"
 #include "google/cloud/bigtable/internal/partial_result_set_resume.h"
 #include "google/cloud/bigtable/internal/partial_result_set_source.h"
+#include "google/cloud/bigtable/internal/query_plan.h"
 #include "google/cloud/bigtable/internal/retry_traits.h"
 #include "google/cloud/bigtable/internal/rpc_policy_parameters.h"
 #include "google/cloud/bigtable/options.h"

google/cloud/bigtable/internal/data_connection_impl_test.cc

@@ -15,6 +15,7 @@
 #include "google/cloud/bigtable/internal/data_connection_impl.h"
 #include "google/cloud/bigtable/data_connection.h"
 #include "google/cloud/bigtable/internal/defaults.h"
+#include "google/cloud/bigtable/internal/query_plan.h"
 #ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS
 #include "google/cloud/bigtable/internal/metrics.h"
 #include "google/cloud/testing_util/fake_clock.h"

google/cloud/bigtable/internal/query_plan.cc

@@ -35,6 +35,10 @@ std::shared_ptr<QueryPlan> QueryPlan::Create(
   return plan;
 }
 
+QueryPlan::~QueryPlan() {
+  if (refresh_timer_.valid()) refresh_timer_.cancel();
+}
+
 void QueryPlan::Initialize() {
   std::unique_lock<std::mutex> lock(mu_);
   if (state_ == RefreshState::kDone) ScheduleRefresh(lock);

google/cloud/bigtable/internal/query_plan.h

@@ -43,6 +43,8 @@ class QueryPlan : public std::enable_shared_from_this<QueryPlan> {
       StatusOr<google::bigtable::v2::PrepareQueryResponse> response,
       RefreshFn fn, std::shared_ptr<Clock> clock = std::make_shared<Clock>());
 
+  ~QueryPlan();
+
   // Invalidates the current QueryPlan and triggers a refresh.
   void Invalidate(Status status, std::string const& invalid_query_plan_id);
 

google/cloud/bigtable/prepared_query.cc

@@ -25,12 +25,6 @@ BoundQuery PreparedQuery::BindParameters(
   return BoundQuery(instance_, query_plan_, std::move(params));
 }
 
-InstanceResource const& PreparedQuery::instance() const { return instance_; }
-
-SqlStatement const& PreparedQuery::sql_statement() const {
-  return sql_statement_;
-}
-
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace bigtable
 }  // namespace cloud

google/cloud/bigtable/prepared_query.h

@@ -17,7 +17,6 @@
 
 #include "google/cloud/bigtable/bound_query.h"
 #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"
@@ -28,28 +27,33 @@
 
 namespace google {
 namespace cloud {
+namespace bigtable_internal {
+GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
+class QueryPlan;
+GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
+}  // namespace bigtable_internal
 namespace bigtable {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 
 // Represents a long-lived query execution plan.
 // Query plans can expire and are refreshed as a background task.
 class PreparedQuery {
  public:
-  // Creates an instance of BoundQuery using the query plan ID from the
-  // response.
-  BoundQuery BindParameters(
-      std::unordered_map<std::string, Value> params) const;
-
-  // Accessors
-  InstanceResource const& instance() const;
-  SqlStatement const& sql_statement() const;
-
   PreparedQuery(InstanceResource instance, SqlStatement sql_statement,
                 std::shared_ptr<bigtable_internal::QueryPlan> query_plan)
       : instance_(std::move(instance)),
         sql_statement_(std::move(sql_statement)),
         query_plan_(std::move(query_plan)) {}
 
+  // Accessors
+  InstanceResource const& instance() const { return instance_; }
+  SqlStatement const& sql_statement() const { return sql_statement_; }
+
+  // Creates an instance of BoundQuery using the query plan ID from the
+  // response.
+  BoundQuery BindParameters(
+      std::unordered_map<std::string, Value> params) const;
+
  private:
   InstanceResource instance_;
   SqlStatement sql_statement_;

google/cloud/bigtable/prepared_query_test.cc

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include "google/cloud/bigtable/prepared_query.h"
+#include "google/cloud/bigtable/internal/query_plan.h"
 #include "google/cloud/bigtable/sql_statement.h"
 #include "google/cloud/testing_util/fake_completion_queue_impl.h"
 #include "google/cloud/testing_util/status_matchers.h"
@@ -25,7 +26,7 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 namespace {
 using ::google::bigtable::v2::PrepareQueryResponse;
 
-TEST(PreparedQuery, DefaultConstructor) {
+TEST(PreparedQueryTest, DefaultConstructor) {
   auto fake_cq_impl = std::make_shared<testing_util::FakeCompletionQueueImpl>();
   Project p("dummy-project");
   InstanceResource instance(p, "dummy-instance");