Skip to content
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

daft.Expression.__or__ and daft.Expression.__and__ should not propagate nulls #3512

Closed
kevinzwang opened this issue Dec 6, 2024 · 0 comments · Fixed by #3544
Closed

daft.Expression.__or__ and daft.Expression.__and__ should not propagate nulls #3512

kevinzwang opened this issue Dec 6, 2024 · 0 comments · Fixed by #3544
Assignees
Labels
bug Something isn't working p1 Important to tackle soon, but preemptable by p0

Comments

@kevinzwang
Copy link
Member

Describe the bug

Our expressions for or + and yield null when either side is null, but when the statement is always true or false regardless of the null-side value, we should yield that instead of propagating nulls.

To Reproduce

Consider this dataframe:

import daft

df = daft.from_pydict({
    "a": [True, False],
    "b": [None, None]
})

df.select(df["a"] | df["b"]) and df.select(df["a"] & df["b"]) both yield a column with two null rows.

Same behavior exists in SQL

Expected behavior

df.select(df["a"] | df["b"]) should yield [True, null] since the first row is always true.

df.select(df["a"] & df["b"]) should yield [null, False] since the first row is always false.

Component(s)

SQL, Other

Additional context

No response

@kevinzwang kevinzwang added bug Something isn't working needs triage p1 Important to tackle soon, but preemptable by p0 and removed needs triage labels Dec 6, 2024
@kevinzwang kevinzwang self-assigned this Dec 6, 2024
kevinzwang added a commit that referenced this issue Dec 11, 2024
Resolves #3512

A fun truth table ($V_{x}$ is the validity $x$, $L$ is left, and $R$ is
right):

| $L$ | $V_{L}$ | $R$ | $V_{R}$ | $∧$ | $V_{∧}$ | $∨$ | $V_{∨}$ |
| :-----: | :-----: | :-----: | :-----: | :-----: | :-----: | :-----: |
:-----: |
| ⬜️ | ⬜️ | ⬜️ | ⬜️ |     | ⬜️ |     | ⬜️ |
| ⬜️ | ⬜️ | ⬜️ | ✅  | ⬜️ | ✅  |     | ⬜️ |
| ⬜️ | ⬜️ | ✅  | ⬜️ |     | ⬜️ |     | ⬜️ |
| ⬜️ | ⬜️ | ✅  | ✅  |     | ⬜️ | ✅  | ✅  |
| ⬜️ | ✅  | ⬜️ | ⬜️ | ⬜️ | ✅  |     | ⬜️ |
| ⬜️ | ✅  | ⬜️ | ✅  | ⬜️ | ✅  | ⬜️ | ✅  |
| ⬜️ | ✅  | ✅  | ⬜️ | ⬜️ | ✅  |     | ⬜️ |
| ⬜️ | ✅  | ✅  | ✅  | ⬜️ | ✅  | ✅  | ✅  |
| ✅  | ⬜️ | ⬜️ | ⬜️ |     | ⬜️ |     | ⬜️ |
| ✅  | ⬜️ | ⬜️ | ✅  | ⬜️ | ✅  |     | ⬜️ |
| ✅  | ⬜️ | ✅  | ⬜️ |     | ⬜️ |     | ⬜️ |
| ✅  | ⬜️ | ✅  | ✅  |     | ⬜️ | ✅  | ✅  |
| ✅  | ✅  | ⬜️ | ⬜️ |     | ⬜️ | ✅  | ✅  |
| ✅  | ✅  | ⬜️ | ✅  | ⬜️ | ✅  | ✅  | ✅  |
| ✅  | ✅  | ✅  | ⬜️ |     | ⬜️ | ✅  | ✅  |
| ✅  | ✅  | ✅  | ✅  | ✅  | ✅  | ✅  | ✅  |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p1 Important to tackle soon, but preemptable by p0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant