Skip to content

Commit 484183d

Browse files
1fanwangkosiew
1fanwang
and
kosiew
authored
fix: preserve negative zero in trunc (#25732)
## Which issue does this PR close? - Closes #25702. ## Rationale for this change @jonasdedden reported that trunc(column) changes -0.0 to 0.0 on the array path, while scalar trunc(-0.0) and trunc(column, 0) keep the sign. That makes the one-argument column form behave differently from the scalar form and from the explicit precision form. After this change, the one-argument Float32 and Float64 array paths use the native trunc() result directly, so the SQL path keeps the sign of negative zero. ## What changes are included in this PR? The Float32 and Float64 one-argument array implementation now matches Rust's trunc() behavior for zero values, so trunc(column) keeps the sign like the scalar and explicit precision forms. ## What is the testing strategy for this PR? The reported behavior is shown below for Float32 and Float64 columns, on upstream main and on this branch. ## Testing Done A unit test in trunc.rs checks the Float32 and Float64 results directly, and math.slt checks the same values through SQL. The reported behavior is also reproducible with a small program. Put the source below in src/main.rs of a new crate, add these dependencies to its Cargo.toml with the path pointing at a checkout, and pass the two queries as arguments to cargo run. ```toml [dependencies] datafusion = { path = "../datafusion/datafusion/core", default-features = false, features = ["sql", "math_expressions"] } tokio = { version = "1", features = ["rt-multi-thread", "macros"] } ``` ```bash cargo run -q -- \ "SELECT 1.0 / trunc(column1), 1.0 / trunc(column1, 0) FROM (VALUES (CAST(-0.0 AS REAL)))" \ "SELECT 1.0 / trunc(column1), 1.0 / trunc(column1, 0) FROM (VALUES (CAST(-0.0 AS DOUBLE)))" ``` Before, on upstream main (91cd7ff). The one-argument form returns inf, the explicit precision form returns -inf: ```text SELECT 1.0 / trunc(column1), 1.0 / trunc(column1, 0) FROM (VALUES (CAST(-0.0 AS REAL))) +-----------------------------+--------------------------------------+ | Float64(1) / trunc(column1) | Float64(1) / trunc(column1,Int64(0)) | +-----------------------------+--------------------------------------+ | inf | -inf | +-----------------------------+--------------------------------------+ SELECT 1.0 / trunc(column1), 1.0 / trunc(column1, 0) FROM (VALUES (CAST(-0.0 AS DOUBLE))) +-----------------------------+--------------------------------------+ | Float64(1) / trunc(column1) | Float64(1) / trunc(column1,Int64(0)) | +-----------------------------+--------------------------------------+ | inf | -inf | +-----------------------------+--------------------------------------+ ``` After, on this branch. Both forms return -inf: ```text SELECT 1.0 / trunc(column1), 1.0 / trunc(column1, 0) FROM (VALUES (CAST(-0.0 AS REAL))) +-----------------------------+--------------------------------------+ | Float64(1) / trunc(column1) | Float64(1) / trunc(column1,Int64(0)) | +-----------------------------+--------------------------------------+ | -inf | -inf | +-----------------------------+--------------------------------------+ SELECT 1.0 / trunc(column1), 1.0 / trunc(column1, 0) FROM (VALUES (CAST(-0.0 AS DOUBLE))) +-----------------------------+--------------------------------------+ | Float64(1) / trunc(column1) | Float64(1) / trunc(column1,Int64(0)) | +-----------------------------+--------------------------------------+ | -inf | -inf | +-----------------------------+--------------------------------------+ ``` <details><summary>Reproducer source: main.rs</summary> ```rust use datafusion::error::Result; use datafusion::prelude::SessionContext; #[tokio::main] async fn main() -> Result<()> { let ctx = SessionContext::new(); for sql in std::env::args().skip(1) { println!("{sql}"); ctx.sql(&sql).await?.show().await?; } Ok(()) } ``` </details> ## Are there any user-facing changes? Yes. trunc(column) now preserves the sign of -0.0 for Float32 and Float64 inputs. --------- Signed-off-by: 1fanwang <[email protected]> Co-authored-by: kosiew <[email protected]>
1 parent 5f21582 commit 484183d

2 files changed

Lines changed: 39 additions & 22 deletions

File tree

‎datafusion/functions/src/math/trunc.rs‎

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,6 @@ impl ScalarUDFImpl for TruncFunc {
162162
}
163163
};
164164

165-
// Whether an explicit precision argument was supplied. The array fast
166-
// paths below must only apply to the two-argument form: single-argument
167-
// `trunc(x)` uses a different zero handling (mapping `-0.0` to `0.0`)
168-
// that must be preserved.
169165
let has_precision_arg = args.args.len() == 2;
170166

171167
// Scalar fast path using tuple matching for (value, precision)
@@ -305,15 +301,11 @@ fn trunc(args: &[ArrayRef]) -> Result<ArrayRef> {
305301

306302
match num.data_type() {
307303
Float64 => match precision {
308-
ColumnarValue::Scalar(Int64(Some(0))) => {
309-
Ok(Arc::new(
310-
args[0]
311-
.as_primitive::<Float64Type>()
312-
.unary::<_, Float64Type>(|x: f64| {
313-
if x == 0_f64 { 0_f64 } else { x.trunc() }
314-
}),
315-
) as ArrayRef)
316-
}
304+
ColumnarValue::Scalar(Int64(Some(0))) => Ok(Arc::new(
305+
args[0]
306+
.as_primitive::<Float64Type>()
307+
.unary::<_, Float64Type>(|x: f64| x.trunc()),
308+
) as ArrayRef),
317309
ColumnarValue::Array(precision) => {
318310
let num_array = num.as_primitive::<Float64Type>();
319311
let precision_array = precision.as_primitive::<Int64Type>();
@@ -327,15 +319,11 @@ fn trunc(args: &[ArrayRef]) -> Result<ArrayRef> {
327319
_ => exec_err!("trunc function requires a scalar or array for precision"),
328320
},
329321
Float32 => match precision {
330-
ColumnarValue::Scalar(Int64(Some(0))) => {
331-
Ok(Arc::new(
332-
args[0]
333-
.as_primitive::<Float32Type>()
334-
.unary::<_, Float32Type>(|x: f32| {
335-
if x == 0_f32 { 0_f32 } else { x.trunc() }
336-
}),
337-
) as ArrayRef)
338-
}
322+
ColumnarValue::Scalar(Int64(Some(0))) => Ok(Arc::new(
323+
args[0]
324+
.as_primitive::<Float32Type>()
325+
.unary::<_, Float32Type>(|x: f32| x.trunc()),
326+
) as ArrayRef),
339327
ColumnarValue::Array(precision) => {
340328
let num_array = num.as_primitive::<Float32Type>();
341329
let precision_array = precision.as_primitive::<Int64Type>();
@@ -555,6 +543,25 @@ mod test {
555543
assert_eq!(floats.value(4), -321.0);
556544
}
557545

546+
#[test]
547+
fn test_truncate_float_zero_sign_one_arg() {
548+
let args: Vec<ArrayRef> = vec![Arc::new(Float64Array::from(vec![-0.0, 0.0]))];
549+
let result = trunc(&args).expect("failed to initialize function truncate");
550+
let floats =
551+
as_float64_array(&result).expect("failed to initialize function truncate");
552+
553+
assert!(floats.value(0).is_sign_negative());
554+
assert!(floats.value(1).is_sign_positive());
555+
556+
let args: Vec<ArrayRef> = vec![Arc::new(Float32Array::from(vec![-0.0, 0.0]))];
557+
let result = trunc(&args).expect("failed to initialize function truncate");
558+
let floats =
559+
as_float32_array(&result).expect("failed to initialize function truncate");
560+
561+
assert!(floats.value(0).is_sign_negative());
562+
assert!(floats.value(1).is_sign_positive());
563+
}
564+
558565
#[test]
559566
fn test_compute_truncate_decimal128() {
560567
// number 12.3456 (scale 4) truncated to 3 places = 12.345

‎datafusion/sqllogictest/test_files/math.slt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,16 @@ SELECT round(125.2345, -3), round(125.2345, -2), round(125.2345, -1), round(125.
8888
----
8989
0 100 130 125 125 125.2 125.23 125.235
9090

91+
query RR
92+
SELECT 1.0 / trunc(column1), 1.0 / trunc(column1, 0) FROM (VALUES (CAST(-0.0 AS DOUBLE)));
93+
----
94+
-Infinity -Infinity
95+
96+
query RR
97+
SELECT 1.0 / trunc(column1), 1.0 / trunc(column1, 0) FROM (VALUES (CAST(-0.0 AS REAL)));
98+
----
99+
-Infinity -Infinity
100+
91101
# Round signed and unsigned integer scalar widths
92102
query IIIIIIII
93103
SELECT

0 commit comments

Comments
 (0)