Skip to content

Fix: emit BigQuery NUMERIC / BIGNUMERIC as JSON numbers, not strings#10

Merged
sagargg merged 1 commit into
mainfrom
fix/numeric-as-json-number
Jun 9, 2026
Merged

Fix: emit BigQuery NUMERIC / BIGNUMERIC as JSON numbers, not strings#10
sagargg merged 1 commit into
mainfrom
fix/numeric-as-json-number

Conversation

@sagargg

@sagargg sagargg commented Jun 9, 2026

Copy link
Copy Markdown
Member

Summary

  • BQ NUMERIC / BIGNUMERIC columns come back as decimal.Decimal. The _json_default and _orjson_default callbacks were stringifying them, so a response carrying price = Decimal("5336.69") was emitted as "price": "5336.69" rather than "price": 5336.69. Clients had to special-case parsing those numeric fields.
  • Switch both callbacks to float(obj) so orjson emits a JSON number. Comment explains the trade-off (values past ~15 significant digits round to the nearest IEEE-754 double); callers needing the full precision should CAST(... AS STRING) in datastore_search_sql.
  • Integer (BQ INT64 -> int) and FLOAT64 paths were already correct — only the Decimal branch changed.

Test plan

  • pytest tests/test_streaming.py — three streaming tests green.
  • pytest tests/ — full 333-test suite green.
  • tests/test_streaming.py now asserts isinstance(record_value, float) explicitly so a regression to strings fails loudly.

Summary by CodeRabbit

Bug Fixes

  • Updated JSON serialization behavior for decimal numeric values across API responses and streaming operations. Decimal values are now correctly emitted as JSON numbers instead of strings, improving standard JSON compliance and compatibility with client libraries and JSON parsers that expect numeric data types rather than string representations.

BQ `NUMERIC` / `BIGNUMERIC` come back as `decimal.Decimal`; the existing
`_json_default` (and `_orjson_default`) stringified them, so clients saw
`"price": "5336.69"` instead of `"price": 5336.69`. Integers and FLOAT64
were already correct — only the Decimal path was off.

Convert Decimal -> float in both default callbacks so orjson emits a
JSON number. Trade-off documented inline: values past ~15 significant
digits round to nearest IEEE-754 double, and callers needing full
precision should `CAST(... AS STRING)` in `datastore_search_sql`.

Tests updated to pin the JSON-number contract explicitly via
`isinstance(..., float)` so a regression to strings fails loudly.
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0e383274-c6a3-43c9-abba-24cb6b9be27f

📥 Commits

Reviewing files that changed from the base of the PR and between aa64e30 and cd7bde0.

📒 Files selected for processing (3)
  • datastore/api/responses.py
  • datastore/services/streaming.py
  • tests/test_streaming.py

📝 Walkthrough

Walkthrough

The PR updates Decimal JSON serialization across the datastore from string representation to numeric representation. API response and streaming serialization functions now convert Decimal to float for JSON emission, with updated docstrings documenting precision trade-offs. Corresponding tests validate the numeric JSON output.

Changes

Decimal JSON Serialization

Layer / File(s) Summary
API response Decimal serialization
datastore/api/responses.py
_orjson_default converts Decimal to JSON number via float(obj) with inline comments clarifying numeric output and precision/rounding behavior.
Streaming Decimal serialization
datastore/services/streaming.py
_json_default emits Decimal as JSON numbers instead of strings, with docstring updated to describe IEEE-754 double-precision implications of the conversion.
Test validation of numeric Decimal JSON
tests/test_streaming.py
Test expectations and assertions changed to validate Decimal as JSON numbers: docstrings clarified, numeric value assertions added, and explicit isinstance(..., float) checks added to pin the type contract.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • datopian/datastore#8: Prior PR modifying Decimal serialization in the same files, changing _orjson_default and _json_default serialization behavior alongside corresponding streaming tests.

Poem

🐰 A decimal hops through JSON streams,
No longer bound in string-enclosed dreams,
Now float it rides, precise enough to fly,
IEEE-754 dancing through the sky!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/numeric-as-json-number

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sagargg sagargg merged commit 12e124d into main Jun 9, 2026
1 of 2 checks passed
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