You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "findepi (via GitHub)" <gi...@apache.org> on 2025/06/30 12:26:48 UTC
[PR] Avoid treating incomparable scalars as equal [datafusion]
findepi opened a new pull request, #16624:
URL: https://github.com/apache/datafusion/pull/16624
Fix calls to `ScalarValue::partial_cmp` that treat `None` return value as equivalent to equality. The `partial_cmp` returns `None` when values cannot be compared, for example they have incompatible types. Apparently, in all these places it is an error condition when values cannot be compared.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Avoid treating incomparable scalars as equal [datafusion]
Posted by "findepi (via GitHub)" <gi...@apache.org>.
findepi merged PR #16624:
URL: https://github.com/apache/datafusion/pull/16624
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Avoid treating incomparable scalars as equal [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #16624:
URL: https://github.com/apache/datafusion/pull/16624#discussion_r2175729099
##########
datafusion/common/src/scalar/mod.rs:
##########
@@ -3343,6 +3339,14 @@ impl ScalarValue {
arr1 == &right
}
+ /// Compare `self` with `other` and return an `Ordering`.
+ /// Returns `Err` if the values cannot be compared, e.g., they have incompatible data types.
Review Comment:
```suggestion
/// Compare `self` with `other` and return an `Ordering`.
///
/// This is the same as [`PartialCmp`] except that it returns
/// `Err` if the values cannot be compared, e.g., they have incompatible data types.
```
##########
datafusion/common/src/scalar/mod.rs:
##########
@@ -3343,6 +3339,14 @@ impl ScalarValue {
arr1 == &right
}
+ /// Compare `self` with `other` and return an `Ordering`.
+ /// Returns `Err` if the values cannot be compared, e.g., they have incompatible data types.
+ pub fn try_cmp(&self, other: &Self) -> Result<Ordering> {
+ self.partial_cmp(other).ok_or_else(|| {
+ _internal_datafusion_err!("Uncomparable values: {self:?}, {other:?}")
Review Comment:
Is this an internal error, in the "it arises from a bug in DataFusion" sense?
If it can come from a malformed query / user input I think it might be better to return a `DataFusionError::Execution` instead.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Avoid treating incomparable scalars as equal [datafusion]
Posted by "findepi (via GitHub)" <gi...@apache.org>.
findepi commented on code in PR #16624:
URL: https://github.com/apache/datafusion/pull/16624#discussion_r2175762054
##########
datafusion/common/src/scalar/mod.rs:
##########
@@ -3343,6 +3339,14 @@ impl ScalarValue {
arr1 == &right
}
+ /// Compare `self` with `other` and return an `Ordering`.
+ /// Returns `Err` if the values cannot be compared, e.g., they have incompatible data types.
+ pub fn try_cmp(&self, other: &Self) -> Result<Ordering> {
+ self.partial_cmp(other).ok_or_else(|| {
+ _internal_datafusion_err!("Uncomparable values: {self:?}, {other:?}")
Review Comment:
i think it's internal in that sense
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org