alamb commented on code in PR #12586:
URL: https://github.com/apache/datafusion/pull/12586#discussion_r1771865616
##########
datafusion/physical-plan/src/aggregates/group_values/row.rs:
##########
@@ -230,6 +231,11 @@ impl GroupValues for GroupValuesRows {
}
*array = cast(array.as_ref(), expected)?;
}
+
+ if expected.is_nested() &&
needs_nested_dictionary_encoding(expected, array)?
+ {
+ *array = dictionary_encode_nested(array.clone(), expected)?;
+ }
Review Comment:
It seems to me like `dictionary_encode_nested` would work on non nested
types as well, and thus you could simply always call it (and avoid the special
case for Dict above too).
That which would make this code simpler and easier to follow. You can
probably avoid the clone if you wanted (not a huge deal give it is an Arc):
```suggestion
*array = dictionary_encode_nested(&array, expected)?;
```
--
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]
