Add biquery query parameter option to support parametrize query#39389
Add biquery query parameter option to support parametrize query#39389i05nagai wants to merge 1 commit into
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enables parameterized queries for BigQueryIO in the Go SDK. By allowing users to pass query parameters, it improves security and flexibility when interacting with BigQuery. The changes include the necessary infrastructure to serialize these parameters for execution within the Beam pipeline, alongside updates to integration test utilities to better support project-specific configurations. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
i05nagai
changed the title
![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 parameterized queries in the Go SDK's BigQuery IO connector, adding a new WithQueryParameters option, serializing parameters during pipeline execution, and updating integration tests. The reviewer feedback is highly constructive and identifies critical issues: JSON serialization will cause type loss for query parameters (e.g., converting integers to floats), which can be resolved by using gob encoding; decoding will fail on empty parameters without a guard check; tests should be expanded to cover numeric types; and encoding should be skipped when no parameters are provided to avoid overhead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| import ( | ||
| "encoding/json" | ||
| "fmt" | ||
|
|
||
| "cloud.google.com/go/bigquery" | ||
| ) | ||
|
|
||
| func encodeQueryParameters(params []bigquery.QueryParameter) ([]byte, error) { | ||
| out, err := json.Marshal(params) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("error encoding JSON: %w", err) | ||
| } | ||
|
|
||
| return out, nil | ||
| } | ||
|
|
||
| func decodeQueryParameters(bytes []byte) ([]bigquery.QueryParameter, error) { | ||
| var out []bigquery.QueryParameter | ||
| if err := json.Unmarshal(bytes, &out); err != nil { | ||
| return nil, fmt.Errorf("error decoding JSON: %w", err) | ||
| } | ||
|
|
||
| return out, nil | ||
| } |
There was a problem hiding this comment.
Using json.Marshal and json.Unmarshal for bigquery.QueryParameter will cause type loss for the Value interface{} field. For example, any integer value (like int or int64) will be unmarshaled as a float64, which will cause BigQuery to fail with type mismatch errors (e.g., when comparing an INT64 column with a FLOAT64 parameter). Similarly, time.Time or civil.Date will be unmarshaled as string, which may also cause query failures.
Consider using gob encoding instead, and registering the common types (like time.Time, civil.Date, civil.DateTime, and civil.Time) using gob.Register so that type fidelity is preserved across the serialization boundary.
import (
"bytes"
"encoding/gob"
"fmt"
"time"
"cloud.google.com/go/bigquery"
"cloud.google.com/go/civil"
)
func init() {
gob.Register(time.Time{})
gob.Register(civil.Date{})
gob.Register(civil.DateTime{})
gob.Register(civil.Time{})
}
func encodeQueryParameters(params []bigquery.QueryParameter) ([]byte, error) {
var buf bytes.Buffer
enc := gob.NewEncoder(&buf)
if err := enc.Encode(params); err != nil {
return nil, fmt.Errorf("error encoding gob: %w", err)
}
return buf.Bytes(), nil
}
func decodeQueryParameters(b []byte) ([]bigquery.QueryParameter, error) {
var out []bigquery.QueryParameter
dec := gob.NewDecoder(bytes.NewReader(b))
if err := dec.Decode(&out); err != nil {
return nil, fmt.Errorf("error decoding gob: %w", err)
}
return out, nil
}| parameters, err := decodeQueryParameters(f.QueryParameters) | ||
| if err != nil { | ||
| return errors.Wrapf(err, "bigqueryio.queryFn: failed to decode query parameters") | ||
| } | ||
| q.Parameters = parameters |
There was a problem hiding this comment.
If f.QueryParameters is empty or nil (which is the case when no parameters are set, or for backward compatibility with older serialized pipelines), calling decodeQueryParameters directly will fail with an error (e.g., unexpected end of JSON input or EOF with gob). We should guard this call and only decode if f.QueryParameters is not empty.
| parameters, err := decodeQueryParameters(f.QueryParameters) | |
| if err != nil { | |
| return errors.Wrapf(err, "bigqueryio.queryFn: failed to decode query parameters") | |
| } | |
| q.Parameters = parameters | |
| if len(f.QueryParameters) > 0 { | |
| parameters, err := decodeQueryParameters(f.QueryParameters) | |
| if err != nil { | |
| return errors.Wrapf(err, "bigqueryio.queryFn: failed to decode query parameters") | |
| } | |
| q.Parameters = parameters | |
| } |
| tests := []struct { | ||
| name string | ||
| val bigquery.QueryParameter | ||
| }{ | ||
| { | ||
| name: "Encode/decode QueryOptions with parameters", | ||
| val: bigquery.QueryParameter{ | ||
| Name: "key", | ||
| Value: "value", | ||
| }, | ||
| }, | ||
| { | ||
| name: "Encode/decode QueryOptions with nil parameter", | ||
| val: bigquery.QueryParameter{ | ||
| Name: "key", | ||
| Value: nil, | ||
| }, | ||
| }, | ||
| } |
There was a problem hiding this comment.
The current test cases only cover string and nil values, which hides the type loss issue of JSON serialization. Please add test cases for int and float64 values to ensure type preservation.
tests := []struct {
name string
val bigquery.QueryParameter
}{
{
name: "Encode/decode QueryOptions with string parameter",
val: bigquery.QueryParameter{
Name: "key",
Value: "value",
},
},
{
name: "Encode/decode QueryOptions with int parameter",
val: bigquery.QueryParameter{
Name: "key",
Value: 42,
},
},
{
name: "Encode/decode QueryOptions with float parameter",
val: bigquery.QueryParameter{
Name: "key",
Value: 3.14,
},
},
{
name: "Encode/decode QueryOptions with nil parameter",
val: bigquery.QueryParameter{
Name: "key",
Value: nil,
},
},
}| queryParameters, err := encodeQueryParameters(queryOptions.parameters) | ||
| if err != nil { | ||
| panic(errors.Wrapf(err, "bigqueryio.Query: failed to encode query parameters")) | ||
| } |
There was a problem hiding this comment.
To avoid unnecessary serialization overhead when no query parameters are provided, we should only encode the parameters if the slice is not empty.
var queryParameters []byte
if len(queryOptions.parameters) > 0 {
var err error
queryParameters, err = encodeQueryParameters(queryOptions.parameters)
if err != nil {
panic(errors.Wrapf(err, "bigqueryio.Query: failed to encode query parameters"))
}
}
|
Assigning reviewers: R: @lostluck for label go. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
1 participant
Addresses: #29382
Also addresses: #25096
Description
This PR supports QueryParameter for BigQueryIO in Go.
Since QueryParameter includes
interface{}field, I followed the same implementation pattern in mongodb package by adding encoded QueryParameters to queryFn.Tests
I successfully ran integrated tests with
I am not sure if I should add integration tests for my change or I should make integration tests runnable with dataflow runner.
If it's required, I'll update my PR.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.