Skip to content

support OR operator in binary evaluate_bounds #15716

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
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

davidhewitt
Copy link
Contributor

Which issue does this PR close?

Rationale for this change

Seems like this should have worked as per the commentary in the issue.

What changes are included in this PR?

  • add OR case to apply_operator
  • correct typo in error message
  • add some tests for evaluate_bounds

Are these changes tested?

Yes

Are there any user-facing changes?

Yes (public function now supports additional operator)

@github-actions github-actions bot added logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates labels Apr 15, 2025
@davidhewitt
Copy link
Contributor Author

The failing tests were added in #14189 and #14735, both which talk about supporting OR as a future possibility.

Ping @berkaysynnada as the reviewer / approver of both those PRs, does this PR look ok to you? If so I will update those tests so that CI is green here.

@berkaysynnada
Copy link
Contributor

Thank you @davidhewitt. I will review this today

Copy link
Contributor

@berkaysynnada berkaysynnada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I really don't remember why didn't we add OR operator in apply_operator then :D

@berkaysynnada
Copy link
Contributor

Failing tests should be fixed after taking a merge from main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Interval arithmetic apply_operator does not support OR
2 participants