-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Compare NULL types #5158
Compare NULL types #5158
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks @melgenek
cc @liukun4515
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it -- thank you @melgenek
I double checked postgres
postgres=# select pg_typeof(null and null);
pg_typeof
-----------
boolean
(1 row)
#Boolean Boolean | ||
onlyif DataFusion | ||
query ?? | ||
select arrow_typeof(null and null), arrow_typeof(null or null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Benchmark runs are scheduled for baseline = e41c4df and contender = 7d2d51b. 7d2d51b is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
The sqllogictest contains the same test as you performed manually https://github.com/melgenek/arrow-datafusion/blob/f1087f5c042ea093c6fd4d700dcecd8d2da6bf4a/datafusion/core/tests/sqllogictests/test_files/pg_compat/pg_compat_type_coercion.slt#L111-L115. It is just not visible in the git diff, because I added it in another pr |
Which issue does this PR close?
Closes #4335.
Rationale for this change
These queries should produce a value instead of an error
What changes are included in this PR?
Null comparison coercion and tests.
I made the null logical comparison produce booleans. This is the Postgres does it, and DataFusion seems to be compatible with Postgres.
Are these changes tested?
Sqllogictests and unit tests
Are there any user-facing changes?
null and null
andnull or null
would produce Boolean null value instead of an error.