apache / apache/datafusion

[DISCUSSION] Potentially consolidate Expr::IsNotUnknown, `IsKnown`, etc

Open
#11,282 4 comments 0 reactions 0 assignees View on GitHub
enhancement
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.