feat(bigtable): add support for Prepared and Bound query #15650
Conversation
product-auto-label
bot
added
the
api: bigtable
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15650 +/- ##
========================================
Coverage 93.10% 93.10%
========================================
Files 2433 2439 +6
Lines 223834 223934 +100
========================================
+ Hits 208402 208504 +102
+ Misses 15432 15430 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Seems unrelated to this PR. |
google/cloud/bigtable/query_test.cc
Outdated
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may |
google/cloud/bigtable/query_test.cc
Outdated
| namespace { | ||
| using ::google::bigtable::v2::PrepareQueryResponse; | ||
|
|
||
| class BasicInputs { |
There was a problem hiding this comment.
I recognize that some of this setup is repeated for each test, but tests should have their inputs/expected values within the body of the test case to improve readability when just looking at a single test. Let's remove this class entirely and put the necessary setup in each test case as needed, even if it means some duplication.
There was a problem hiding this comment.
I agree it's more readable. Done.
There was a problem hiding this comment.
Let's split these classes and their tests into separate sets of files: prepared_query(_test)?.[h|cc] and bound_query(_test)?.[h|cc].
There was a problem hiding this comment.
Now we have separate files. Thanks.
Have you had a chance to rebase since I submitted #15649? |
diegomarquezp
force-pushed
the
bigtable-sql/prepared-bound-query
branch
from
be62a56 to
b11b3a5
Compare
I just rebased it. Thank you. |
|
Converting back to draft as there are failing targets |
2 participants
Adds a new
query.[h|cc]file with implementations both forBoundQueryandPreparedQuery.Not all of the functionality is implemented as other parts will enable further development.
Notably,
QueryPlan's refresh function has been defined with dummy behavior.Additionally,
PreparedQuery's private constructor has been made public for testing purposes in the meantime.