expr: clamp round(numeric, scale) instead of overflowing

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;

src/expr/src/scalar/func.rs

@@ -481,11 +481,22 @@ fn round_numeric_binary(a: OrderedDecimal<Numeric>, mut b: i32) -> Result<Numeri
     let mut a = a.0;
     let mut cx = numeric::cx_datum();
     let a_exp = a.exponent();
-    if a_exp > 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.

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)
 ----

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