Skip to content

fix(bigquery-driver): Return int64/float64 as strings in query/stream#10738

Merged
ovr merged 4 commits intomasterfrom
fix/bigquery-driver-numeric-string
Apr 24, 2026
Merged

fix(bigquery-driver): Return int64/float64 as strings in query/stream#10738
ovr merged 4 commits intomasterfrom
fix/bigquery-driver-numeric-string

Conversation

@ovr
Copy link
Copy Markdown
Member

@ovr ovr commented Apr 23, 2026

Normalize numeric hydration so integer and floating-point columns are returned as strings, matching the existing NUMERIC/BIGNUMERIC behavior and avoiding JS Number precision pitfalls.

@ovr ovr requested a review from a team as a code owner April 23, 2026 14:49
@github-actions github-actions Bot added driver:bigquery Issues related to the BigQuery driver javascript Pull requests that update Javascript code data source driver labels Apr 23, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 23, 2026

Claude finished @ovr's task in 1m 43s —— View job


Code Review: fix(bigquery-driver): Return int64/float64 as strings

  • Read and understand the diff
  • Review BigQueryDriver.ts changes
  • Review HydrationStream.ts changes
  • Review test changes
  • Post comprehensive review

Overall: This is a clean, well-scoped fix. The PR correctly addresses JavaScript Number precision loss for INT64 and FLOAT64 BigQuery types, aligning them with the existing NUMERIC/BIGNUMERIC string-return behavior.


How it works

The fix has two complementary parts:

  1. wrapIntegers: true in query() and stream() — tells the BigQuery SDK to wrap INT64 values as objects with a .value string property (instead of lossy JS number). The existing check at HydrationStream.ts:18 (value.value && typeof value.value === 'string') already extracts these correctly.

  2. New typeof value === 'number' handler (HydrationStream.ts:22-27) — catches FLOAT64 values (which still arrive as JS number even with wrapIntegers) and converts them to strings via .toString().

This gives complete coverage:

BigQuery Type SDK Representation Handler
NUMERIC / BIGNUMERIC Big object isBig() check
INT64 Wrapped { value: string } value.value check
FLOAT64 JS number New typeof value === 'number' check

Detailed Feedback

HydrationStream.ts — Non-finite number passthrough (line 25-27)

if (typeof value === 'number') {
  if (Number.isFinite(value)) {
    return value.toString();
  }
  return value;  // NaN, Infinity, -Infinity returned as-is
}

The guard for Number.isFinite is defensive, which is fine. In practice BigQuery won't return NaN/Infinity from standard queries, so this branch is unreachable in normal operation — but it's a reasonable safety net. Worth noting that NaN serializes to null in JSON and Infinity throws, so if these ever do appear, downstream consumers will still have issues. Not a blocker, just something to be aware of.

HydrationStream.tsvalue.value check breadth (line 18)

if (value && value.value && typeof value.value === 'string') {
  return value.value;
}

This predates this PR but is worth calling out: this check matches any object with a truthy string .value property, not just BigQuery's wrapped integers. In this context it's safe since the SDK controls the shape, but it's a broad pattern. No action needed.

HydrationStream.ts — Type guard fix (line 7)

The Big.BigBig fix is correct. The big.js library exports Big as both the type and constructor; Big.Big was accessing a nested type that happens to work but is not the canonical form.

BigQueryDriver.tswrapIntegers placement

wrapIntegers: true is correctly added to both query() (line 198) and stream() (line 342). It's correctly absent from loadPreAggregationIntoTable() (which doesn't return result rows) and loadTablesForDataset() (which queries metadata columns only).

Tests

Good coverage:

  • Uses 9223372036854775807 (2^63 - 1) and -9223372036854775808 (-2^63) for INT64 — well beyond Number.MAX_SAFE_INTEGER (2^53 - 1), so these would silently lose precision without this fix
  • Tests both query() and stream() code paths via assertHydrationResults
  • FLOAT64 test values (1265.88, 0.1) verify that regular floating-point numbers are also stringified

One suggestion: it could be valuable to add a test case for FLOAT64('Infinity') or FLOAT64('NaN') to exercise the non-finite branch — but BigQuery actually rejects these literals in standard SQL, so it's not practical to test. Not a blocker.


Summary

No issues found. The change is focused, correct, and well-tested. The numeric type → string normalization is consistent with NUMERIC/BIGNUMERIC behavior and prevents real precision bugs for large integers and floating-point values. LGTM.

@ovr ovr force-pushed the fix/bigquery-driver-numeric-string branch from 6bae1f0 to ebbe153 Compare April 23, 2026 15:49
@ovr ovr requested a review from a team as a code owner April 23, 2026 15:49
@ovr ovr force-pushed the fix/bigquery-driver-numeric-string branch from ebbe153 to 4de2ef3 Compare April 23, 2026 16:28
@ovr ovr force-pushed the fix/bigquery-driver-numeric-string branch from 4de2ef3 to 6d267fa Compare April 23, 2026 16:30
Normalize numeric hydration so integer and floating-point columns are
returned as strings, matching the existing NUMERIC/BIGNUMERIC behavior
and avoiding JS `Number` precision pitfalls.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

❌ Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.03%. Comparing base (c3a5709) to head (e08822a).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
...ages/cubejs-bigquery-driver/src/HydrationStream.ts 50.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10738      +/-   ##
==========================================
+ Coverage   58.02%   58.03%   +0.01%     
==========================================
  Files         215      215              
  Lines       16689    16693       +4     
  Branches     3358     3360       +2     
==========================================
+ Hits         9683     9688       +5     
  Misses       6514     6514              
+ Partials      492      491       -1     
Flag Coverage Δ
cube-backend 58.03% <57.14%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ovr ovr merged commit 15626b1 into master Apr 24, 2026
74 of 76 checks passed
@ovr ovr deleted the fix/bigquery-driver-numeric-string branch April 24, 2026 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

data source driver driver:bigquery Issues related to the BigQuery driver javascript Pull requests that update Javascript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant