Skip to content

Interval arithmetic apply_operator does not support OR #15715

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
davidhewitt opened this issue Apr 15, 2025 · 0 comments · May be fixed by #15716
Open

Interval arithmetic apply_operator does not support OR #15715

davidhewitt opened this issue Apr 15, 2025 · 0 comments · May be fixed by #15716
Labels
bug Something isn't working

Comments

@davidhewitt
Copy link
Contributor

davidhewitt commented Apr 15, 2025

Describe the bug

Originally requested in #7883, which got closed by #8276

Looks like that PR added support to OR to propagate_constraints but missed OR out of apply_operator (which is used by evaluate_bounds) -

pub fn apply_operator(op: &Operator, lhs: &Interval, rhs: &Interval) -> Result<Interval> {
match *op {
Operator::Eq => lhs.equal(rhs),
Operator::NotEq => lhs.equal(rhs)?.not(),
Operator::Gt => lhs.gt(rhs),
Operator::GtEq => lhs.gt_eq(rhs),
Operator::Lt => lhs.lt(rhs),
Operator::LtEq => lhs.lt_eq(rhs),
Operator::And => lhs.and(rhs),
Operator::Plus => lhs.add(rhs),
Operator::Minus => lhs.sub(rhs),
Operator::Multiply => lhs.mul(rhs),
Operator::Divide => lhs.div(rhs),
_ => internal_err!("Interval arithmetic does not support the operator {op}"),
}

Seems like this should just be added?

To Reproduce

#[test]
fn test_evaluate_bounds_bool() -> Result<()> {
    let schema = Schema::new(vec![
        Field::new("a", DataType::Boolean, false),
        Field::new("b", DataType::Boolean, false),
    ]);

    let a = Arc::new(Column::new("a", 0)) as _;
    let b = Arc::new(Column::new("b", 1)) as _;

    // Test OR bounds - currently not working
    let or_expr = binary_expr(Arc::clone(&a), Operator::Or, Arc::clone(&b), &schema)?;
    let or_bounds = or_expr.evaluate_bounds(&[
        &Interval::make(Some(true), Some(true))?,
        &Interval::make(Some(false), Some(false))?,
    ])?;
    assert_eq!(or_bounds, Interval::make(Some(true), Some(true))?);

    // Test AND bounds - working fine
    let and_expr =
        binary_expr(Arc::clone(&a), Operator::And, Arc::clone(&b), &schema)?;
    let and_bounds = and_expr.evaluate_bounds(&[
        &Interval::make(Some(true), Some(true))?,
        &Interval::make(Some(false), Some(false))?,
    ])?;
    assert_eq!(and_bounds, Interval::make(Some(false), Some(false))?);

    Ok(())
}

Expected behavior

Looks like this test should pass.

Additional context

I have a PR to fix, will submit shortly.

@davidhewitt davidhewitt added the bug Something isn't working label Apr 15, 2025
@davidhewitt davidhewitt linked a pull request Apr 15, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant