feat(bigquery): add QueryResultsFormat and ArrowSerializationOptions configurations - #13942
feat(bigquery): add QueryResultsFormat and ArrowSerializationOptions configurations#13942jinseopkim0 wants to merge 9 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
4f0d80d to
d43166d
Compare
d43166d to
b441a9d
Compare
…rrowSerializationOptionsConverter
… Arrow configuration classes
…ons.Builder and clarify Javadocs
| private @Nullable String bufferCompression; | ||
| private @Nullable String picosTimestampPrecision; |
There was a problem hiding this comment.
Do you think it's possible we can set a default value for these so they they can be non-null and we can assert that they always will be non-null? especially in the setters?
e.g. uncompressed and micro precision?
| public Builder setBufferCompression(String bufferCompression) { | ||
| this.bufferCompression = checkNotNull(bufferCompression, "bufferCompression cannot be null"); | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * <b>[Beta]</b> Sets the timestamp precision for Arrow timestamp types. | ||
| * | ||
| * <p>Note: Only applies when {@link QueryResultsFormat#ARROW} is enabled. For Arrow result | ||
| * streams, this precision setting governs binary Arrow timestamp column types and takes | ||
| * precedence over {@link DataFormatOptions.TimestampFormatOptions}, which applies to default | ||
| * {@link QueryResultsFormat#STRUCT_ENCODING} JSON results. | ||
| */ | ||
| @BetaApi | ||
| public Builder setPicosTimestampPrecision(String picosTimestampPrecision) { |
There was a problem hiding this comment.
For these setters, do you think we should create enum wrappers around the options? If there is a typo in the precision on compression codec, it may fail with the apiary model
| if (options == null) { | ||
| return null; | ||
| } | ||
| com.google.api.services.bigquery.model.ArrowSerializationOptions optionsPb = | ||
| new com.google.api.services.bigquery.model.ArrowSerializationOptions(); | ||
| if (options.getBufferCompression() != null) { | ||
| optionsPb.setBufferCompression(options.getBufferCompression()); | ||
| } | ||
| if (options.getPicosTimestampPrecision() != null) { | ||
| optionsPb.setPicosTimestampPrecision(options.getPicosTimestampPrecision()); |
There was a problem hiding this comment.
Follow the above, do you think we can just assert that the params are non-null?
Also if we set a default value for ArrowSerializationOptions's params, then I don't think we need the null checks here
| */ | ||
| @BetaApi | ||
| public Builder setQueryResultsFormat(QueryResultsFormat queryResultsFormat) { | ||
| this.queryResultsFormat = queryResultsFormat; |
There was a problem hiding this comment.
nit: perhaps non-null checks here? We should be able to assert that users must pass valid values here
2 participants
Stacked PR 1 of 3: Exposes the public configuration API surface (
QueryResultsFormatandArrowSerializationOptions) and binds them to QueryJobConfiguration.Note: The new classes and methods are annotated with
@BetaApito indicate that the API surface is experimental while implementation PRs (PR 2 of 3 and PR 3 of 3) are merged. The@BetaApiannotation will be removed upon completion of the final PR in the stack.b/540476814