alamb commented on code in PR #10908:
URL: https://github.com/apache/datafusion/pull/10908#discussion_r1649449221
##########
datafusion/functions-array/src/string.rs:
##########
@@ -281,6 +283,49 @@ pub(super) fn array_to_string_inner(args: &[ArrayRef]) ->
Result<ArrayRef> {
Ok(arg)
}
+ Dictionary(_, _) => {
+ let any_dict_array = arr.as_any_dictionary_opt().ok_or_else(||
{
Review Comment:
Thank you @EduardoVega -- it is very impressive you got this to work
I am somewhat concerned that this is quite a bit of code (it actually
handles dictionaries of any type, not just string dictionaries, and that it
also supports arbitrary key types -- not just the integer types specified)
I am going to try and simplify this a bit
##########
datafusion/functions-array/src/string.rs:
##########
@@ -281,6 +283,49 @@ pub(super) fn array_to_string_inner(args: &[ArrayRef]) ->
Result<ArrayRef> {
Ok(arg)
}
+ Dictionary(_, _) => {
+ let any_dict_array = arr.as_any_dictionary_opt().ok_or_else(||
{
+ DataFusionError::Internal(format!(
+ "could not cast {} to AnyDictionaryArray",
+ arr.data_type()
+ ))
+ })?;
+
+ let mut values: Vec<String> = vec![];
+ macro_rules! array_function {
+ ($ARRAY_TYPE:ident) => {{
+ for x in downcast_arg!(any_dict_array.values(),
$ARRAY_TYPE) {
+ if let Some(x) = x {
+ values.push(x.to_string());
+ }
+ }
+ }};
+ }
+ call_array_function!(any_dict_array.values().data_type(),
false);
+
+ let mut keys: Vec<usize> = vec![];
+ macro_rules! array_function {
+ ($ARRAY_TYPE:ident) => {{
+ for x in downcast_arg!(any_dict_array.keys(),
$ARRAY_TYPE) {
+ if let Some(x) = x {
+
keys.push(x.to_string().parse::<usize>().unwrap());
Review Comment:
this works, but converts integers to strings just to parse them again -- I
think we can just support string dictionaries and avoid this
##########
datafusion/functions-array/src/string.rs:
##########
@@ -281,6 +283,49 @@ pub(super) fn array_to_string_inner(args: &[ArrayRef]) ->
Result<ArrayRef> {
Ok(arg)
}
+ Dictionary(_, _) => {
+ let any_dict_array = arr.as_any_dictionary_opt().ok_or_else(||
{
Review Comment:
I pushed 5a5397bef that I think simplifies the implementation a bit -- let
me know what you think
##########
datafusion/functions-array/src/string.rs:
##########
@@ -281,6 +281,24 @@ pub(super) fn array_to_string_inner(args: &[ArrayRef]) ->
Result<ArrayRef> {
Ok(arg)
}
+ Dictionary(..) => {
+ let any_dict_array = arr
+ .as_any_dictionary_opt()
+ .ok_or_else( || {
+ DataFusionError::Internal(
+ format!("could not cast {} to AnyDictionaryArray",
arr.data_type())
+ )
+ })?;
+ compute_array_to_string(
Review Comment:
Amazing @EduardoVega -- thank you so much -- this is quite an impressive
first contribution. I didn't realize it was going to be quite as tricky.
--
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-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
