asolimando commented on code in PR #18342:
URL: https://github.com/apache/datafusion/pull/18342#discussion_r2472315640
##########
datafusion/core/tests/physical_optimizer/sanity_checker.rs:
##########
@@ -379,6 +379,32 @@ async fn test_analyzer() -> Result<()> {
Ok(())
}
+#[tokio::test]
+async fn test_order_by_case_requirement() -> Result<()> {
+ let sql = r#"
+ WITH typ(oid, typnamespace, typname, typtype) AS (
+ SELECT * FROM (VALUES (1, 10, 't1', 'b'))
+ UNION ALL SELECT * FROM (VALUES (2, NULL, 't2', 'b'))
+ UNION ALL SELECT * FROM (VALUES (3, 12, 't3', NULL))
Review Comment:
Nit: maybe adding a non-NULL value which is retained by the filter but not
matching any `WHEN` branch like `m`? We are exercising the implicit `ELSE null`
with rows having `typtype` as `NULL` value, but it might be worth checking with
explicit values too?
##########
datafusion/expr-common/src/interval_arithmetic.rs:
##########
@@ -583,7 +583,10 @@ impl Interval {
upper: ScalarValue::Boolean(Some(upper)),
})
}
- _ => internal_err!("Incompatible data types for logical
conjunction"),
+ // TODO: bounds MAY be certain if either LHS or RHS is certainly
false
+
+ // Return UNCERTAIN when intervals don't have concrete boolean
bounds
+ _ => Ok(Self::UNCERTAIN),
Review Comment:
This make sense to me, under the classic 3-valued logic, in such cases it's
fine to return `unknown`.
This method should be used with care as it would probably be unsafe for
simplifications, as the TODO suggests we are not handling cases like `FALSE AND
UNKNOWN` which is `FALSE`, or `TRUE OR UNKNOWN` which is `TRUE`.
Depending on where we invoke this method, we need to be aware of the
unknown-as-false semantics (filters) and unknown-as-null semantics (order by,
projections, etc.), and decide if it is safe to use this method or not.
Since we were previously erroring out, I think it's in any case an
improvement but just wanted to point that out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
