Merge branch 'main' into schema-change-error

mpeddada1 · web-flow · commit 31f035362da8 · 2025-11-12T17:16:18.000-05:00
diff --git a/.github/workflows/test-runner.yml b/.github/workflows/test-runner.yml
@@ -127,6 +127,8 @@ jobs:
       execute-integration-tests: true
     secrets: inherit
   windows-cmake:
+    # Disabled
+    if: false
     name: Windows-CMake
     needs: [pre-flight]
     uses: ./.github/workflows/windows-cmake.yml
@@ -152,7 +154,7 @@ jobs:
       # macos-bazel is disabled
       # macos-cmake is disabled
       - windows-bazel
-      - windows-cmake
+      # windows-cmake is disabled is disabled
     # Run even if the other jobs failed or were skipped.
     if: always()
     runs-on: ubuntu-latest
diff --git a/google/cloud/bigtable/CMakeLists.txt b/google/cloud/bigtable/CMakeLists.txt
@@ -249,7 +249,7 @@ add_library(
     read_modify_write_rule.h
     resource_names.cc
     resource_names.h
-    results.h
+    result_source_interface.h
     retry_policy.h
     row.h
     row_key.h
@@ -260,6 +260,8 @@ add_library(
     row_reader.h
     row_set.cc
     row_set.h
+    row_stream.cc
+    row_stream.h
     rpc_backoff_policy.cc
     rpc_backoff_policy.h
     rpc_retry_policy.cc
@@ -511,6 +513,7 @@ if (BUILD_TESTING)
         row_range_test.cc
         row_reader_test.cc
         row_set_test.cc
+        row_stream_test.cc
         row_test.cc
         rpc_backoff_policy_test.cc
         rpc_retry_policy_test.cc
diff --git a/google/cloud/bigtable/bigtable_client_unit_tests.bzl b/google/cloud/bigtable/bigtable_client_unit_tests.bzl
@@ -87,6 +87,7 @@ bigtable_client_unit_tests = [
     "row_range_test.cc",
     "row_reader_test.cc",
     "row_set_test.cc",
+    "row_stream_test.cc",
     "row_test.cc",
     "rpc_backoff_policy_test.cc",
     "rpc_retry_policy_test.cc",
diff --git a/google/cloud/bigtable/bound_query.cc b/google/cloud/bigtable/bound_query.cc
@@ -35,10 +35,12 @@ google::bigtable::v2::ExecuteQueryRequest BoundQuery::ToRequestProto() const {
 
   google::protobuf::Map<std::string, google::bigtable::v2::Value> parameters;
   for (auto const& kv : parameters_) {
-    parameters[kv.first] =
-        bigtable_internal::ValueInternals::ToProto(kv.second).second;
+    auto type_value = bigtable_internal::ValueInternals::ToProto(kv.second);
+    google::bigtable::v2::Value v = std::move(type_value.second);
+    *v.mutable_type() = std::move(type_value.first);
+    parameters[kv.first] = std::move(v);
   }
-  *result.mutable_params() = parameters;
+  *result.mutable_params() = std::move(parameters);
   return result;
 }
 
diff --git a/google/cloud/bigtable/bound_query_test.cc b/google/cloud/bigtable/bound_query_test.cc
@@ -97,12 +97,14 @@ TEST(BoundQuery, ToRequestProto) {
   auto val1 = proto.params().find("val1")->second;
   EXPECT_TRUE(val1.has_bool_value());
   EXPECT_EQ(true, val1.bool_value());
+  EXPECT_TRUE(val1.type().has_bool_type());
 
   // The second parameter is a double.
   EXPECT_TRUE(proto.params().contains("val2"));
   auto val2 = proto.params().find("val2")->second;
   EXPECT_TRUE(val2.has_float_value());
   EXPECT_EQ(2.0, val2.float_value());
+  EXPECT_TRUE(val2.type().has_float64_type());
 
   // Cancel all pending operations, satisfying any remaining futures.
   fake_cq_impl->SimulateCompletion(false);
diff --git a/google/cloud/bigtable/bytes.h b/google/cloud/bigtable/bytes.h
@@ -44,6 +44,9 @@ class Bytes {
   /// An empty sequence.
   Bytes() = default;
 
+  /// Stops copying at the null-terminator character from input bytes.
+  explicit Bytes(char const* bytes) : bytes_(bytes) {}
+
   /// @name Construction from a sequence of octets.
   ///@{
   template <typename InputIt>
diff --git a/google/cloud/bigtable/bytes_test.cc b/google/cloud/bigtable/bytes_test.cc
@@ -105,8 +105,8 @@ TEST(Bytes, OutputStream) {
       {Bytes(std::string("a\377B")), R"(B"a\377B")"},
       {Bytes(std::string("!@#$%^&*()-.")), R"(B"!@#$%^&*()-.")"},
       {Bytes(std::string(3, '\0')), R"(B"\000\000\000")"},
-      {Bytes(""), R"(B"\000")"},
-      {Bytes("foo"), R"(B"foo\000")"},
+      {Bytes(""), R"(B"")"},
+      {Bytes("foo"), R"(B"foo")"},
   };
 
   for (auto const& tc : test_cases) {
diff --git a/google/cloud/bigtable/client.h b/google/cloud/bigtable/client.h
@@ -15,7 +15,16 @@
 #ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_BIGTABLE_CLIENT_H
 #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_BIGTABLE_CLIENT_H
 
+#include "google/cloud/bigtable/bound_query.h"
 #include "google/cloud/bigtable/data_connection.h"
+#include "google/cloud/bigtable/instance_resource.h"
+#include "google/cloud/bigtable/prepared_query.h"
+#include "google/cloud/bigtable/sql_statement.h"
+#include "google/cloud/bigtable/version.h"
+#include "google/cloud/future.h"
+#include "google/cloud/options.h"
+#include "google/cloud/status_or.h"
+#include <memory>
 
 namespace google {
 namespace cloud {
@@ -81,7 +90,9 @@ class Client {
    * @param opts Unused for now
    */
   explicit Client(std::shared_ptr<DataConnection> conn, Options opts = {})
-      : conn_(std::move(conn)), opts_(std::move(opts)) {}
+      : conn_(std::move(conn)),
+        opts_(google::cloud::internal::MergeOptions(std::move(opts),
+                                                    conn_->options())) {}
 
   /**
    * Prepares a query for future execution.
diff --git a/google/cloud/bigtable/data_connection.cc b/google/cloud/bigtable/data_connection.cc
@@ -21,7 +21,7 @@
 #include "google/cloud/bigtable/internal/partial_result_set_source.h"
 #include "google/cloud/bigtable/internal/row_reader_impl.h"
 #include "google/cloud/bigtable/options.h"
-#include "google/cloud/bigtable/results.h"
+#include "google/cloud/bigtable/result_source_interface.h"
 #include "google/cloud/background_threads.h"
 #include "google/cloud/common_options.h"
 #include "google/cloud/credentials.h"
diff --git a/google/cloud/bigtable/data_connection.h b/google/cloud/bigtable/data_connection.h
@@ -20,11 +20,12 @@
 #include "google/cloud/bigtable/mutation_branch.h"
 #include "google/cloud/bigtable/mutations.h"
 #include "google/cloud/bigtable/prepared_query.h"
-#include "google/cloud/bigtable/results.h"
+#include "google/cloud/bigtable/result_source_interface.h"
 #include "google/cloud/bigtable/row.h"
 #include "google/cloud/bigtable/row_key_sample.h"
 #include "google/cloud/bigtable/row_reader.h"
 #include "google/cloud/bigtable/row_set.h"
+#include "google/cloud/bigtable/row_stream.h"
 #include "google/cloud/bigtable/sql_statement.h"
 #include "google/cloud/backoff_policy.h"
 #include "google/cloud/options.h"
diff --git a/google/cloud/bigtable/google_cloud_cpp_bigtable.bzl b/google/cloud/bigtable/google_cloud_cpp_bigtable.bzl
@@ -128,14 +128,15 @@ google_cloud_cpp_bigtable_hdrs = [
     "query_row.h",
     "read_modify_write_rule.h",
     "resource_names.h",
-    "results.h",
+    "result_source_interface.h",
     "retry_policy.h",
     "row.h",
     "row_key.h",
     "row_key_sample.h",
     "row_range.h",
     "row_reader.h",
     "row_set.h",
+    "row_stream.h",
     "rpc_backoff_policy.h",
     "rpc_retry_policy.h",
     "sql_statement.h",
@@ -240,6 +241,7 @@ google_cloud_cpp_bigtable_srcs = [
     "row_range.cc",
     "row_reader.cc",
     "row_set.cc",
+    "row_stream.cc",
     "rpc_backoff_policy.cc",
     "rpc_retry_policy.cc",
     "sql_statement.cc",
diff --git a/google/cloud/bigtable/internal/data_connection_impl.cc b/google/cloud/bigtable/internal/data_connection_impl.cc
@@ -27,7 +27,7 @@
 #include "google/cloud/bigtable/internal/retry_traits.h"
 #include "google/cloud/bigtable/internal/rpc_policy_parameters.h"
 #include "google/cloud/bigtable/options.h"
-#include "google/cloud/bigtable/results.h"
+#include "google/cloud/bigtable/result_source_interface.h"
 #include "google/cloud/bigtable/retry_policy.h"
 #include "google/cloud/background_threads.h"
 #include "google/cloud/grpc_options.h"
@@ -765,7 +765,8 @@ StatusOr<bigtable::PreparedQuery> DataConnectionImpl::PrepareQuery(
     return std::move(response).status();
   }
   auto const* func = __func__;
-  auto refresh_fn = [this, request, current, func]() mutable {
+  auto refresh_fn = [this, request, func]() mutable {
+    auto current = google::cloud::internal::SaveCurrentOptions();
     auto retry = retry_policy(*current);
     auto backoff = backoff_policy(*current);
     auto operation_context = operation_context_factory_->PrepareQuery(
@@ -847,7 +848,8 @@ future<StatusOr<bigtable::PreparedQuery>> DataConnectionImpl::AsyncPrepareQuery(
           return std::move(response).status();
         }
 
-        auto refresh_fn = [this, request, current, func]() mutable {
+        auto refresh_fn = [this, request, func]() mutable {
+          auto current = google::cloud::internal::SaveCurrentOptions();
           auto retry = retry_policy(*current);
           auto backoff = backoff_policy(*current);
           auto operation_context = operation_context_factory_->PrepareQuery(
diff --git a/google/cloud/bigtable/internal/data_connection_impl.h b/google/cloud/bigtable/internal/data_connection_impl.h
@@ -21,7 +21,7 @@
 #include "google/cloud/bigtable/internal/operation_context_factory.h"
 #include "google/cloud/bigtable/internal/partial_result_set_reader.h"
 #include "google/cloud/bigtable/prepared_query.h"
-#include "google/cloud/bigtable/results.h"
+#include "google/cloud/bigtable/result_source_interface.h"
 #include "google/cloud/background_threads.h"
 #include "google/cloud/options.h"
 #include "google/cloud/status_or.h"
diff --git a/google/cloud/bigtable/internal/partial_result_set_source.h b/google/cloud/bigtable/internal/partial_result_set_source.h
@@ -17,7 +17,7 @@
 
 #include "google/cloud/bigtable/internal/operation_context.h"
 #include "google/cloud/bigtable/internal/partial_result_set_reader.h"
-#include "google/cloud/bigtable/results.h"
+#include "google/cloud/bigtable/result_source_interface.h"
 #include "google/cloud/bigtable/value.h"
 #include "google/cloud/bigtable/version.h"
 #include "google/cloud/options.h"
diff --git a/google/cloud/bigtable/internal/query_plan.cc b/google/cloud/bigtable/internal/query_plan.cc
@@ -50,8 +50,14 @@ void QueryPlan::ScheduleRefresh(std::unique_lock<std::mutex> const&) {
   std::weak_ptr<QueryPlan> plan = shared_from_this();
   refresh_timer_ =
       cq_.MakeDeadlineTimer(refresh_deadline)
-          .then([plan](future<StatusOr<std::chrono::system_clock::time_point>>
-                           result) {
+          .then([plan, current = internal::SaveCurrentOptions()](
+                    future<StatusOr<std::chrono::system_clock::time_point>>
+                        result) {
+            // Options are stored in a thread_local variable. When this timer
+            // expires and this lambda is executed we need to restore the
+            // Options that were saved in the capture group as a different
+            // thread may be used.
+            internal::OptionsSpan options_span(current);
             if (result.get().ok()) {
               if (auto p = plan.lock()) {
                 p->ExpiredRefresh();
diff --git a/google/cloud/bigtable/query_row.cc b/google/cloud/bigtable/query_row.cc
@@ -65,49 +65,6 @@ bool operator==(QueryRow const& a, QueryRow const& b) {
   return a.values_ == b.values_ && *a.columns_ == *b.columns_;
 }
 
-//
-// RowStreamIterator
-//
-
-RowStreamIterator::RowStreamIterator() = default;
-
-RowStreamIterator::RowStreamIterator(Source source)
-    : source_(std::move(source)) {
-  ++*this;
-}
-
-RowStreamIterator& RowStreamIterator::operator++() {
-  if (!row_ok_) {
-    source_ = nullptr;  // Last row was an error; become "end"
-    return *this;
-  }
-  row_ = source_();
-  row_ok_ = row_.ok();
-  if (row_ && row_->size() == 0) {
-    source_ = nullptr;  // No more Rows to consume; become "end"
-    return *this;
-  }
-  return *this;
-}
-
-RowStreamIterator RowStreamIterator::operator++(int) {
-  auto old = *this;
-  ++*this;
-  return old;
-}
-
-bool operator==(RowStreamIterator const& a, RowStreamIterator const& b) {
-  // Input iterators may only be compared to (copies of) themselves and end.
-  // See https://en.cppreference.com/w/cpp/named_req/InputIterator. Therefore,
-  // by definition, all input iterators are equal unless one is end and the
-  // other is not.
-  return !a.source_ == !b.source_;
-}
-
-bool operator!=(RowStreamIterator const& a, RowStreamIterator const& b) {
-  return !(a == b);
-}
-
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace bigtable
 }  // namespace cloud
diff --git a/google/cloud/bigtable/query_row.h b/google/cloud/bigtable/query_row.h
diff --git a/google/cloud/bigtable/query_row_test.cc b/google/cloud/bigtable/query_row_test.cc
diff --git a/google/cloud/bigtable/result_source_interface.h b/google/cloud/bigtable/result_source_interface.h
diff --git a/google/cloud/bigtable/row_stream.cc b/google/cloud/bigtable/row_stream.cc
diff --git a/google/cloud/bigtable/row_stream.h b/google/cloud/bigtable/row_stream.h
diff --git a/google/cloud/bigtable/row_stream_test.cc b/google/cloud/bigtable/row_stream_test.cc
diff --git a/google/cloud/bigtable/tests/data_integration_test.cc b/google/cloud/bigtable/tests/data_integration_test.cc

Original file line number	Diff line number	Diff line change
`@@ -35,10 +35,12 @@ google::bigtable::v2::ExecuteQueryRequest BoundQuery::ToRequestProto() const {`
`35`	`35`
`36`	`36`	`google::protobuf::Map<std::string, google::bigtable::v2::Value> parameters;`
`37`	`37`	`for (auto const& kv : parameters_) {`
`38`		`- parameters[kv.first] =`
`39`		`- bigtable_internal::ValueInternals::ToProto(kv.second).second;`
	`38`	`+ auto type_value = bigtable_internal::ValueInternals::ToProto(kv.second);`
	`39`	`+ google::bigtable::v2::Value v = std::move(type_value.second);`
	`40`	`+ *v.mutable_type() = std::move(type_value.first);`
	`41`	`+ parameters[kv.first] = std::move(v);`
`40`	`42`	`}`
`41`		`- *result.mutable_params() = parameters;`
	`43`	`+ *result.mutable_params() = std::move(parameters);`
`42`	`44`	`return result;`
`43`	`45`	`}`
`44`	`46`