Skip to content

Conversation

@mpeddada1
Copy link
Collaborator

@mpeddada1 mpeddada1 commented Oct 28, 2025

Introducing implementation to create a PreparedQuery through DataConnectionImpl#PrepareQuery.

@mpeddada1 mpeddada1 changed the title feat(bigtablee): implement PrepareQuery in DataConnectionImpl feat(bigtable): implement PrepareQuery in DataConnectionImpl Oct 28, 2025
@mpeddada1 mpeddada1 marked this pull request as ready for review October 28, 2025 15:46
@mpeddada1 mpeddada1 requested a review from a team as a code owner October 28, 2025 15:46
@codecov
Copy link

codecov bot commented Oct 28, 2025

Codecov Report

❌ Patch coverage is 97.95918% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 93.10%. Comparing base (30acc3c) to head (d1dc7fa).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...le/cloud/bigtable/internal/data_connection_impl.cc 94.11% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #15661      +/-   ##
==========================================
- Coverage   93.11%   93.10%   -0.01%     
==========================================
  Files        2439     2439              
  Lines      224091   224136      +45     
==========================================
+ Hits       208655   208680      +25     
- Misses      15436    15456      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mpeddada1
Copy link
Collaborator Author

demo-fedora-pr is resulting in the following error when emplace is used:

 /workspace/google/cloud/bigtable/internal/data_connection_impl.cc: In member function 'virtual google::cloud::v2_44::StatusOr<google::cloud::bigtable::v2_44::PreparedQuery> google::cloud::bigtable_internal::v2_44::DataConnectionImpl::PrepareQuery(const google::cloud::bigtable::v2_44::PrepareQueryParams&)':
Step #2: /workspace/google/cloud/bigtable/internal/data_connection_impl.cc:629:36: error: 'class google::protobuf::Map<std::__cxx11::basic_string<char>, google::bigtable::v2::Type>' has no member named 'emplace'
Step #2:   629 |     request.mutable_param_types()->emplace(p.first, p.second.type());
Step #2:       |                                    ^~~~~~~

On the other hand, the demo-debian-bookwork-pr results in the following error when insert is used:

/workspace/google/cloud/bigtable/internal/data_connection_impl.cc:629:42: error: no matching function for call to 'google::protobuf::Map<std::__cxx11::basic_string<char>, google::bigtable::v2::Type>::insert(std::pair<std::__cxx11::basic_string<char>, google::bigtable::v2::Type>)'
Step #2:   629 |     request.mutable_param_types()->insert(
Step #2:       |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
Step #2:   630 |         std::make_pair(p.first, p.second.type()));

}
auto response = google::cloud::internal::RetryLoop(
retry_policy(*current), backoff_policy(*current),
Idempotency::kNonIdempotent,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should check with the Bigtable team to see if we're correct in treating this RPC as non-idempotent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm that's a good point, will confirm if this the desired behavior (created #15663 for an immediate follow up)

@mpeddada1 mpeddada1 merged commit 4808d65 into googleapis:main Oct 28, 2025
63 of 67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants