devanshu0987 commented on code in PR #20099:
URL: https://github.com/apache/datafusion/pull/20099#discussion_r2754600705
##########
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:
Hi, you are right.
The comment is misleading here. And the test case should move to
`test_floor_preimage_decimal_non_integer`
`Decimal32(Some(i32::MAX - 50), 9, 2)` is exactly forcing the case explained
in your earlier comment.
`2147483597` = `21474835.97` which has fractional part and hence the
preimage will be None. This test is not the right place for this.
But, `21474835.97` logically is not also representable by Decimal(9,2). That
is wrong as well.
Thanks. Let me fix this.
--
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]
