tustvold commented on code in PR #4707:
URL: https://github.com/apache/arrow-rs/pull/4707#discussion_r1322080738
##########
arrow-arith/src/arity.rs:
##########
@@ -105,10 +105,11 @@ where
let dict_values = array.values().as_any().downcast_ref().unwrap();
let values = try_unary::<T, F, T>(dict_values, op)?;
- Ok(Arc::new(array.with_values(&values)))
+ Ok(Arc::new(array.with_values(Arc::new(values))))
}
/// Applies an infallible unary function to an array with primitive values.
+#[deprecated(note = "Use arrow_array::AnyDictionaryArray")]
Review Comment:
My major motivation for deprecating it was to move away from these
quasi-generic APIs, that are the worst of both worlds. They have to
parameterised on the value type, whilst also not being properly generic.
Additionally in this particular case it results in codegen for each type of key
type, and returns a PrimitiveArray when it could preserve the dictionary.
There is an example on AnyDictionaryArray of how to handle this better -
https://docs.rs/arrow-array/latest/arrow_array/array/trait.AnyDictionaryArray.html.
Swapping the with_values for the take kernel will hydrate the dictionary as a
PrimitiveArray if that is the desired behaviour, this will be both faster and
result in less codegen
TLDR this function has very peculiar semantics that I think encourage a
problematic pattern
--
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]
