martin-g commented on code in PR #20099:
URL: https://github.com/apache/datafusion/pull/20099#discussion_r2754369273
##########
datafusion/functions/src/math/floor.rs:
##########
@@ -463,4 +540,240 @@ mod tests {
"Expected None for zero args"
);
}
+
+ // ============ Decimal32 Tests (mirrors float/int tests) ============
+
+ #[test]
+ fn test_floor_preimage_decimal_valid_cases() {
+ // ===== Decimal32 =====
+ // Positive integer decimal: 100.00 (scale=2, so raw=10000)
+ // floor(x) = 100.00 -> x in [100.00, 101.00)
+ assert_preimage_range(
+ ScalarValue::Decimal32(Some(10000), 9, 2),
+ ScalarValue::Decimal32(Some(10000), 9, 2), // 100.00
+ ScalarValue::Decimal32(Some(10100), 9, 2), // 101.00
+ );
+
+ // Smaller positive: 50.00
+ assert_preimage_range(
+ ScalarValue::Decimal32(Some(5000), 9, 2),
+ ScalarValue::Decimal32(Some(5000), 9, 2), // 50.00
+ ScalarValue::Decimal32(Some(5100), 9, 2), // 51.00
+ );
+
+ // Negative integer decimal: -5.00
+ assert_preimage_range(
+ ScalarValue::Decimal32(Some(-500), 9, 2),
+ ScalarValue::Decimal32(Some(-500), 9, 2), // -5.00
+ ScalarValue::Decimal32(Some(-400), 9, 2), // -4.00
+ );
+
+ // Zero: 0.00
+ assert_preimage_range(
+ ScalarValue::Decimal32(Some(0), 9, 2),
+ ScalarValue::Decimal32(Some(0), 9, 2), // 0.00
+ ScalarValue::Decimal32(Some(100), 9, 2), // 1.00
+ );
+
+ // Scale 0 (pure integer): 42
+ assert_preimage_range(
+ ScalarValue::Decimal32(Some(42), 9, 0),
+ ScalarValue::Decimal32(Some(42), 9, 0),
+ ScalarValue::Decimal32(Some(43), 9, 0),
+ );
+
+ // ===== Decimal64 =====
+ assert_preimage_range(
+ ScalarValue::Decimal64(Some(10000), 18, 2),
+ ScalarValue::Decimal64(Some(10000), 18, 2), // 100.00
+ ScalarValue::Decimal64(Some(10100), 18, 2), // 101.00
+ );
+
+ // Negative
+ assert_preimage_range(
+ ScalarValue::Decimal64(Some(-500), 18, 2),
+ ScalarValue::Decimal64(Some(-500), 18, 2), // -5.00
+ ScalarValue::Decimal64(Some(-400), 18, 2), // -4.00
+ );
+
+ // Zero
+ assert_preimage_range(
+ ScalarValue::Decimal64(Some(0), 18, 2),
+ ScalarValue::Decimal64(Some(0), 18, 2),
+ ScalarValue::Decimal64(Some(100), 18, 2),
+ );
+
+ // ===== Decimal128 =====
+ assert_preimage_range(
+ ScalarValue::Decimal128(Some(10000), 38, 2),
+ ScalarValue::Decimal128(Some(10000), 38, 2), // 100.00
+ ScalarValue::Decimal128(Some(10100), 38, 2), // 101.00
+ );
+
+ // Negative
+ assert_preimage_range(
+ ScalarValue::Decimal128(Some(-500), 38, 2),
+ ScalarValue::Decimal128(Some(-500), 38, 2), // -5.00
+ ScalarValue::Decimal128(Some(-400), 38, 2), // -4.00
+ );
+
+ // Zero
+ assert_preimage_range(
+ ScalarValue::Decimal128(Some(0), 38, 2),
+ ScalarValue::Decimal128(Some(0), 38, 2),
+ ScalarValue::Decimal128(Some(100), 38, 2),
+ );
+
+ // ===== Decimal256 =====
+ assert_preimage_range(
+ ScalarValue::Decimal256(Some(i256::from(10000)), 76, 2),
+ ScalarValue::Decimal256(Some(i256::from(10000)), 76, 2), // 100.00
+ ScalarValue::Decimal256(Some(i256::from(10100)), 76, 2), // 101.00
+ );
+
+ // Negative
+ assert_preimage_range(
+ ScalarValue::Decimal256(Some(i256::from(-500)), 76, 2),
+ ScalarValue::Decimal256(Some(i256::from(-500)), 76, 2), // -5.00
+ ScalarValue::Decimal256(Some(i256::from(-400)), 76, 2), // -4.00
+ );
+
+ // Zero
+ assert_preimage_range(
+ ScalarValue::Decimal256(Some(i256::ZERO), 76, 2),
+ ScalarValue::Decimal256(Some(i256::ZERO), 76, 2),
+ ScalarValue::Decimal256(Some(i256::from(100)), 76, 2),
+ );
+ }
+
+ #[test]
+ fn test_floor_preimage_decimal_non_integer() {
+ // floor(x) = 1.30 has NO SOLUTION because floor always returns an
integer
+ // Therefore preimage should return None for non-integer decimals
+
+ // Decimal32
+ assert_preimage_none(ScalarValue::Decimal32(Some(130), 9, 2)); // 1.30
+ assert_preimage_none(ScalarValue::Decimal32(Some(-250), 9, 2)); //
-2.50
+ assert_preimage_none(ScalarValue::Decimal32(Some(370), 9, 2)); // 3.70
+ assert_preimage_none(ScalarValue::Decimal32(Some(1), 9, 2)); // 0.01
+
+ // Decimal64
+ assert_preimage_none(ScalarValue::Decimal64(Some(130), 18, 2)); // 1.30
+ assert_preimage_none(ScalarValue::Decimal64(Some(-250), 18, 2)); //
-2.50
+
+ // Decimal128
+ assert_preimage_none(ScalarValue::Decimal128(Some(130), 38, 2)); //
1.30
+ assert_preimage_none(ScalarValue::Decimal128(Some(-250), 38, 2)); //
-2.50
+
+ // Decimal256
+ assert_preimage_none(ScalarValue::Decimal256(Some(i256::from(130)),
76, 2)); // 1.30
+ assert_preimage_none(ScalarValue::Decimal256(Some(i256::from(-250)),
76, 2)); // -2.50
+ }
+
+ #[test]
+ fn test_floor_preimage_decimal_overflow() {
+ // Test near MAX where adding scale_factor would overflow
+
+ // Decimal32: i32::MAX
+ // For scale=2, we add 100, so i32::MAX - 50 would overflow
+ assert_preimage_none(ScalarValue::Decimal32(Some(i32::MAX - 50), 9,
2));
Review Comment:
What exactly is the idea of this test ?
`2147483647 - 50` => 2147483597, has 10 digits and does not fit into
precision=9
I think there is no call to add 100 at all here.
##########
datafusion/functions/src/math/floor.rs:
##########
@@ -310,9 +351,45 @@ fn int_preimage_bounds<I: CheckedAdd + One + Copy>(n: I)
-> Option<(I, I)> {
Some((n, upper))
}
+/// Compute preimage bounds for floor function on decimal types.
+/// For floor(x) = n, the preimage is [n, n+1).
+/// Returns None if:
+/// - The value has a fractional part (floor always returns integers)
+/// - Adding 1 would overflow
+fn decimal_preimage_bounds<D: DecimalType>(
+ value: D::Native,
+ precision: u8,
+ scale: i8,
+) -> Option<(D::Native, D::Native)>
+where
+ D::Native: DecimalCast + ArrowNativeTypeOp + std::ops::Rem<Output =
D::Native>,
+{
+ // Use rescale_decimal to compute "1" at target scale (avoids manual pow)
+ // Convert integer 1 (scale=0) to the target scale
+ let one_scaled: D::Native = rescale_decimal::<D, D>(
+ D::Native::ONE, // value = 1
+ 1, // input_precision = 1
+ 0, // input_scale = 0 (integer)
+ precision, // output_precision
+ scale, // output_scale
+ )?;
+
+ // floor always returns an integer, so if value has a fractional part,
there's no solution
+ // Check: value % one_scaled != 0 means fractional part exists
+ if scale > 0 && value % one_scaled != D::Native::ZERO {
Review Comment:
What about negative scales ?
Arrow supports them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
