Skip to content

fix(query): print wide decimals at full precision - #299

Merged
anoop-narang merged 2 commits into
mainfrom
fix/decimal-precision
Sep 10, 2026
Merged

fix(query): print wide decimals at full precision#299
anoop-narang merged 2 commits into
mainfrom
fix/decimal-precision

Conversation

@anoop-narang

Copy link
Copy Markdown
Contributor

The bug

A DECIMAL wide enough to need more than ~17 significant digits printed as a rounded float, in every output format, with no error and no exit-code change:

$ hotdata query -d $DB -o csv \
    "SELECT CAST('99999999999999999999.99' AS DECIMAL(38,2)) AS money,
            CAST(CAST('99999999999999999999.99' AS DECIMAL(38,2)) AS VARCHAR) AS exact"
money,exact
1e+20,99999999999999999999.99

The exact column is the same value cast to text, in the same row — proof the digits reached the CLI intact. The HTTP API returns 99999999999999999999.99 for the same query. So the platform was right and the screen was wrong, and checking against the API made it more confusing rather than less.

DECIMAL is where money lives, so this printed wrong money into -o csv exports and -o json pipelines.

Cause

Two independent losses, both client-side.

  1. Inline path. Rows were deserialized into serde_json::Value, which has no arbitrary-precision number variant — anything that does not fit an i64/u64 goes through f64. The digits were gone before the CLI saw them.
  2. Arrow path. The CLI held exact Arrow Decimal128 values, rendered each to JSON text, then parsed that text back into a Value — reintroducing the same loss on data it already had right.

Integers were unaffected (Value has real i64/u64 variants). In practice this is DECIMAL.

Fix

Rows carry hotdata::JsonCell (new in 0.17.0), which holds the cell's JSON text. Nothing is decided at parse time, so a number prints with the digits the service sent. -o json still emits it unquoted — it stays a JSON number, not a string, so jq pipelines are unaffected.

While translating the render paths, three unwrap_or_default() fallbacks were replaced with expect. kind() has already identified the cell before the accessor is called, so None is unreachable — and substituting an empty string or empty list would put a value on screen the service never sent, which is the same defect this PR exists to remove.

One visible change

A list or struct cell now prints on a single line in -o json:

-       [                    +       [1,2,3],
-         1,
-         2,
-         3
-       ],

Scalars are unaffected, and the JSON is identical to any parser. Re-indenting would require parsing the cell text, which is the step that loses the digits. Pinned by json_output_writes_a_composite_cell_on_one_line so it is a documented fact rather than a surprise.

Verification

Manual, against a local server, with the fixed binary:

-o csv     99999999999999999999.99,99999999999999999999.99
-o table   │ 99999999999999999999.99 │ 99999999999999999999.99 │
-o json    money: value=99999999999999999999.99 type=number   (via jq)
           list : value=[1,2,3]                 type=array
           null : value=null                    type=null

money matches the exact text cast in all three, and the raw API body agrees.

Both loss sites are covered by tests: the inline path, and the Arrow path via a truncated 200k-row result followed through /results/{id}. The previous a_wide_decimal_is_rounded_a_known_limitation, which pinned the lossy behaviour, is replaced by a_wide_decimal_keeps_every_digit.

582 tests pass against the published 0.17.0. clippy clean, fmt clean.

A DECIMAL needing more than ~17 significant digits printed as a rounded
float: `SELECT CAST('99999999999999999999.99' AS DECIMAL(38,2))` came
out as `1e20` in every output format, while the same query against the
HTTP API returned every digit. Wrong money on screen, with no error and
no exit-code change.

Two independent losses, both on this side. Result rows arrived already
rounded, because they were deserialized into `serde_json::Value`, which
has no arbitrary-precision number. And the Arrow path re-introduced the
same loss on data it held exactly, by rendering each cell to JSON text
and parsing it back into a `Value`.

Rows now carry `hotdata::JsonCell` (0.17.0), which holds a cell's JSON
text, so nothing is decided at parse time and a number is printed with
the digits the service sent. `-o json` still emits it unquoted, so the
output stays a JSON number rather than becoming a string.

`kind()` identifies each cell before the accessors are called, so the
accessors cannot decline; they now panic on that unreachable state
rather than substituting an empty string or list, which would put a
value on screen that the service never sent.

One visible change: a list or struct cell prints on a single line in
`-o json`. Re-indenting it would mean parsing the text, which is what
loses the digits. Scalars are unaffected and the JSON is unchanged to
any parser.
@anoop-narang
anoop-narang requested a review from a team as a code owner September 10, 2026 13:46
@anoop-narang
anoop-narang requested review from zfarrell and removed request for a team September 10, 2026 13:46
Comment thread src/commands/query.rs Outdated
Comment thread src/output/table.rs
claude[bot]
claude Bot previously approved these changes Sep 10, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed the full diff plus the surrounding code in src/commands/query.rs and src/output/table.rs. Both loss sites are fixed and covered by tests. Two non-blocking inline comments.

Checked and found correct:

  • -o csv, -o table and -o json all read the cell text, so no render path re-parses through serde_json::Value.
  • The composite-on-one-line change does not affect the integration tests. tests/results_arrow.rs and tests/query_async_polling.rs compare parsed values, not raw text.
  • The RawValue private-struct hazard described for arbitrary_precision does not reach -o yaml. print_result accepts only json, csv and table, and JsonCell appears in no model that workspace.rs, jobs.rs or results.rs serializes to YAML.
  • CHANGELOG needs no manual edit. Entries are generated from the conventional commit subject, and CI / changelog passed.

CI / test was still queued at review time, so the test results are unverified here.

@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 14 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/output/table.rs 73.33% 8 Missing ⚠️
src/commands/query.rs 94.54% 6 Missing ⚠️

📢 Thoughts on this report? Let us know!

`truncate_array` formats the short and head/tail branches separately,
so a regression could land on one alone; assert a wide decimal survives
both, and that the elision counts every element.

The `encode_cell` doc said a wide result does not allocate per value.
It copies the cell's text out of the shared buffer, so it allocates one
String per cell; only the encode buffer is reused.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Prior nits addressed in 0a5950a. The encode_cell doc comment now matches the code. truncate_array_keeps_element_precision_on_both_branches covers both formatting branches, including the 7-element case.

No new findings. CI was still queued or in progress at review time, so this approval does not assert the test result.

@anoop-narang
anoop-narang merged commit 9d529da into main Sep 10, 2026
14 checks passed
@anoop-narang
anoop-narang deleted the fix/decimal-precision branch September 10, 2026 13:59
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