Skip to content

Replace π-related bound constants with next_up / next_down #16712

New issue
New issue
Open
#16823
Open
Replace π-related bound constants with next_up / next_down#16712
#16823
Assignees
rthummaluru
Labels
good first issueGood for newcomers
@ding-young

Description

@ding-young
ding-young
opened on Jul 8, 2025

Background

Rust 1.86 stabilized f64::{next_up, next_down} and f32::{next_down, next_up}, so we can now replace several hardcoded floating-point constants using these methods. Replacing manually defined constants with std library can improve clarity and correctness.

See several functions starting from

datafusion/datafusion/common/src/scalar/mod.rs

Lines 1191 to 1202 in a089eff

/// Returns a [`ScalarValue`] representing PI's upper bound
pub fn new_pi_upper(datatype: &DataType) -> Result<ScalarValue> {
// TODO: replace the constants with next_up/next_down when
// they are stabilized: https://doc.rust-lang.org/std/primitive.f64.html#method.next_up
match datatype {
DataType::Float32 => Ok(ScalarValue::from(consts::PI_UPPER_F32)),
DataType::Float64 => Ok(ScalarValue::from(consts::PI_UPPER_F64)),
_ => {
_internal_err!("PI_UPPER is not supported for data type: {:?}", datatype)
}
}
}

and https://github.com/apache/datafusion/blob/main/datafusion/common/src/scalar/consts.rs

Solution

  1. Update related functions in datafusion/common/src/scalar/mod.rs
  2. Verify that the updated values match the current constants, or document any small differences that arise due to precision changes.

I think this is a good first issue :)

The TODO comment is originally added in #11584

Metadata

Metadata

Assignees

Labels

good first issueGood for newcomers

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions