fix(jq): round large int literals to 17 decimal digits before f64 arithmetic (#2906) - #2939
Merged
Conversation
CoverageTotal: 93.51% ⚪ 0.02 pp vs Comparing
🔇 0 ignored region(s), 91 tolerated region(s)
Patch coveragePatch: 98.23% (445/453 new lines covered)
Uncovered new lines (8)
|
CoverageTotal: 93.42% ⚪ 0.02 pp vs Comparing
🔇 0 ignored region(s), 92 tolerated region(s)
Patch coveragePatch: 98.23% (445/453 new lines covered)
Uncovered new lines (8)
|
newhoggy
force-pushed
the
issue-2906-jq-large-int-round
branch
from
September 14, 2026 03:37
2026aaf to
107c0de
Compare
…thmetic (#2906) `succinctly jq -n '869389897822472004 + 944331'` printed 869389897823416400 where jq 1.7.1 prints 869389897823416300. #2631 routed past-2^53 results through the f64 model but widened each operand with a bare `i64 as f64`; jq instead converts a *literal* through `jvp_literal_number_to_double` (src/jv.c): `decNumberReduce` under a DECIMAL64 context with `digits = 17` (half-even), then a correctly-rounded strtod -- a double rounding that lands on a different double for some 18/19-digit values. Modelled that way, jq's answer was reproduced on 5600/5600 random differential cases; the earlier "~4% unexplained" figure came from modelling the operands as exact integers. Add `jq_literal_int_to_f64` (identity below 10^17, integer-only rounding above) and an `EvalSemantics::INT_LITERAL_ROUNDS_TO_17_DIGITS` gate, and use it wherever jq mode widens an `Int`: `jq_checked_int_arith`'s fallback, the mixed arms of `+ - * /`, `%` (now jq's `binop_mod` truncate-the-double model, so `869389897822472004 % 1000` is 936 and `9007199254740993 % 2` is 0), `numeric_repr_eq`, and ordering via `compare_values`, which becomes generic over `S` so `sort`/`unique`/`min`/`max`/`bsearch`/`delpaths` inherit the gate. yq mode keeps its exact int64 arithmetic and plain cast. Out of scope, recorded in docs/compliance/jq/limitations.md: >17-digit float and over-i64 literals, the math builtins' own widening, floor-family display past 2^53, and the reindex bridge re-parsing a computed double as an integer literal before `sort`/`unique`/`min`/`max`.
…ning (#2906) Review of the #2906 fix found two regressions in its comparison half. jq's `jvp_number_cmp` has two rules, not one: a literal against a *computed* double widens the literal through the 17-digit rounding, but two *literals* compare exactly as decimals (`decNumberCompare`). Rounding unconditionally made `869389897822472004 == 869389897822471936.0` true (jq: false) and `869389897822472004.0` unequal to its own integer spelling (jq: equal). And `OwnedValue`'s mode-blind `PartialEq` -- which the yq presentation layer uses to align comments across a `|=` -- had inherited the jq rounding, so a yq-mode `Int` no longer matched the plain-cast double yq's own arithmetic produces and comments moved onto the wrong elements. Add `jq_numeric_cmp`: bare `Int` and `NumberLiteral` are literals, bare `Float` is computed; two literals are ordered by their correctly-rounded doubles (monotonic, so a strict order is exact) and only a tie falls through to `cmp_decimal_literals`, an exact digit comparison of the two spellings. jq-mode `==` (`owned_value_eq`) and ordering (`compare_values`) go through it; `PartialEq`/`numeric_repr_eq`/`numeric_repr_cmp` go back to the plain widening, so yq mode and its alignment hash are untouched. As a consequence `9007199254740993 == 9007199254740992.0` is now jq's `false` rather than the recorded divergence. Also from the review: rename the gate to `DECNUMBER_LITERALS`; take `%`'s in-range operands straight to `wrapping_rem` and delegate the rest to `mod_floats` (one copy of `binop_mod`); let `jq_checked_int_arith`'s exact arm use `jq_f64_backed_int`; keep direct `Int`/`Int` and `Float`/`Float` arms in `compare_values`; derive `delete_paths_*`'s yq flag from `S`; correct the `materialize_lazy_keys` comment; pin the three follow-up gaps (#2936, #2937, #2938) with a characterization test; and note jq's `dtoi` UB at 2^63 and the `-x % y` precedence gap it exposes.
Five findings from a high-effort /code-review of the #2906 branch: - jq_numeric_cmp now fast-paths any Int/NumberLiteral(Int) pair to a plain i64 comparison before building a Literal/Side for either operand -- the common case for sort/unique/min/max/bsearch over ordinary parsed-JSON integer arrays, which previously always paid for the full decNumber-literal machinery even though it always resolved to the same x.cmp(&y). - decompose_decimal_literal now reuses strip_leading_sign and parse_literal_exponent/ExpParse instead of a third independent sign-stripping and exponent-saturation implementation, matching the precedent #106/#1304 set for exactly this duplication pattern. - The x86_64 dtoi/INT64_MIN claim in limitations.md and the matching eval.rs comment is reworded as unverified (no x86_64 jq 1.7.1 was available to capture it live), instead of stated as fact. - materialize_lazy_keys's hardcoded compare_values::<JqSemantics> now cites the actual parse-time enforcement (try_parse_builtin's ParserMode::Yq check) it relies on, rather than an unexplained comment; threading S through the whole streaming lazy-materialization pipeline for an already parse-time-enforced invariant wasn't worth the additional plumbing. - arith_mod's float-truncation gate is reordered so its condition is a single negation matching its own comment, instead of needing De Morgan's law to map one to the other.
…rows (#2906) `decompose_decimal_literal` now reuses `parse_literal_exponent`, which saturates an over-long exponent to `i128::MIN`/`MAX`; the fraction and digit-count adjustments around that value then overflowed (a debug-build panic on `1e<45 digits> == 2e<45 digits>`, reachable because both doubles are infinite and the tie falls through to the decimal comparison). Clamp the parsed exponent to +/-2^100 first, and pin three such spellings. Rebasing onto main also picked up #2902, whose reindex bridge now hands a computed float back as a bare `Float` instead of re-parsing its printed digits as an integer literal -- which is exactly the gap #2938 was filed for. Its `sort`/`unique`/`max`/`==` rows now match jq, so they move out of the characterization test into the positive comparison rows, and the limitations entry records #2938 as closed by #2902.
newhoggy
force-pushed
the
issue-2906-jq-large-int-round
branch
from
September 14, 2026 03:51
107c0de to
2f95331
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #2906.
succinctly jq -n '869389897822472004 + 944331'printed869389897823416400; jq 1.7.1 prints869389897823416300. The issue recorded a residual "no model explains ~4% of cases" bucket and shelved it as needing arbitrary-precision decimal arithmetic. It does not: the earlier analysis modelled the operands as exact integers.Root cause (from jq 1.7.1
src/jv.c, not black-box fitting): every parsed number is kept as an exactdecNumberliteral, and the first time arithmetic reads it,jvp_literal_number_to_doublerunsdecNumberReduceunder a DECIMAL64 context withdigits = 17(round-half-even), then a correctly-roundedstrtod. That double rounding lands on a different double thani64 as f64for some 18/19-digit values:869389897822472004→869389897822472000→ nearest double…471936(tie to even), whereas the exact integer's nearest double is…472064. Computed numbers are plain doubles and are never re-rounded. Modelled that way, jq's answer was reproduced on every one of 1200 random+ - * /cases, 200 input-sourced cases (negatives, over-i64), 100 program-literal negatives, and — after the fix — 4148/4150 random differential cases spanning arithmetic, comparisons and the ordering builtins (the two remaining mismatches are >17-digit float literals, filed as #2936).For an
i64the two conversions differ only at|n| >= 10^17(18–19 digits); everything in[2^53, 10^17)was already right.Scope: integers, jq mode only, both evaluators. yq mode's exact
int64arithmetic and plain-cast widening are untouched (pinned by a yq companion test; real yq v4.53.3 gives869389897823416335).Changes
jq_literal_int_to_f64(src/jq/value.rs): the 17-digit half-even rounding with integer arithmetic only (identity below10^17), gated by a newEvalSemantics::DECNUMBER_LITERALS(truefor jq,falsefor yq) throughint_to_f64::<S>.jq_checked_int_arith's fallback and the mixedInt/Floatarms of+ - * /widen through it;%follows jq'sbinop_mod(each operand's double truncated tointmax_t, result held as a double) — in-range operands stay a singlewrapping_rem, the rest delegate tomod_floats.869389897822472004 % 1000is now936(jq) instead of the exact4;9007199254740993 % 2is0.jvp_number_cmp's two rules via a newjq_numeric_cmp: a literal against a computed double widens through the rounding; two literals compare exactly as decimals (decNumberCompare) — doubles first (monotonic, so a strict order is exact), digit comparison only on a tie. jq-mode==(owned_value_eq::<S>) and ordering (compare_values, now generic overS, sosort/unique/group_by/min/max/bsearch/delpathsinherit it) go through it. The mode-blindPartialEqkeeps the old plain widening — the yq presentation layer relies on it for comment alignment (a review round caught that routing it through the jq rounding moved yq comments onto the wrong elements). As a consequence9007199254740993 == 9007199254740992.0is now jq'sfalserather than a recorded divergence.docs/compliance/jq/limitations.md: the jq: large-integer +/- arithmetic can round to a different f64 than real jq even when both operands are non-negative #2906 entry is rewritten as resolved, with the mechanism, what stays open, and two things the fix makes visible without changing (jq'sdtoiUB at exactly2^63on x86_64; the pre-existing-x % yprecedence gap).Review round
/code-review high(8 angles) found two real regressions in the first comparison change — two literals must compare exactly, and the mode-blindPartialEqis what yq's comment alignment relies on — fixed in the second commit (jq_numeric_cmp;PartialEqback on the plain cast). The orchestrator's follow-up commit adds anInt/Intfast path tojq_numeric_cmp, reuses the module's sign/exponent helpers, and rewords the x86_64dtoinote as unverified; the last commit clamps the saturated exponent that reuse introduced (a debug-build panic on1e<45 digits> == 2e<45 digits>) and promotes the #2938 rows to positive expectations, since #2902 (now onmain) makes the bridge hand a computed float back as a bareFloat.Out of scope, filed as follow-ups (pinned by a characterization test)
i64integer literals (same mechanism, needs the mode plumbed through the number-materialisation funnels).as_f64,get_float_value), and floor/ceil/round/trunc print exact ints past 2^53 #2937 — math builtins widen with a bare cast (869389897822472004 | sqrt), andfloor/ceil/round/trunc/lengthprint exact digits past2^53where jq prints the double.jq: reindex bridge re-parses a computed double past 2^53 as an integer literal, so sort/unique/min/max compare it exactly #2938— the reindex bridge re-parsing a computed double as an integer literal beforesort/unique/min/max: closed, resolved by jq/yq: a computed float read back out of a container regains literal spelling in tostring (1.0 vs 1) #2902 landing first (rows now pinned as positive expectations).Test plan
i64::MIN/MAXvectors (bit-exact vs jq), a 20 000-sample seeded property test (string-oracle agreement via{:.16e}, sign symmetry, the "already a double ⇒ no-op" lemma, monotonicity, yq mode bit-identical),cmp_decimal_literals, andjq_numeric_cmpvsowned_value_eq::<JqSemantics>.tests/jq_cli_tests.rs, every expectation captured live from/usr/bin/jq1.7.1): arithmetic incl.%, literal-vs-computed and literal-vs-literal comparisons through==/</sort/unique/bsearch/index/IN/group_by/min, stdin/--argjson/tonumber/fromjsonsources, must-not-change display/small-int/overflow/jq: unary minus in filter text preserves an exact literal where real jq collapses it to a double, masking divergences in the large-integer class #2357 rows, and the follow-up characterization test. yq companion intests/yq_cli_tests.rs(captured from yq v4.53.3).cargo test --features cli --no-fail-fast,--no-default-features, default features;cargo clippy --all-targets --all-features -- -D warnings;cargo fmt --check;RUSTDOCFLAGS=-D warnings cargo doc --no-deps --all-features./code-review high(8 angles) — every finding applied (commits 2–5) or recorded as a follow-up.compare_valuesis now generic overS(same shape as the existingowned_value_eq::<S>), which doubles its monomorphisation; no instruction-count change is expected on the guarded identity rows, but the release profile is codegen-sensitive there — watch that job.