diff --git a/src/expr/src/interpret.rs b/src/expr/src/interpret.rs index 0ea5390de5413..5bba046327695 100644 --- a/src/expr/src/interpret.rs +++ b/src/expr/src/interpret.rs @@ -3360,6 +3360,48 @@ mod tests { ); } + /// `round` must not depend on a numeric's exponent: `Row` encoding folds + /// trailing zeroes into it, so the interpreter, which reads its datums back + /// out of a `Row`, would otherwise disagree with the evaluator and pushdown + /// could discard a part it has to keep. + #[mz_ore::test] + #[cfg_attr(miri, ignore)] + fn test_round_numeric_representation_independent() { + use mz_repr::adt::date::Date; + + let arena = RowArena::new(); + let lit = |d: Datum, ty: ReprScalarType| { + let mut row = Row::default(); + row.packer().push(d); + MirScalarExpr::Literal(Ok(row), ty.nullable(false)) + }; + + // `extract` hands `round` a `946684800` whose exponent is zero, where + // the same value read out of a `Row` is `9.466848E+8`. + let expr = lit( + Datum::Date(Date::from_pg_epoch(0).unwrap()), + ReprScalarType::Date, + ) + .call_unary(UnaryFunc::ExtractDate(ExtractDate(DateTimeUnits::Epoch))) + .call_binary( + lit(Datum::Int32(i32::MAX), ReprScalarType::Int32), + BinaryFunc::from(RoundNumericBinary), + ); + + let relation = ReprRelationType::new(vec![]); + let range = ColumnSpecs::new(&relation, &arena).expr(&expr).range; + match expr.eval(&[], &arena) { + Ok(value) => assert!( + range.may_contain(value), + "interpreter ruled out {value:?}, which the evaluator produced: {range:?}", + ), + Err(_) => assert!( + range.may_fail(), + "interpreter ruled out the error the evaluator produced: {range:?}", + ), + } + } + #[mz_ore::test] fn test_trace() { use super::Trace; diff --git a/src/expr/src/scalar/func.rs b/src/expr/src/scalar/func.rs index 9dd82c1392921..fde6bd7a99086 100644 --- a/src/expr/src/scalar/func.rs +++ b/src/expr/src/scalar/func.rs @@ -481,11 +481,22 @@ fn round_numeric_binary(a: OrderedDecimal, mut b: i32) -> Result 0 && b > 0 || a_exp < 0 && -a_exp < b { + if a.is_finite() && (a_exp >= 0 && b > 0 || a_exp < 0 && -a_exp < b) { // This condition indicates: // - a is a value without a decimal point, b is a positive number // - a has a decimal point, but b is larger than its scale - // In both of these situations, right-pad the number with zeroes, which // is most easily done with rescale. + // In both of these situations, right-pad the number with zeroes, which + // is most easily done with rescale. + // + // NOTE: `a_exp == 0` has no decimal point either, and the path below + // would shift it left by `b` digits and overflow for large `b`. Equal + // values whose trailing zeroes `Row` encoding folded into a positive + // exponent land here, so the two must not diverge. + // + // NOTE: `Infinity` and `NaN` report an exponent of zero too, but + // `rescale` on an infinity is an invalid operation that yields `NaN` + // without setting the overflow status checked below. The rounding path + // propagates both unchanged, as PostgreSQL does. // Ensure rescale doesn't exceed max precision by putting a ceiling on // b equal to the maximum remaining scale the value can support. diff --git a/test/sqllogictest/numeric.slt b/test/sqllogictest/numeric.slt index 24be504a442d7..39701ae7d7474 100644 --- a/test/sqllogictest/numeric.slt +++ b/test/sqllogictest/numeric.slt @@ -972,6 +972,31 @@ SELECT round(6e38, 39) ---- 600000000000000000000000000000000000000 +# A scale above the value's fractional digits cannot change it, so it clamps to +# what the datum holds rather than overflowing. +query R +SELECT round(123::numeric, 38) +---- +123 + +query R +SELECT round(5::numeric, 2147483647) +---- +5 + +# `extract` hands `round` a numeric that never passed through a row, so its +# exponent is zero where an equal value read from a row carries a positive one. +query R +SELECT round(extract(epoch FROM DATE '2000-01-01'), 2147483647) +---- +946684800 + +# The infinities carry an exponent of zero too, but a scale cannot change them. +query RR +SELECT round(sum(f1), 2), round(-sum(f1), 2) FROM (VALUES ('999999999999999999999999999999999999999'::numeric), ('999999999999999999999999999999999999999')) t (f1) +---- +Infinity -Infinity + query R SELECT round(19.87, -1) ---- diff --git a/test/testdrive/decimal-overflow.td b/test/testdrive/decimal-overflow.td index d7d2b75419893..af6c03851c440 100644 --- a/test/testdrive/decimal-overflow.td +++ b/test/testdrive/decimal-overflow.td @@ -32,10 +32,15 @@ contains:value out of range: underflow ! SELECT '999999999999999999999999999999999999999'::decimal * 10::decimal; contains:value out of range: overflow -# ROUND creates a value that is too large -! SELECT ROUND('999999999999999999999999999999999999999'::decimal,1); +# ROUND carries into a value that is too large +! SELECT ROUND('999999999999999999999999999999999999999'::decimal,-1); contains:value out of range: overflow +# A positive scale only right-pads with zeroes, so it cannot make the value too +# large. The scale clamps to what the datum can hold. +> SELECT ROUND('999999999999999999999999999999999999999'::decimal,1); +999999999999999999999999999999999999999 + # POW ! SELECT POW(99999::decimal,9); contains:value out of range: overflow