Skip to content

fix(go): fall back to row-based reader when Storage Read isn't accelerated - #6

Open
will-sargent-dbtlabs wants to merge 1 commit into
mainfrom
will/storage-api-fallback
Open

fix(go): fall back to row-based reader when Storage Read isn't accelerated#6
will-sargent-dbtlabs wants to merge 1 commit into
mainfrom
will/storage-api-fallback

Conversation

@will-sargent-dbtlabs

Copy link
Copy Markdown

What

runQuery currently hard-fails a query whenever !iter.IsAccelerated() — i.e. the BigQuery Storage Read API session wasn't available, whether from a permissions gap (readSessionUser) or the Storage API being transiently unavailable. This PR makes that condition fall back to the existing row-based reader (newRowBasedArrowIterator) instead of failing, since the row-based path doesn't require Storage Read at all.

Why

We saw this surface as an intermittent, non-deterministic dbt test failure at a customer (not_null_... failing, then passing 10s later on rerun, no data or code change in between) — tracked publicly at dbt-labs/dbt-core#15463. That issue was closed as Stale for lack of a fully deterministic repro, but the code-level cause is this hard failure on !IsAccelerated().

This repo already has the row-based reader wired in via useLegacyAPI (ported in #5 from the legacy driver), but only as an opt-in decided before the query runs. This PR just OR's the reactive case (!IsAccelerated() at read time) into the same fallback, so a lost/denied Storage session degrades gracefully instead of failing the query.

Related upstream: adbc-drivers/bigquery#66 (no fallback when Storage isn't available) and adbc-drivers/bigquery#171 (adds timeouts + config scaffolding for a future REST fallback, but explicitly defers implementing it). This PR doesn't touch adbc-drivers#171's code paths, just the adjacent !IsAccelerated() branch.

Testing

go build ./... passes. Also added a logger.WarnContext on the fallback path so it's diagnosable in the field rather than silent.

cc @ajhlee-dbt @xuliangs @serramatutu since you've been the ones actively porting/maintaining this file — flagging in case this overlaps with anything you're mid-flight on.

…rated

!iter.IsAccelerated() previously hard-failed the query whenever the
Storage Read API session wasn't available, whether from a permissions
gap or a transient outage. The row-based reader doesn't need Storage
Read, so use it as a fallback instead of failing the read — this is
the same OR-fallback proposed against dbt-labs/dbt-core#15463.
@will-sargent-dbtlabs

will-sargent-dbtlabs commented Aug 5, 2026

Copy link
Copy Markdown
Author

Requesting review before merging — this touches the same file you three have been actively porting/patching, so a second pair of eyes seems worth it.

On CI: the failing checks (Test/*, Validate bigquery latest/linux_amd64, lint & pre-commit) are not caused by this diff:

  • Test/* and Validate bigquery latest/linux_amd64 fail at the google-github-actions/auth step (must specify exactly one of "workload_identity_provider" or "credentials_json") — this is broken the same way on main's own latest CI run right now, unrelated to this PR.
  • lint & pre-commit fails on staticcheck/errcheck findings in python_models.go and statement.go, an Apache RAT license-header gap in PORT_REVIEW.md, and a gofix suggestion on row_based_iterator.go — none of those are files this PR touches.
  • go build ./... passes locally against this branch, and Check PR (the one PR-specific check) is green.

Happy to help chase the pre-existing CI/lint debt separately if useful, but didn't want to scope-creep this PR into fixing it.

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