Skip to content

fix(bigquery): totalRows returned by fetchCachedPage#11874

Closed
zecke wants to merge 1 commit intogoogleapis:mainfrom
zecke:zecke-fix-totalrows
Closed

fix(bigquery): totalRows returned by fetchCachedPage#11874
zecke wants to merge 1 commit intogoogleapis:mainfrom
zecke:zecke-fix-totalrows

Conversation

@zecke
Copy link

@zecke zecke commented Mar 19, 2025

The RowIterator.TotalRows is wrong when hitting the cached page on the fast path (e.g. not changing start index or updating the page info).

fixes: #11873
@zecke zecke requested review from a team and Neenu1995 March 19, 2025 14:24
@zecke zecke force-pushed the zecke-fix-totalrows branch from 44cd5f2 to 22dfd2b Compare March 19, 2025 14:25
@zecke
fix(bigquery): Fix totalRows returned by fetchCachedPage
af6b753 
The RowIterator.TotalRows is wrong when hitting the cached page on the
fast path (e.g. not changing start index or updating the page info).

fixes: googleapis#11873
@zecke zecke force-pushed the zecke-fix-totalrows branch from 22dfd2b to af6b753 Compare March 19, 2025 14:26
@alvarowolfx alvarowolfx requested review from alvarowolfx and shollyman and removed request for Neenu1995 March 31, 2025 19:38
@alvarowolfx alvarowolfx changed the title fix(bigquery) Fix totalRows returned by fetchCachedPage Apr 1, 2025
@alvarowolfx alvarowolfx added the api: bigquery Issues related to the BigQuery API. label Apr 7, 2025
alvarowolfx
alvarowolfx requested changes Apr 7, 2025
View reviewed changes
Copy link
Contributor

@alvarowolfx alvarowolfx left a comment

Choose a reason for hiding this comment

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

Overall looks good, but can you add an integration test that shows that issue ? It happens when running two queries in sequence ?
@zecke
Copy link
Author

zecke commented Apr 9, 2025

Overall looks good, but can you add an integration test that shows that issue ? It happens when running two queries in sequence ?

Sure. Do you have any indication of the cost of running the BigQuery integration_test.go?
@alvarowolfx
Copy link
Contributor

alvarowolfx commented Apr 9, 2025

Overall looks good, but can you add an integration test that shows that issue ? It happens when running two queries in sequence ?

Sure. Do you have any indication of the cost of running the BigQuery integration_test.go?

I don't recommended running the full suite, you can run just one test with a command like: go test -run ^TestIntegration_StorageReadClientProject -v. Not sure about the costs of running the full suite, but running just a few tests should be under the free tier.

I can also take the work over and write the tests for it, so I can run the integration tests with my corp account here. Let me know what works best for you.
@zecke
Copy link
Author

zecke commented Apr 12, 2025

I can also take the work over and write the tests for it, so I can run the integration tests with my corp account here. Let me know what works best for you.

Thanks for the offer. Please take this over and write the test/make changes as needed.
@alvarowolfx alvarowolfx mentioned this pull request May 9, 2025
Merged
@alvarowolfx
Copy link
Contributor

alvarowolfx commented May 9, 2025

superseded by #12230
@alvarowolfx alvarowolfx closed this May 9, 2025
gcf-merge-on-green bot pushed a commit that referenced this pull request May 9, 2025
@alvarowolfx
fix(bigquery): cache total rows count (#12230)
202dce0 
Total rows on fast query path was using the amount of rows returned on the first page, which was incorrect for result sets with multiple pages.

Supersedes #11874
Fixes #11873
@release-please release-please bot mentioned this pull request May 9, 2025
Merged
2FaceS-bit pushed a commit to 2FaceS-bit/google-cloud-go that referenced this pull request May 12, 2025
@alvarowolfx @2FaceS-bit
fix(bigquery): cache total rows count (googleapis#12230)
750bf89 
Total rows on fast query path was using the amount of rows returned on the first page, which was incorrect for result sets with multiple pages.

Supersedes googleapis#11874
Fixes googleapis#11873
gcf-merge-on-green bot pushed a commit that referenced this pull request May 12, 2025
@release-please
chore(main): release bigquery 1.68.0 (#11911)
8e7d0b7 
🤖 I have created a release *beep* *boop*
---


## [1.68.0](https://togithub.com/googleapis/google-cloud-go/compare/bigquery/v1.67.0...bigquery/v1.68.0) (2025-05-12)


### Features

* **bigquery/analyticshub:** Support new feature Sharing Cloud Pubsub Streams via AH (GA) and Subscriber Email logging feature ([#11908](https://togithub.com/googleapis/google-cloud-go/issues/11908)) ([a21d596](https://togithub.com/googleapis/google-cloud-go/commit/a21d5965fa3f4322da9563425350ba1079279d5a))
* **bigquery/storage:** Increased the number of partitions can be written in a single request ([43bc515](https://togithub.com/googleapis/google-cloud-go/commit/43bc51591e4ffe7efc76449bb00e3747cda2c944))
* **bigquery:** Add performance insights ([#12101](https://togithub.com/googleapis/google-cloud-go/issues/12101)) ([aef68ab](https://togithub.com/googleapis/google-cloud-go/commit/aef68abaa336e0ecd1f488ef6cb3d6b0e8930835))
* **bigquery:** Add some missing fields to BQ stats  ([#12212](https://togithub.com/googleapis/google-cloud-go/issues/12212)) ([77b08e8](https://togithub.com/googleapis/google-cloud-go/commit/77b08e8e72ece0d56ff8f86dcbfe44b944ab083f))
* **bigquery:** Add WriteTruncateData write disposition ([#12013](https://togithub.com/googleapis/google-cloud-go/issues/12013)) ([b1126a3](https://togithub.com/googleapis/google-cloud-go/commit/b1126a3580a0c81c1d7df7cf138d17c748adefbc))
* **bigquery:** New client(s) ([#12228](https://togithub.com/googleapis/google-cloud-go/issues/12228)) ([f229bd9](https://togithub.com/googleapis/google-cloud-go/commit/f229bd9b90830d96781d3f9059b64dbfece1690b))
* **bigquery:** Support managed iceberg tables ([#11931](https://togithub.com/googleapis/google-cloud-go/issues/11931)) ([35e0774](https://togithub.com/googleapis/google-cloud-go/commit/35e0774bf17166dbaa88eba286f40ad91d9aa68a))
* **bigquery:** Support per-job reservation assignment ([#12078](https://togithub.com/googleapis/google-cloud-go/issues/12078)) ([c9cebcc](https://togithub.com/googleapis/google-cloud-go/commit/c9cebcceebc5fb5eecacf99e18652e0c2a53cc6c))


### Bug Fixes

* **bigquery:** Cache total rows count ([#12230](https://togithub.com/googleapis/google-cloud-go/issues/12230)) ([202dce0](https://togithub.com/googleapis/google-cloud-go/commit/202dce02888c5d1d2821732145d5780e5c07ba05)), refs [#11874](https://togithub.com/googleapis/google-cloud-go/issues/11874) [#11873](https://togithub.com/googleapis/google-cloud-go/issues/11873)
* **bigquery:** Parse timestamps with timezone info ([#11950](https://togithub.com/googleapis/google-cloud-go/issues/11950)) ([530d522](https://togithub.com/googleapis/google-cloud-go/commit/530d522a1f8622e51310680cce31ff1dae007f81))
* **bigquery:** Update google.golang.org/api to 0.229.0 ([3319672](https://togithub.com/googleapis/google-cloud-go/commit/3319672f3dba84a7150772ccb5433e02dab7e201))
* **bigquery:** Upgrade gRPC service registration func ([7c01015](https://togithub.com/googleapis/google-cloud-go/commit/7c01015f2aafb5eeb0237accced76b059bc7635d))


### Documentation

* **bigquery/storage:** Updated the number of partitions (from 100 to 900) can be inserted, updated and deleted in a single request ([43bc515](https://togithub.com/googleapis/google-cloud-go/commit/43bc51591e4ffe7efc76449bb00e3747cda2c944))

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
This was referenced May 20, 2025
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigquery Issues related to the BigQuery API.

2 participants

@zecke @alvarowolfx