fix(bigtable): add materialized view routing param to ReadRows and Sa… - #13918
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.
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
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for routing requests targeting materialized views in the Bigtable client. It updates EnhancedBigtableStub to extract instance-level routing parameters when a materialized view name is specified, and adds corresponding integration and header routing tests. The feedback suggests extracting the duplicated routing parameter extraction logic across the different callables in EnhancedBigtableStub into a single helper method to improve maintainability and code readability.
| r.getMaterializedViewName().isEmpty() | ||
| ? composeRequestParams( | ||
| r.getAppProfileId(), r.getTableName(), r.getAuthorizedViewName()) | ||
| : composeInstanceLevelRequestParams( | ||
| MaterializedViewName.parse(r.getMaterializedViewName()) | ||
| .getInstanceName() | ||
| .toString(), | ||
| r.getAppProfileId())) |
There was a problem hiding this comment.
The logic to extract routing parameters for read operations (which can target a table, authorized view, or materialized view) is duplicated across multiple callables. Consider extracting this into a helper method to improve maintainability and readability.
Here is an example of the helper method you can add to the class:
private Map<String, String> composeReadRequestParams(
String appProfileId,
String tableName,
String authorizedViewName,
String materializedViewName) {
if (materializedViewName.isEmpty()) {
return composeRequestParams(appProfileId, tableName, authorizedViewName);
}
return composeInstanceLevelRequestParams(
MaterializedViewName.parse(materializedViewName).getInstanceName().toString(),
appProfileId);
} composeReadRequestParams(
r.getAppProfileId(),
r.getTableName(),
r.getAuthorizedViewName(),
r.getMaterializedViewName())| r.getMaterializedViewName().isEmpty() | ||
| ? composeRequestParams( | ||
| r.getAppProfileId(), r.getTableName(), r.getAuthorizedViewName()) | ||
| : composeInstanceLevelRequestParams( | ||
| MaterializedViewName.parse(r.getMaterializedViewName()) | ||
| .getInstanceName() | ||
| .toString(), | ||
| r.getAppProfileId())) |
| r.getMaterializedViewName().isEmpty() | ||
| ? composeRequestParams( | ||
| r.getAppProfileId(), | ||
| r.getTableName(), | ||
| r.getAuthorizedViewName()) | ||
| : composeInstanceLevelRequestParams( | ||
| MaterializedViewName.parse(r.getMaterializedViewName()) | ||
| .getInstanceName() | ||
| .toString(), | ||
| r.getAppProfileId())) |
🤖 I have created a release *beep* *boop* --- <details><summary>1.89.0</summary> ## [1.89.0](v1.88.0...v1.89.0) (2026-07-29) ### Features * **agentidentity:** onboard v1 and v1beta API versions ([#13796](#13796)) ([1b1300d](1b1300d)) * **auth:** add JSpecify Null annotations to Auth ([#13842](#13842)) ([f6f24d4](f6f24d4)) * **bigquery-jdbc:** add `SSLTrustStoreType` and `SSLTrustStoreProvider` connection properties ([#13858](#13858)) ([9449be1](9449be1)) * **bigquery-jdbc:** add otel trace and span IDs to local logs ([#13935](#13935)) ([2801fbd](2801fbd)) * **bigquery-jdbc:** implement BigQueryParameterMetaData and dynamic type mappings ([#13812](#13812)) ([3d67dba](3d67dba)) * **bigquery-jdbc:** implement parameter setters in PreparedStatement ([#13792](#13792)) ([94f7404](94f7404)) * **bigquery-jdbc:** Migrate `getImportedKeys` and `getCrossReference` to BQ API ([#13692](#13692)) ([082b046](082b046)) * **bigquery-jdbc:** migrate `getPrimaryKeys` to use BQ API ([#13691](#13691)) ([1951f49](1951f49)) * **bigquery-jdbc:** OpenTelemetry integration in BQ JDBC ([#12902](#12902)) ([af18f65](af18f65)) * **bigquery-jdbc:** optimize memory footprint for JSON result set streaming ([#13660](#13660)) ([11f26d3](11f26d3)) * **bigquery-jdbc:** standardize parameter handling and calendar defensive copying across statement interfaces ([#13805](#13805)) ([ccd13eb](ccd13eb)) * **bigtable:** add view_parameters support to BoundStatement ([#13673](#13673)) ([d5cc437](d5cc437)) * **bigtable:** BigtableDataClientFactory session support ([#13829](#13829)) ([284ce13](284ce13)) * **commerceproducer:** onboard v1beta API ([#13814](#13814)) ([61b89b3](61b89b3)) * Default to least-in-flight balancing for Bigtable unary clients ([#13802](#13802)) ([ac9ccd1](ac9ccd1)) * **firestore:** Add support for 16MB documents ([#13478](#13478)) ([1b7c2e0](1b7c2e0)) * **gapic-generator:** add JSpecify Null annotations to the generator classes ([#13769](#13769)) ([843bd7c](843bd7c)) * **gapic-generator:** Add Nullable annotation to generated classes ([#13558](#13558)) ([e3e9d0b](e3e9d0b)) * **gapic-generator:** Add NullMarked annotation to generated classes ([#13584](#13584)) ([b7a8504](b7a8504)) * **gax-httpjson:** Add Post Quantum Cryptography (PQC) Support by default via Conscrypt ([#13853](#13853)) ([550df81](550df81)) * **gax-java:** add JSpecify Null annotations to gax ([#13799](#13799)) ([65aee08](65aee08)) * **google/cloud/sql:** onboard a new library ([#13864](#13864)) ([38e272e](38e272e)) * **google/maps/navconnect/v1:** onboard a new library ([#13927](#13927)) ([2256394](2256394)) * **maps-isochrones:** onboard v1 API ([#13817](#13817)) ([3037ab3](3037ab3)) * port secure_context testing support to executor proxy ([#13522](#13522)) ([0f81bf0](0f81bf0)) * **productregistry:** onboard v1 API ([#13816](#13816)) ([9517313](9517313)) * **storage:** allow checksum on appendable upload finalization ([#13833](#13833)) ([ddf9add](ddf9add)) * **storage:** enable App-Centric Observability (ACO) support in Otel ([#13248](#13248)) ([4329896](4329896)) ### Bug Fixes * **bigquery-jdbc:** Add PerConnectionHandler to list of excempted logging classes ([#13888](#13888)) ([50b3c24](50b3c24)) * **bigquery-jdbc:** add preferIPv4Stack to argLine for Kokoro reliability ([#13923](#13923)) ([d5e33d6](d5e33d6)) * **bigquery-jdbc:** add service resource transformer for standalone IT ([#13893](#13893)) ([dc80fe8](dc80fe8)) * **bigquery-jdbc:** align metadata methods error handling with spec ([#13793](#13793)) ([d85fb10](d85fb10)) * **bigquery-jdbc:** fix WriteAPI when running in restricted environment ([#13856](#13856)) ([66ba925](66ba925)) * **bigquery-jdbc:** refine temporal timezone coercion and PreparedStatement parameter setters ([#13813](#13813)) ([6f68c4d](6f68c4d)) * **bigquery-jdbc:** resolve `ITOpenTelemetryTest` pipeline and trace validation failures ([#13898](#13898)) ([c18141d](c18141d)) * **bigquery-jdbc:** resolve failing otel IT in nightly ([#13915](#13915)) ([ac79713](ac79713)) * **bigquery:** resultSet.getLong() does not truncate for large int64 values ([#13718](#13718)) ([bc19822](bc19822)) * **bigquery:** support optional fields in BigLakeConfiguration to prevent NPE on Iceberg/Lakehouse tables ([#13733](#13733)) ([e2cca4d](e2cca4d)) * **bigtable:** add materialized view routing param to ReadRows and Sa… ([#13918](#13918)) ([4ddf250](4ddf250)) * **bigtable:** bound SessionPoolImpl lock to prevent pod-wide wedge ([#13890](#13890)) ([ed87a68](ed87a68)) * **bigtable:** fix session creation leaks ([#13887](#13887)) ([d586d07](d586d07)) * **bigtable:** prevent ClientConfigurationManagerTest from wedging on… ([#13907](#13907)) ([725086d](725086d)) * **bigtable:** stop installing DirectpathEnforcer on the directpath pool ([#13880](#13880)) ([5f2e056](5f2e056)) * **bom:** make release-note-generation Java 8 compatible ([#13837](#13837)) ([bc18390](bc18390)) * **ci:** fix java-cloud-bom release-notes workflow errors ([#13682](#13682)) ([b679835](b679835)) * deprecate resource detector ([#13844](#13844)) ([aba4f01](aba4f01)) * **deps:** align logback versions and add java8 profile in storage ([#13678](#13678)) ([7e57092](7e57092)) * do not start stream with direct executor ([#13945](#13945)) ([630e790](630e790)) * fix java-cloud-bom README update workflow after monorepo migration ([#13892](#13892)) ([5b8e295](5b8e295)) * **oauth2_http:** Avoid retrying on 4xx errors during GCE metadata ping ([#13715](#13715)) ([537c16c](537c16c)) * regenerate ([#13714](#13714)) ([8a72860](8a72860)) * regenerate libraries ([#13703](#13703)) ([a29ea79](a29ea79)), refs [#13690](#13690) * **release:** handle missing release tags gracefully in release-note-generation ([#13795](#13795)) ([43005fb](43005fb)) * **release:** resolve first-party-dependencies SNAPSHOT in libraries-bom ([#13790](#13790)) ([d5bfe21](d5bfe21)) * **spanner:** avoid data race on DIRECTPATH_CHANNEL_CREATED by using volatile ([#13727](#13727)) ([1e05ea5](1e05ea5)) * **spanner:** prevent fastpath tablet routing flaps ([#13803](#13803)) ([4dabdac](4dabdac)) * **storage:** BidiAppendableUpload Takeover operation fixes ([#13776](#13776)) ([f2d0474](f2d0474)) * **storage:** correctly insert explicit nulls for json patch updates ([#13716](#13716)) ([4fb3f4b](4fb3f4b)) * update group id mapping ([#13698](#13698)) ([50a72e1](50a72e1)) * use a new managed channel builder when creating channels ([#13684](#13684)) ([a049999](a049999)) ### Performance Improvements * **bigquery-jdbc:** optimize getExportedKeys performance using hybrid metadata lookup ([#13734](#13734)) ([c9738ea](c9738ea)) ### Dependencies * Add Conscrypt to shared-deps ([#13838](#13838)) ([05ce6ce](05ce6ce)) * move conscrypt from third-party-dependencies POM to gax-java POM ([#13948](#13948)) ([1e634ec](1e634ec)) * **shared-deps:** migrate awaitility to shared-dependencies ([#13671](#13671)) ([abb91ef](abb91ef)) * **shared-deps:** switch conscrypt shared dependency to conscrypt-openjdk-uber ([#13845](#13845)) ([2e3f208](2e3f208)) * Update gRPC-Java to v1.82.2 ([#13877](#13877)) ([da228d8](da228d8)) * Update http-client to v2.2.0 ([#13854](#13854)) ([334a2c5](334a2c5)) * Update Protobuf-Java to v4.33.6 ([#13876](#13876)) ([5c6478c](5c6478c)) * Upgrade Guava to v33.6.0-jre ([#13875](#13875)) ([41a7a52](41a7a52)) ### Documentation * add ErrorProne and NullAway integration guide and JSpecify migration playbook ([#13882](#13882)) ([d5ea739](d5ea739)) * add JSpecify nullness guidelines to AGENTS.md ([#13881](#13881)) ([54b846b](54b846b)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
…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.