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