sweb commented on code in PR #22892:
URL: https://github.com/apache/datafusion/pull/22892#discussion_r3408381314
##########
datafusion/common/src/scalar/mod.rs:
##########
@@ -2585,53 +2586,99 @@ impl ScalarValue {
/// distance is greater than [`usize::MAX`]. If the type is a float, then
the distance will be
/// rounded to the nearest integer.
///
- ///
/// Note: the datatype itself must support subtraction.
+ #[deprecated(since = "54.0.0", note = "Use distance_u64 instead")]
pub fn distance(&self, other: &ScalarValue) -> Option<usize> {
+ self.distance_u64(other)
+ .and_then(|d| usize::try_from(d).ok())
+ }
+
+ /// Absolute distance between two numeric values (of the same type). This
method will return
+ /// None if either one of the arguments are null. It might also return
None if the resulting
+ /// distance is greater than [`u64::MAX`]. If the type is a float, then
the distance will be
+ /// rounded to the nearest integer.
+ ///
+ /// Note: the datatype itself must support subtraction.
+ pub fn distance_u64(&self, other: &ScalarValue) -> Option<u64> {
match (self, other) {
- (Self::Int8(Some(l)), Self::Int8(Some(r))) => Some(l.abs_diff(*r)
as _),
- (Self::Int16(Some(l)), Self::Int16(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::Int32(Some(l)), Self::Int32(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::Int64(Some(l)), Self::Int64(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::UInt8(Some(l)), Self::UInt8(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::UInt16(Some(l)), Self::UInt16(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::UInt32(Some(l)), Self::UInt32(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::UInt64(Some(l)), Self::UInt64(Some(r))) =>
Some(l.abs_diff(*r) as _),
+ (Self::Int8(Some(l)), Self::Int8(Some(r))) => Some(l.abs_diff(*r)
as u64),
+ (Self::Int16(Some(l)), Self::Int16(Some(r))) =>
Some(l.abs_diff(*r) as u64),
+ (Self::Int32(Some(l)), Self::Int32(Some(r))) =>
Some(l.abs_diff(*r) as u64),
+ (Self::Int64(Some(l)), Self::Int64(Some(r))) =>
Some(l.abs_diff(*r)),
+ (Self::UInt8(Some(l)), Self::UInt8(Some(r))) =>
Some(l.abs_diff(*r) as u64),
+ (Self::UInt16(Some(l)), Self::UInt16(Some(r))) =>
Some(l.abs_diff(*r) as u64),
+ (Self::UInt32(Some(l)), Self::UInt32(Some(r))) =>
Some(l.abs_diff(*r) as u64),
+ (Self::UInt64(Some(l)), Self::UInt64(Some(r))) =>
Some(l.abs_diff(*r)),
// TODO: we might want to look into supporting ceil/floor here for
floats.
(Self::Float16(Some(l)), Self::Float16(Some(r))) => {
- Some((f16::to_f32(*l) - f16::to_f32(*r)).abs().round() as _)
+ let diff = (f16::to_f32(*l) - f16::to_f32(*r)).abs().round();
+ if diff <= u64::MAX as f32 {
+ Some(diff as u64)
+ } else {
+ None
+ }
}
(Self::Float32(Some(l)), Self::Float32(Some(r))) => {
- Some((l - r).abs().round() as _)
+ let diff = (l - r).abs().round();
+ if diff <= u64::MAX as f32 {
+ Some(diff as u64)
+ } else {
+ None
+ }
}
(Self::Float64(Some(l)), Self::Float64(Some(r))) => {
- Some((l - r).abs().round() as _)
+ let diff = (l - r).abs().round();
+ if diff <= u64::MAX as f64 {
Review Comment:
Thank you very much for your review! Addressed in
https://github.com/apache/datafusion/pull/22892/changes/c356c995c3fcc60323509122a71fadd1c96dbbc1
##########
datafusion/common/src/scalar/mod.rs:
##########
@@ -2585,53 +2586,99 @@ impl ScalarValue {
/// distance is greater than [`usize::MAX`]. If the type is a float, then
the distance will be
/// rounded to the nearest integer.
///
- ///
/// Note: the datatype itself must support subtraction.
+ #[deprecated(since = "54.0.0", note = "Use distance_u64 instead")]
pub fn distance(&self, other: &ScalarValue) -> Option<usize> {
+ self.distance_u64(other)
+ .and_then(|d| usize::try_from(d).ok())
+ }
+
+ /// Absolute distance between two numeric values (of the same type). This
method will return
+ /// None if either one of the arguments are null. It might also return
None if the resulting
+ /// distance is greater than [`u64::MAX`]. If the type is a float, then
the distance will be
+ /// rounded to the nearest integer.
+ ///
+ /// Note: the datatype itself must support subtraction.
+ pub fn distance_u64(&self, other: &ScalarValue) -> Option<u64> {
match (self, other) {
- (Self::Int8(Some(l)), Self::Int8(Some(r))) => Some(l.abs_diff(*r)
as _),
- (Self::Int16(Some(l)), Self::Int16(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::Int32(Some(l)), Self::Int32(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::Int64(Some(l)), Self::Int64(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::UInt8(Some(l)), Self::UInt8(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::UInt16(Some(l)), Self::UInt16(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::UInt32(Some(l)), Self::UInt32(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::UInt64(Some(l)), Self::UInt64(Some(r))) =>
Some(l.abs_diff(*r) as _),
+ (Self::Int8(Some(l)), Self::Int8(Some(r))) => Some(l.abs_diff(*r)
as u64),
+ (Self::Int16(Some(l)), Self::Int16(Some(r))) =>
Some(l.abs_diff(*r) as u64),
+ (Self::Int32(Some(l)), Self::Int32(Some(r))) =>
Some(l.abs_diff(*r) as u64),
+ (Self::Int64(Some(l)), Self::Int64(Some(r))) =>
Some(l.abs_diff(*r)),
+ (Self::UInt8(Some(l)), Self::UInt8(Some(r))) =>
Some(l.abs_diff(*r) as u64),
+ (Self::UInt16(Some(l)), Self::UInt16(Some(r))) =>
Some(l.abs_diff(*r) as u64),
+ (Self::UInt32(Some(l)), Self::UInt32(Some(r))) =>
Some(l.abs_diff(*r) as u64),
+ (Self::UInt64(Some(l)), Self::UInt64(Some(r))) =>
Some(l.abs_diff(*r)),
// TODO: we might want to look into supporting ceil/floor here for
floats.
(Self::Float16(Some(l)), Self::Float16(Some(r))) => {
- Some((f16::to_f32(*l) - f16::to_f32(*r)).abs().round() as _)
+ let diff = (f16::to_f32(*l) - f16::to_f32(*r)).abs().round();
+ if diff <= u64::MAX as f32 {
+ Some(diff as u64)
+ } else {
+ None
+ }
}
(Self::Float32(Some(l)), Self::Float32(Some(r))) => {
- Some((l - r).abs().round() as _)
+ let diff = (l - r).abs().round();
+ if diff <= u64::MAX as f32 {
Review Comment:
Addressed in
https://github.com/apache/datafusion/pull/22892/changes/c356c995c3fcc60323509122a71fadd1c96dbbc1
##########
datafusion/common/src/scalar/mod.rs:
##########
@@ -2585,53 +2586,99 @@ impl ScalarValue {
/// distance is greater than [`usize::MAX`]. If the type is a float, then
the distance will be
/// rounded to the nearest integer.
///
- ///
/// Note: the datatype itself must support subtraction.
+ #[deprecated(since = "54.0.0", note = "Use distance_u64 instead")]
pub fn distance(&self, other: &ScalarValue) -> Option<usize> {
+ self.distance_u64(other)
+ .and_then(|d| usize::try_from(d).ok())
+ }
+
+ /// Absolute distance between two numeric values (of the same type). This
method will return
+ /// None if either one of the arguments are null. It might also return
None if the resulting
+ /// distance is greater than [`u64::MAX`]. If the type is a float, then
the distance will be
+ /// rounded to the nearest integer.
+ ///
+ /// Note: the datatype itself must support subtraction.
+ pub fn distance_u64(&self, other: &ScalarValue) -> Option<u64> {
match (self, other) {
- (Self::Int8(Some(l)), Self::Int8(Some(r))) => Some(l.abs_diff(*r)
as _),
- (Self::Int16(Some(l)), Self::Int16(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::Int32(Some(l)), Self::Int32(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::Int64(Some(l)), Self::Int64(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::UInt8(Some(l)), Self::UInt8(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::UInt16(Some(l)), Self::UInt16(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::UInt32(Some(l)), Self::UInt32(Some(r))) =>
Some(l.abs_diff(*r) as _),
- (Self::UInt64(Some(l)), Self::UInt64(Some(r))) =>
Some(l.abs_diff(*r) as _),
+ (Self::Int8(Some(l)), Self::Int8(Some(r))) => Some(l.abs_diff(*r)
as u64),
+ (Self::Int16(Some(l)), Self::Int16(Some(r))) =>
Some(l.abs_diff(*r) as u64),
+ (Self::Int32(Some(l)), Self::Int32(Some(r))) =>
Some(l.abs_diff(*r) as u64),
+ (Self::Int64(Some(l)), Self::Int64(Some(r))) =>
Some(l.abs_diff(*r)),
+ (Self::UInt8(Some(l)), Self::UInt8(Some(r))) =>
Some(l.abs_diff(*r) as u64),
+ (Self::UInt16(Some(l)), Self::UInt16(Some(r))) =>
Some(l.abs_diff(*r) as u64),
+ (Self::UInt32(Some(l)), Self::UInt32(Some(r))) =>
Some(l.abs_diff(*r) as u64),
+ (Self::UInt64(Some(l)), Self::UInt64(Some(r))) =>
Some(l.abs_diff(*r)),
// TODO: we might want to look into supporting ceil/floor here for
floats.
(Self::Float16(Some(l)), Self::Float16(Some(r))) => {
Review Comment:
Addressed in
https://github.com/apache/datafusion/pull/22892/changes/c356c995c3fcc60323509122a71fadd1c96dbbc1
Since this is now a helper, I am also checking for >= 0
--
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]
