Skip to content

fix(go/adbc/driver/bigquery): Avoid creating arrow iterator when schema is empty #2614

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.

Sign up for GitHub

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

Jump to bottom
Merged
zeroshade merged 3 commits into from
Mar 14, 2025
Merged

fix(go/adbc/driver/bigquery): Avoid creating arrow iterator when schema is empty #2614

zeroshade merged 3 commits into from
Mar 14, 2025

Conversation

@gliga
Copy link
Contributor

@gliga gliga commented Mar 14, 2025 •
edited
Loading

This PR avoids creating arrow iterator in case the resulting schema is empty. The issue was that IsAccelerated (in google-cloud-go/bigquery/storage_iterator.go) would return false in case there is no result (e.g., one is running drop schema) when creating ArrowIterator. Now, before we try to get ArrowIterator, we check if schema is empty.

The PR also adds tests following (i.e., trying to follow) the existing formats.

Closes: #2173

@gliga gliga requested a review from zeroshade as a code owner March 14, 2025 15:22
@github-actions github-actions bot added this to the ADBC Libraries 18 milestone Mar 14, 2025
zeroshade
zeroshade approved these changes Mar 14, 2025
View reviewed changes
Copy link
Member

@zeroshade zeroshade left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks!

@zeroshade
Copy link
Member

zeroshade commented Mar 14, 2025

Looks like some linting and test failures that need to be addressed

@gliga
Copy link
Contributor Author

gliga commented Mar 14, 2025

Looks like some linting and test failures that need to be addressed

Thanks Matt, I will take a look soon.

@gliga
Copy link
Contributor Author

gliga commented Mar 14, 2025

The only issue I saw was related to lint. The other issue (failing snowflake) is unlikely related to this change, unless you think otherwise. Thanks

@gliga
Copy link
Contributor Author

gliga commented Mar 14, 2025

The lint failed in the second run because of the missing license. Fixed it.

But here is something that was interesting to me.
I ran pre commit hook on my own (even for the previous commit) and the only item that was failing was Java (because I had no Java installed). I ignored it as I did not touch any java file. However, once I installed Java and run again, then the license-related item started failing. So the latest push fixed that one.

@zeroshade zeroshade merged commit a1c5c93 into apache:main Mar 14, 2025
38 of 40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zeroshade zeroshade zeroshade approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

ADBC Libraries 18

Development

Successfully merging this pull request may close these issues.

Attempting to drop a Bigquery table through cur.execute throws Error about "storage read API"

2 participants

@gliga @zeroshade