Skip to content
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

BQ merge fixes #1241

Merged
merged 1 commit into from
Feb 10, 2024
Merged

BQ merge fixes #1241

merged 1 commit into from
Feb 10, 2024

Conversation

serprex
Copy link
Contributor

@serprex serprex commented Feb 9, 2024

BQ it.Next(&row) seems to mutate the target,
https://github.com/googleapis/google-cloud-go/blob/f049c9751415f9fc4c81c1839a8371782cfc016c/bigquery/value.go#L490
so reallocate a fresh row each iteration

ArrayChunks: replace with ArrayIterChunks. Document behavior when input is empty slice
Also reduces allocations, since we've seen people having 1000+ columns

go 1.22 has a rangefunc experiment, so eventually we can replace this with that https://go.dev/wiki/RangefuncExperiment

@serprex serprex force-pushed the array-chunks-edgecase branch 3 times, most recently from 1b4e5e2 to 6968360 Compare February 10, 2024 02:19
BQ `it.Next(&row)` seems to mutate the target,
https://github.com/googleapis/google-cloud-go/blob/f049c9751415f9fc4c81c1839a8371782cfc016c/bigquery/value.go#L490
so reallocate a fresh row each iteration

ArrayChunks: replace with ArrayIterChunks. Document behavior when input is empty slice

This reduces allocations, since we've seen people having 1000+ columns

go 1.22 has a rangefunc experiment, so eventually we can replace this with that
https://go.dev/wiki/RangefuncExperiment
@serprex serprex force-pushed the array-chunks-edgecase branch from 6968360 to 75f15b5 Compare February 10, 2024 02:20
@serprex serprex merged commit d7aef0a into main Feb 10, 2024
7 checks passed
@serprex serprex deleted the array-chunks-edgecase branch February 10, 2024 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants