Jefffrey commented on code in PR #21315:
URL: https://github.com/apache/datafusion/pull/21315#discussion_r3104937011
##########
datafusion/functions-aggregate-common/src/min_max.rs:
##########
@@ -413,6 +413,22 @@ macro_rules! min_max {
min_max_generic!(lhs, rhs, $OP)
}
+ (lhs, rhs)
+ if matches!(lhs, ScalarValue::Dictionary(_, _))
+ || matches!(rhs, ScalarValue::Dictionary(_, _)) =>
+ {
+ let (lhs, lhs_key_type) = dictionary_scalar_parts(lhs);
+ let (rhs, rhs_key_type) = dictionary_scalar_parts(rhs);
+ let result = min_max_generic!(lhs, rhs, $OP);
+
+ match lhs_key_type.zip(rhs_key_type) {
+ Some((key_type, _)) => {
+ ScalarValue::Dictionary(Box::new(key_type.clone()),
Box::new(result))
Review Comment:
So now we require the key types to be exact same? This doesn't seem to
follow the new logic introduced that allows logically equivalent values to be
compared 🤔
It seems like we're saying we are allowed to compare Float32 with
Dictionary(Int32, Float32); but we cannot compare Dictionary(Int8, Float32)
with Dictionary(Int32, Float32)?
--
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]
