[DISCUSSION] Potentially consolidate Expr::IsNotUnknown, `IsKnown`, etc
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
### Is your feature request related to a problem or challenge?
@findepi noted in [slack](https://the-asf.slack.com/archives/C04RJ0C85UZ/p1720119166686379)
> What is IsNotUnknown expression (and why IsNotNull is not enough)?
@Dandandan noted that
> I think it is the same, yes. AFAICT we wouldn't need it and we could transform it to IsNotNull
### Describe the solution you'd like
It would be nice to look into Expr Variants like `IsNotUnknown`,` IsUnknown`, `IsTrue`, `IsFalse`, (e.g. https://docs.rs/datafusion/latest/datafusion/logical_expr/enum.Expr.html#variant.IsUnknown )
And see if there is a way to consolidate them into a few number of variants
### Describe alternatives you've considered
_No response_
### Additional context
I believe most of these variants came in via #3275 from @
Contributor guide
Research direction
Start with the logical_expr::Expr variants mentioned in the issue and compare IsNotUnknown, IsUnknown, IsTrue, IsFalse, IsKnown, and IsNotNull in the DataFusion API documentation. Trace their usages and review issue #3275 for their origin. Done requires a resolved consolidation plan and confirmation that affected behavior remains correct.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- databases
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100