Skip to content

Apply only the caller's rounding mode in round/3 - #248

Merged
ericmj merged 3 commits into
ericmj:mainfrom
tomciopp:round-mode-fix
Aug 29, 2026
Merged

Apply only the caller's rounding mode in round/3#248
ericmj merged 3 commits into
ericmj:mainfrom
tomciopp:round-mode-fix

Conversation

@tomciopp

Copy link
Copy Markdown
Contributor

Fixes #247 . Implements the minimal fix agreed there: round to places under
mode, with only the result going through the context; no new signalling; and
the exponent limits kept on the input.

The bug

round/3 rounded twice for any value whose coefficient carried more
significant digits than the context precision. The first rounding used
Context.get().rounding, not the mode argument, so the result could violate
the defining property of the mode the caller asked for.

Decimal.Context.set(%Decimal.Context{precision: 3})

Decimal.round(Decimal.new("0.4995"), 1, :down)
#=> Decimal.new("0.5")

The context's :half_up carried 0.4995 to 0.500; :down then truncated
that. Truncating the input gives 0.4.

The context's mode intrudes in both directions, truncating first under
:floor discards the nonzero digits :ceiling needs to see:

Decimal.Context.set(%Decimal.Context{precision: 3, rounding: :floor})

Decimal.round(Decimal.new("0.4001"), 1, :ceiling)
#=> Decimal.new("0.4")

Cause

round/3 normalized its input, and normalize/1 applies the context on both
of its non-zero branches. context/5 rounds the coefficient to ctx.precision
under ctx.rounding, so do_round/5 received a value already rounded under a
mode the caller never asked for.

Reachability

At the default context the parse limit (max_digits: 34) and the precision
(34) coincide, so an over-precision literal never survives Decimal.new/1
it is rejected as a parse error. That check guards only the parsing path though,
and new/3 performs no digit count.

new/3 is the path a database driver takes. Postgrex.Extensions.Numeric
decodes a numeric by accumulating base-10000 digit groups from the binary
wire format and calling Decimal.new(sign, coef, -scale), so nothing consults
max_digits. A Postgres numeric with no type modifier is unconstrained, so a
column value or an AVG(numeric) result can carry far more than 34
significant digits straight into round/3 at the default context:

coef = String.to_integer("4" <> String.duplicate("9", 33) <> "5")

Decimal.round(Decimal.new(1, coef, -35), 1, :down)
#=> Decimal.new("0.5") at the default precision; 0.4 after this change

That is the construction Postgrex produces, and the regression test uses it.

The fix

Drop the precision pass on the input. This is the bug itself: the rounding
that used ctx.rounding in place of mode.

Keep the exponent limits on the input, via a new exponent_limited/2 — the
exponent half of context/5 without the precision half. do_round/5 scales
the coefficient by pow10(exp - target_exp) when exp > target_exp, which is
unbounded for an exponent built through new/3; without the check,
Decimal.round(Decimal.new(1, 1, 10_000_000), 0) would build a
ten-million-digit coefficient only for the context to discard it. Checking
before the zero-strip rather than after is safe: stripping moves exp and the
digit count in opposite directions by the same amount, so the adjusted exponent
is identical either way.

Because that check can itself overflow to Infinity, round/3 keeps a
%Decimal{coef: :inf} case arm. It catches an overflow produced there, not an
Infinity passed in, the function clause above already handles that.

Strip trailing zeros directly with strip_trailing_zeros/2, which is what
normalize/1 contributed besides the context call. This one is an optimization
rather than a correctness requirement. split_digits/3 handles trailing zeros
identically with or without it, but it keeps do_round/5 off needlessly wide
coefficients.

Those three are also sufficient: nothing else between the input and
do_round/5 rounds, so mode is the only rounding round/3 now applies. The
final context/4 call on the result is unchanged and still bounds it to the
precision. That is the same contract every other operation has, and it acts on
the rounded result rather than on the digits mode was deciding about.

Behavior changes

  • Inputs wider than the precision round once, under the caller's mode.

  • round/3 no longer sets :inexact/:rounded for an over-precision input.
    It never signalled for the digits it discards itself, so it now signals for
    neither rather than only in that incidental case, keeping it a
    round-to-integral-value style operation that does not signal for discarded
    digits.

Not a regression

Long-standing, not introduced by #240 or #241. At eb75d13, the base commit
for both, round/3 already called normalize/1 and normalize/1 already
ended in |> context.

Tests

  • Mode isolation across :down, :ceiling and :floor contexts
  • The 35-digit new/3 case at the default precision
  • A bounded-time check that a 10^7 exponent still overflows without
    materializing the coefficient

Existing round/3 tests pass unchanged: they use small values at the default
precision, where the removed pass was already a no-op.

@ericmj ericmj changed the title Apply only the caller's rounding mode in Apply only the caller's rounding mode in round/3 Aug 29, 2026
@ericmj
ericmj merged commit 92a28e6 into ericmj:main Aug 29, 2026
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.

round/3 ignores the caller's rounding mode for values wider than the context precision

2 participants