Support all boolean operations #719
Conversation
sarahyurick
requested review from
ayushdg,
galipremsagar,
jdye64 and
charlesbluca
as code owners
Codecov Report
@@ Coverage Diff @@
## datafusion-sql-planner #719 +/- ##
=========================================================
Coverage ? 70.08%
=========================================================
Files ? 73
Lines ? 3624
Branches ? 747
=========================================================
Hits ? 2540
Misses ? 941
Partials ? 143 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| except TypeError: | ||
| literal_type = SqlTypeName.NULL | ||
| literal_value = None |
There was a problem hiding this comment.
Out of interest, what type error are we catching here?
| @@ -625,7 +625,7 @@ impl PyExpr { | |||
| pub fn bool_value(&mut self) -> PyResult<bool> { | |||
| match &self.expr { | |||
| Expr::Literal(scalar_value) => match scalar_value { | |||
| ScalarValue::Boolean(iv) => Ok(iv.unwrap()), | |||
| ScalarValue::Boolean(Some(iv)) => Ok(*iv), | |||
There was a problem hiding this comment.
@charlesbluca your question is related to the change here - originally, "IS UNKNOWN" would panic here when the value is null. The change here allows us to throw the type error on the next line.
Not sure if we can come up with a case where it wouldn't be Some(true), Some(false), or None
|
Hi @charlesbluca @ayushdg @galipremsagar @jdye64 this is ready for review now |
There was a problem hiding this comment.
Outside of suggestions already made by @charlesbluca the rest looks good to me, will go ahead and approve from my end
With the recent updates to DataFusion and minor Dask SQL changes, "IS TRUE/FALSE" and "IS [NOT] UNKNOWN" are functional.
"IS NOT TRUE" and "IS NOT FALSE" should be functional once apache/datafusion#3252 is merged.
UPDATE: As long as all checks pass, this should be ready to merge.