Skip to content

feat(bigquery): add QueryResultsFormat and ArrowSerializationOptions configurations - #13942

Draft
jinseopkim0 wants to merge 1 commit into
mainfrom
feat-bigquery-arrow-config
Draft

feat(bigquery): add QueryResultsFormat and ArrowSerializationOptions configurations#13942
jinseopkim0 wants to merge 1 commit into
mainfrom
feat-bigquery-arrow-config

Conversation

@jinseopkim0

Copy link
Copy Markdown
Contributor

Stacked PR 1 of 3: Exposes the public configuration API surface (QueryResultsFormat and ArrowSerializationOptions) and binds them to QueryJobConfiguration.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for Apache Arrow serialization options in BigQuery queries by adding the ArrowSerializationOptions class, the QueryResultsFormat enum, and integrating them into QueryJobConfiguration. However, the new options are currently not mapped in QueryJobConfiguration.toPb() and QueryJobConfiguration.fromPb(), which will cause them to be silently dropped during serialization. It is recommended to implement these mappings to ensure the feature functions correctly.

Comment on lines +267 to +270
// Test toPb/fromPb (not preserved)
QueryJobConfiguration jobFromPb = QueryJobConfiguration.fromPb(job.toPb());
assertNull(jobFromPb.getQueryResultsFormat());
assertNull(jobFromPb.getArrowSerializationOptions());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The new configuration options queryResultsFormat and arrowSerializationOptions are currently not mapped in QueryJobConfiguration.toPb() and QueryJobConfiguration.fromPb(). As a result, these options will be silently dropped when the configuration is serialized and sent to the BigQuery service, making the feature non-functional. Please implement the mapping in QueryJobConfiguration and update this test to assert that the fields are correctly preserved during the toPb/fromPb roundtrip.

Suggested change
// Test toPb/fromPb (not preserved)
QueryJobConfiguration jobFromPb = QueryJobConfiguration.fromPb(job.toPb());
assertNull(jobFromPb.getQueryResultsFormat());
assertNull(jobFromPb.getArrowSerializationOptions());
// Test toPb/fromPb (preserved)
QueryJobConfiguration jobFromPb = QueryJobConfiguration.fromPb(job.toPb());
assertEquals(format, jobFromPb.getQueryResultsFormat());
assertEquals(options, jobFromPb.getArrowSerializationOptions());

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.

1 participant