You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/10/18 09:23:03 UTC

(datafusion) 01/01: fix: restore floating-point rounding mode after panic (#26412)

This is an automated email from the ASF dual-hosted git repository.

github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-26412-3470bd0fbc016df5280493bb41c2e7ced3a4e626
in repository https://gitbox.apache.org/repos/asf/datafusion.git

commit 61c34724fcadf50f0e2eb36e3b03f9c0b675d78c
Author: Emanuele Micheletti <mi...@hotmail.com>
AuthorDate: Sun Oct 18 09:22:39 2026 +0000

    fix: restore floating-point rounding mode after panic (#26412)
    
    ## Which issue does this PR close?
    
    - Closes #7356.
    
    ## Rationale for this change
    
    A caught panic in `alter_fp_rounding_mode` leaves the thread using the
    temporary rounding mode, which can affect later floating-point
    calculations on that thread.
    
    ## What changes are included in this PR?
    
    Restore the saved rounding mode with a private `Drop` guard so cleanup
    also runs during unwinding. The change is confined to `rounding.rs` and
    adds no dependency or public API.
    
    ## What is the testing strategy for this PR?
    
    The regression test starts in downward rounding mode, panics during an
    upward-rounded operation, catches the panic, and checks that downward
    rounding was restored. It restores the test thread's original mode
    before asserting. This requires a Rust unit test because SQL and the
    DataFrame API cannot directly supply the panicking callback.
    
    The test was run against the unfixed implementation and failed because
    upward rounding remained active.
    
    Validation:
    
    - `cargo fmt --all` and `cargo fmt --all -- --check` passed.
    - `cargo test --profile ci -p datafusion-common -p
    datafusion-expr-common` passed: 847 unit tests and 89 documentation
    tests, with five documentation tests ignored.
    - `cargo clippy -p datafusion-common -p datafusion-expr-common
    --all-targets --all-features -- -D warnings` passed.
    
    `cargo clippy --all-targets --all-features -- -D warnings` reported
    three pre-existing `mutable_key_type` errors in
    `datafusion/optimizer/src/push_down_filter.rs`, tracked by #26181. Its
    remaining bundled Protobuf build was stopped after these errors.
    
    The full lint suite, SQL logic suite, and extended workspace tests were
    started but did not complete locally. The separate core and CLI test run
    was not reached.
    
    ## Are there any user-facing changes?
    
    After a caught panic, subsequent floating-point calculations on the same
    thread retain the rounding mode that was active before the operation. No
    public API or normal SQL behavior changes.
---
 datafusion/common/src/rounding.rs | 44 +++++++++++++++++++++++++++++++++++----
 1 file changed, 40 insertions(+), 4 deletions(-)

diff --git a/datafusion/common/src/rounding.rs b/datafusion/common/src/rounding.rs
index b3b514e863..11aede0b24 100644
--- a/datafusion/common/src/rounding.rs
+++ b/datafusion/common/src/rounding.rs
@@ -271,11 +271,18 @@ where
         not(target_os = "windows")
     ))]
     unsafe {
-        let current = fegetround();
+        struct RoundingModeGuard(i32);
+
+        impl Drop for RoundingModeGuard {
+            fn drop(&mut self) {
+                unsafe { fesetround(self.0) };
+            }
+        }
+
+        // Restore the previous mode even if the operation panics.
+        let _guard = RoundingModeGuard(fegetround());
         fesetround(if UPPER { FE_UPWARD } else { FE_DOWNWARD });
-        let result = operation(lhs, rhs);
-        fesetround(current);
-        result
+        operation(lhs, rhs)
     }
     #[cfg(any(
         not(any(target_arch = "x86_64", target_arch = "aarch64")),
@@ -288,6 +295,35 @@ where
 mod tests {
     use super::{next_down, next_up};
 
+    #[cfg(all(
+        any(target_arch = "x86_64", target_arch = "aarch64"),
+        not(target_os = "windows")
+    ))]
+    #[test]
+    fn test_alter_fp_rounding_mode_restores_after_panic() {
+        use super::{FE_DOWNWARD, alter_fp_rounding_mode, fegetround, fesetround};
+        use crate::ScalarValue;
+
+        unsafe {
+            let original = fegetround();
+            fesetround(FE_DOWNWARD);
+
+            let result = std::panic::catch_unwind(|| {
+                alter_fp_rounding_mode::<true, _>(
+                    &ScalarValue::Null,
+                    &ScalarValue::Null,
+                    |_, _| panic!("operation panicked"),
+                )
+            });
+
+            let restored = fegetround();
+            fesetround(original);
+
+            assert!(result.is_err());
+            assert_eq!(restored, FE_DOWNWARD);
+        }
+    }
+
     #[test]
     fn test_next_down() {
         let x = 1.0f64;


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org