alamb commented on code in PR #18625:
URL: https://github.com/apache/datafusion/pull/18625#discussion_r2518757423
##########
datafusion/expr-common/src/interval_arithmetic.rs:
##########
@@ -1738,6 +1738,44 @@ impl From<ScalarValue> for NullableInterval {
}
impl NullableInterval {
+ /// An interval that only contains 'false'.
+ pub const FALSE: Self = NullableInterval::NotNull {
+ values: Interval::CERTAINLY_FALSE,
+ };
+
+ /// An interval that only contains 'true'.
+ pub const TRUE: Self = NullableInterval::NotNull {
+ values: Interval::CERTAINLY_TRUE,
+ };
+
+ /// An interval that only contains 'unknown' (boolean null).
+ pub const UNKNOWN: Self = NullableInterval::Null {
+ datatype: DataType::Boolean,
+ };
+
+ /// An interval that only contains 'true' and 'false'.
+ /// This interval is equivalent to [Interval::UNCERTAIN].
+ pub const TRUE_OR_FALSE: Self = NullableInterval::NotNull {
+ values: Interval::UNCERTAIN,
+ };
+
+ /// An interval that only contains 'true' and 'unknown'.
+ pub const TRUE_OR_UNKNOWN: Self = NullableInterval::MaybeNull {
+ values: Interval::CERTAINLY_TRUE,
+ };
+
+ /// An interval that only contains 'false' and 'unknown'.
+ pub const FALSE_OR_UNKNOWN: Self = NullableInterval::MaybeNull {
+ values: Interval::CERTAINLY_FALSE,
+ };
+
+ /// An interval that contains all possible boolean values: 'true', 'false'
and 'unknown'.
+ ///
+ /// Note that this is different from [Interval::UNCERTAIN] which only
contains 'true' and 'false'.
+ pub const UNCERTAIN: Self = NullableInterval::MaybeNull {
Review Comment:
> For NullableInterval::UNCERTAIN I'm inclined to go for ANY_TRUTH_VALUE
rather than TRUE_OR_FALSE_OR_UNKNOWN just to keep it somewhat short.
I defer to you -- I can also live with `UNCERTAIN` (my personal distaste for
SQL NULL shoudn't cause you to do more work 😆 )
> @alamb I was wondering if we might want to consider renaming the Interval
constants. I think this mapping would be nicer since they match exactly with
their NullableInterval equivalents then in both name and semantics and we're
also at least using the SQL terminology consistently.
I agree it would be a nice change
> For backwards compatibility it might be useful to retain the old constants
and mark them deprecated. Not sure what the DataFusion policy on stuff like
that is.
Yes I agree that would be best and is consistent with the API health policy
is here: https://datafusion.apache.org/contributor-guide/api-health.html
--
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]
