Skip to content

Fix double rounding of number literals with more than 17 digits - #3695

Open
NgoQuocViet2001 wants to merge 1 commit into
jqlang:masterfrom
NgoQuocViet2001:fix-decnum-double-rounding
Open

NgoQuocViet2001 wants to merge 1 commit into
jqlang:masterfrom
NgoQuocViet2001:fix-decnum-double-rounding

Conversation

@NgoQuocViet2001

Copy link
Copy Markdown

Converting a number literal to a double first rounds it to 17 significant digits with decNumberReduce(), then passes that string to jvp_strtod(). When the 17-digit value lands on or past the midpoint between two doubles, the second rounding goes the wrong way:

$ jq -n '100000000000000041 * 1'
100000000000000030    # before
100000000000000050    # after

The nearest double is 100000000000000048. Rounding to 17 digits first gives 100000000000000040, an exact midpoint, and ties-to-even then picks 100000000000000032. A --disable-decnum build already prints 100000000000000050. The 17-digit step came from #2949, where it was meant to avoid allocating for digits that cannot change the result, but near a midpoint they can.

This passes every digit of the literal to jvp_strtod(), which rounds correctly. Literals with up to 17 digits still use the stack buffer; longer ones use a temporary heap buffer of digits + 15 bytes, the same size jvp_literal_number_literal() allocates. The unused decNumberDoublePrecision type and DEC_NUMBER_DOUBLE_EXTRA_UNITS are removed.

The new case in tests/jq.test fails before the change and passes with and without decNumber.

Ran (Ubuntu 24.04, gcc 13.3):

  • make check: 9/9 suites pass, tests/jq.test 599/599. Without the fix it is 598/599, with all four new values wrong.
  • make check with --disable-decnum: 9/9 pass.
  • make check with --enable-asan --enable-ubsan: 9/9 pass, no new sanitizer reports.

Checked:

  • 4,500 random literals with more than 17 significant digits, about a quarter of them next to a midpoint, against Python's float(): 680 wrong before, 0 after.
  • -0, 1E+1000, -1E+1000, 1E-1000, subnormals, values near DBL_MAX and 1e999999999 give the same results as before.
  • A literal with 1,000,000 digits still converts in under 0.1s.

Converting a decNumber literal to a double first rounded it to 17
significant digits and then passed that string to strtod. When the
17-digit result lands on or past the midpoint between two doubles, the
second rounding picks the farther one, so 100000000000000041 * 1 gave
100000000000000030 instead of 100000000000000050. The --disable-decnum
build already returns the nearest double.

Pass every digit of the literal to strtod, which rounds correctly, and
drop the now unused 17-digit decNumber type.

This branch has not been deployed

No deployments
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