Skip to content

fix: emit NULL literal for nil values in SelectInlineResults#9336

Merged
begelundmuller merged 5 commits into
mainfrom
jon/bq-nil-parameter
Apr 30, 2026
Merged

fix: emit NULL literal for nil values in SelectInlineResults#9336
begelundmuller merged 5 commits into
mainfrom
jon/bq-nil-parameter

Conversation

@TheRillJon
Copy link
Copy Markdown
Contributor

  • BigQuery's Go client rejects nil in bigquery.QueryParameter.Value
  • When a two-phase comparison aggregation returns NULL dimension values (e.g. acquisition_channel is NULL in some rows), getArgExpr was emitting a ? placeholder with a nil arg
  • Now emits a SQL NULL literal instead, and skips appending nil to the positional args slice

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

Developed in collaboration with Claude Code

BigQuery's Go client rejects nil parameter values in
`bigquery.QueryParameter`. When a metrics aggregation query with
comparison time ranges returns NULL dimension values, `getArgExpr`
was emitting a `?` placeholder with a nil arg, causing
"bigquery: nil parameter" errors.

Emit a SQL `NULL` literal instead and skip appending nil to the
args slice so positional parameter alignment is preserved.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@TheRillJon TheRillJon requested a review from a team as a code owner April 29, 2026 13:43
@k-anshul k-anshul requested a review from begelundmuller April 30, 2026 08:40
Copy link
Copy Markdown
Contributor

@begelundmuller begelundmuller left a comment

Choose a reason for hiding this comment

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

LGTM

@begelundmuller begelundmuller merged commit 14c5b1a into main Apr 30, 2026
10 checks passed
@begelundmuller begelundmuller deleted the jon/bq-nil-parameter branch April 30, 2026 09:20
}

func getArgExpr(val any, typ runtimev1.Type_Code) (string, any, error) {
if val == nil {
Copy link
Copy Markdown
Member

@pjain1 pjain1 Apr 30, 2026

Choose a reason for hiding this comment

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

Just saw this PR and this is not needed for CH as its driver handles null values correctly, same for duckdb.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I did a brief test on CLI and seems to work with this also.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey @pjain1

So just below this line there is this check :

if typ == runtimev1.Type_CODE_DATE {
  t, ok := val.(time.Time)
  if !ok {
	  return "", nil, fmt.Errorf("could not cast value %v to time.Time for date type", val)
  }
  return "toDate(?)", t.Format(time.DateOnly), nil
}

For nil values it will return error: could not cast value nil....
Is there any issue with adding this check ? That will make the below condition safer.

Copy link
Copy Markdown
Member

@pjain1 pjain1 May 1, 2026

Choose a reason for hiding this comment

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

OK got it, no issues with the check.

k-anshul added a commit that referenced this pull request May 1, 2026
* fix: emit NULL literal for nil values in `SelectInlineResults`

BigQuery's Go client rejects nil parameter values in
`bigquery.QueryParameter`. When a metrics aggregation query with
comparison time ranges returns NULL dimension values, `getArgExpr`
was emitting a `?` placeholder with a nil arg, causing
"bigquery: nil parameter" errors.

Emit a SQL `NULL` literal instead and skip appending nil to the
args slice so positional parameter alignment is preserved.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* add tests

* also add check in duckdb and clickhouse

* more extensive test

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Anshul Khandelwal <12948312+k-anshul@users.noreply.github.com>
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.

4 participants