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

Minor: improve documentation for IsNotNull, DISTINCT, etc #8052

Merged
merged 2 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions datafusion/expr/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,21 +99,21 @@ pub enum Expr {
SimilarTo(Like),
/// Negation of an expression. The expression's type must be a boolean to make sense.
Not(Box<Expr>),
/// Whether an expression is not Null. This expression is never null.
/// True if argument is not NULL, false otherwise. This expression itself is never NULL.
IsNotNull(Box<Expr>),
/// Whether an expression is Null. This expression is never null.
/// True if argument is NULL, false otherwise. This expression itself is never NULL.
IsNull(Box<Expr>),
/// Whether an expression is True. Boolean operation
/// True if argument is true, false otherwise. This expression itself is never NULL.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

IsTrue(Box<Expr>),
/// Whether an expression is False. Boolean operation
/// True if argument is false, false otherwise. This expression itself is never NULL.
IsFalse(Box<Expr>),
/// Whether an expression is Unknown. Boolean operation
/// True if argument is NULL, false otherwise. This expression itself is never NULL.
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be a little bit misleading, prob we need to add what is unknown ....
https://en.wikipedia.org/wiki/Boolean_data_type#SQL

NULL BOOLEAN and UNKNOWN "may be used interchangeably to mean exactly the same thing"

so here the user can think that unknown the same as null, but its boolean null, not sure if it matters tbh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think saying these expressions result in NULL is accurate. There is indeed some subtlety related to using expressions that evaluate to NULL (booleans) for filters -- where the value does not pass the filter if it evaluates to false or NULL (it only passes if it is true). 🤯

Copy link
Member

Choose a reason for hiding this comment

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

Here's a fun example of that subtlety: https://www.db-fiddle.com/f/xbHAYw59p4tgtR1L39p84x/0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NOT IN (aka what are the semantics of an Anti join with NULL). 🤯 indeed

IsUnknown(Box<Expr>),
/// Whether an expression is not True. Boolean operation
/// True if argument is FALSE or NULL, false otherwise. This expression itself is never NULL.
IsNotTrue(Box<Expr>),
/// Whether an expression is not False. Boolean operation
/// True if argument is TRUE OR NULL, false otherwise. This expression itself is never NULL.
IsNotFalse(Box<Expr>),
/// Whether an expression is not Unknown. Boolean operation
/// True if argument is TRUE or FALSE, false otherwise. This expression itself is never NULL.
IsNotUnknown(Box<Expr>),
/// arithmetic negation of an expression, the operand must be of a signed numeric data type
Negative(Box<Expr>),
Expand Down
8 changes: 6 additions & 2 deletions datafusion/expr/src/operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,13 @@ pub enum Operator {
And,
/// Logical OR, like `||`
Or,
/// IS DISTINCT FROM
/// `IS DISTINCT FROM` (see [`distinct`])
///
/// [`distinct`]: arrow::compute::kernels::cmp::distinct
IsDistinctFrom,
/// IS NOT DISTINCT FROM
/// `IS NOT DISTINCT FROM` (see [`not_distinct`])
///
/// [`not_distinct`]: arrow::compute::kernels::cmp::not_distinct
IsNotDistinctFrom,
/// Case sensitive regex match
RegexMatch,
Expand Down