fix(bigquery): allow timestamp format overrides in createQueryStream#7905
fix(bigquery): allow timestamp format overrides in createQueryStream#7905rafal-hawrylak wants to merge 1 commit intogoogleapis:mainfrom
Conversation
## Description This PR fixes a bug in createQueryStream where formatOptions were being stripped from the query object before reaching the internal request builder. This caused the library to always use the default ISO8601_STRING format, which is rejected by some BigQuery backends. By extracting these options in queryAsStream_ and passing them through to the request builder, users can now successfully opt-out of the new default behavior when necessary. ## Impact - createQueryStream now correctly handles formatOptions.timestampOutputFormat and formatOptions.useInt64Timestamp if provided in the options object. - Bypasses 'timestamp_output_format is not supported yet' errors in restricted environments. ## Testing - Updated unit tests in test/bigquery.ts to verify the 'Gatekeeper' fix. - Added a specific test case to verify that formatOptions are correctly passed through. - Verified that all existing BigQuery tests pass.
product-auto-label
bot
added
the
api: bigquery
danieljbruce
left a comment
danieljbruce
left a comment
There was a problem hiding this comment.
The required change isn't this simple. This removes a desired feature for some of our users. I can take this on if you like.
| delete query.wrapIntegers; | ||
| delete query.parseJSON; | ||
| delete (query as any)['formatOptions.timestampOutputFormat']; | ||
| delete (query as any)['formatOptions.useInt64Timestamp']; |
There was a problem hiding this comment.
We can't just delete timestampOutputFormat and useInt64Timestamp properties from being sent to the backend. Some of our users are likely relying on this feature.
The core issue is that this code was developed on the old repository with a project that had allowlisted a feature that we don't plan to make GA until Q3. So we has to pin the version as shown in #7286. We have a few options:
-
Make source code changes so that requests don't set
timestampOutputFormatby default and then this won't be an issue for users. The tests still won't pass for a project that isn't allowlisted, but it won't affect users. -
We can stage a PR to reverse the changes between 8.1.1 and 8.2.0 so that you can add newer changes and do a release which will unblock future work. Then we can revert this PR which will introduce the changes again when the feature is GA.
Option #2 is probably cleaner. Let me know what you think.
There was a problem hiding this comment.
We can't just delete timestampOutputFormat and useInt64Timestamp properties from being sent to the backend. Some of our users are likely relying on this feature.
Both timestampOutputFormat and useInt64Timestamp are copied to ops and sent to backend via ops, just as other options like location or parseJSON. As far as I understand it's just the query struct itself is stripped from those additional options.
Option #2 is probably cleaner. Let me know what you think.
We do not rely on or expect time formatting changes so for us the most important factor is whichever is the fastest to be released.
There was a problem hiding this comment.
Both timestampOutputFormat and useInt64Timestamp are copied to ops and sent to backend via ops
I see. But if a user calls the query method directly and we release the latest version of Bigquery then we are going to see #7286 come back again. I am working on this on main...revert-bq-changes. I'll try to get a fix out today
danieljbruce
left a comment
danieljbruce
left a comment
There was a problem hiding this comment.
The main issue remains that if we bump the release to the latest version then users will still encounter the issue that they encountered earlier. I'll try to fix this.
| delete query.wrapIntegers; | ||
| delete query.parseJSON; | ||
| delete (query as any)['formatOptions.timestampOutputFormat']; | ||
| delete (query as any)['formatOptions.useInt64Timestamp']; |
There was a problem hiding this comment.
Both timestampOutputFormat and useInt64Timestamp are copied to ops and sent to backend via ops
I see. But if a user calls the query method directly and we release the latest version of Bigquery then we are going to see #7286 come back again. I am working on this on main...revert-bq-changes. I'll try to get a fix out today
|
I actually also see failures when I run the system tests for a non-allowlisted timestamp precision project. How did the system tests perform for your project? 
|
|
#7946 was merged so I think we can close this as it is obselete. |
2 participants
Description
This PR fixes a bug in createQueryStream where formatOptions were being stripped from the query object before reaching the internal request builder. This caused the library to always use the default ISO8601_STRING format, which is rejected by some BigQuery backends.
By extracting these options in queryAsStream_ and passing them through to the request builder, users can now successfully opt-out of the new default behavior when necessary.
Impact
Testing