-
Notifications
You must be signed in to change notification settings - Fork 310
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: use BigQuery Storage client by default #55
Conversation
25572ab
to
1bb7283
Compare
I did some timings to compare fetching data with and without the BQ Storage client, results below. Spoiler: When using the BQ Storage API, performance improvements are enormous. Query
Source tables
Timings
|
fde9441
to
c01648d
Compare
f080813
to
1aa0c62
Compare
1aa0c62
to
3f73a43
Compare
3f73a43
to
00bf584
Compare
12b2171
to
dbef14d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Semantic satiation seeing all the v1/v1beta references. Sorry it took me so long to get to this. Couple minor nits, with a couple more interesting bits (avro vs arrow, small result set, versioning for storage).
5b82c76
to
c715a74
Compare
* feat: add HOUR support for time partitioning interval
486309f
to
6e35d09
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the slogging through this.
Just a thought - since this is a pretty significant change, how about releasing all the other smaller changes and fixes that have accumulated since the last release separately, and release this one separately for an easier rollback, should that be necessary? |
Isolating this change to its own release seems prudent. |
Putting this on hold until the next release is made, we will ship it separately. |
With the new release (1.25.0) now out, we can now merge this and release in the near future (possibly with miscellaneous related cleanup fixes). |
Closes #86.
Closes #84.
No issue yet, just a POC preview.The PR is now ready to be reviewed. It uses the BQ Storage API by default to fetch query results, and removes the "beta" label from that feature.
It's probably easier to review this PR commit by commit.
Key points: