fix(go/adbc/driver/bigquery): Use number of rows (rather than schema) to check if we need an empty arrow iterator #2674
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…mpty arrow iterator
felipecrv
changed the title
felipecrv
approved these changes
Apr 7, 2025
Contributor
|
I like this fix much more than the previous one -- empty record batch readers may have a schema. I think the BigQuery Go SDK assumes that the caller knows that the SQL being passed is not going to produce a result set which is not the case from the ADBC driver code just forwarding SQL it receives. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As the newly added tests shows, we might end up with a schema (
create view) and yet no rows in which case we would still have an issue #2173This PR changes so we check if the number of rows is empty (and keep the schema).
Note: The potential issue with using Schema vs. TotalRows is the same (as the comment says for both of the fields they might not be initialized in some cases before the first call to Next).