fix(bigtable): add materialized view routing param to ReadRows and Sa… - #13918
fix(bigtable): add materialized view routing param to ReadRows and Sa…#13918mutianf wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for routing read requests (ReadRows and SampleRowKeys) targeting materialized views in Cloud Bigtable. Since materialized views are instance-scoped, they route on the instance name instead of the table name. The changes introduce helper methods to extract the instance name, update the stub to handle this routing logic, and add corresponding tests. The feedback suggests omitting the app_profile_id parameter from the routing headers when it is empty to maintain consistency with write requests and avoid sending empty parameters to the server.
| if (!materializedViewName.isEmpty()) { | ||
| return ImmutableMap.of( | ||
| "name", | ||
| NameUtil.extractInstanceNameFromMaterializedViewName(materializedViewName), | ||
| "app_profile_id", | ||
| appProfileId); | ||
| } |
There was a problem hiding this comment.
If appProfileId is empty, ImmutableMap.of will still include it as an empty string (i.e., "app_profile_id" -> ""). In contrast, composeWriteRequestParams omits the app_profile_id key entirely when it is empty. To maintain consistency and avoid sending an empty routing parameter to the server, we should omit app_profile_id when it is empty.
| if (!materializedViewName.isEmpty()) { | |
| return ImmutableMap.of( | |
| "name", | |
| NameUtil.extractInstanceNameFromMaterializedViewName(materializedViewName), | |
| "app_profile_id", | |
| appProfileId); | |
| } | |
| if (!materializedViewName.isEmpty()) { | |
| String instanceName = | |
| NameUtil.extractInstanceNameFromMaterializedViewName(materializedViewName); | |
| if (appProfileId.isEmpty()) { | |
| return ImmutableMap.of("name", instanceName); | |
| } | |
| return ImmutableMap.of("name", instanceName, "app_profile_id", appProfileId); | |
| } |
f9be7f9 to
f4c6c3d
Compare
…mpleRowKeys ReadRows and SampleRowKeys can target a materialized view, but the client's request-params extractor only emitted table_name + app_profile_id and ignored materialized_view_name. Per the google.api.routing annotation in bigtable.proto, materialized views are instance-scoped and must route on the instance name. Split the header composition into composeReadRequestParams (materialized-view capable) and composeWriteRequestParams (table/authorized-view only), and add NameUtil.extractInstanceNameFromMaterializedViewName. Add HeadersTest coverage asserting the instance-name routing param for both operations, and a ReadIT integration test that reads through a materialized view.
f4c6c3d to
eda51af
Compare
…mpleRowKeys
ReadRows and SampleRowKeys can target a materialized view, but the client's request-params extractor only emitted table_name + app_profile_id and ignored materialized_view_name. Per the google.api.routing annotation in bigtable.proto, materialized views are instance-scoped and must route on the instance name.
Split the header composition into composeReadRequestParams (materialized-view capable) and composeWriteRequestParams (table/authorized-view only), and add NameUtil.extractInstanceNameFromMaterializedViewName.
Add HeadersTest coverage asserting the instance-name routing param for both operations, and a ReadIT integration test that reads through a materialized view.