You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/07/02 16:30:32 UTC
[GitHub] [arrow-rs] viirya opened a new pull request, #1990: Support DictionaryArray in unary kernel
viirya opened a new pull request, #1990:
URL: https://github.com/apache/arrow-rs/pull/1990
# Which issue does this PR close?
<!---
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #1989.
# Rationale for this change
<!---
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
# What changes are included in this PR?
<!---
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
# Are there any user-facing changes?
<!---
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking change` label.
-->
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] viirya commented on a diff in pull request #1990: Support DictionaryArray in unary kernel
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #1990:
URL: https://github.com/apache/arrow-rs/pull/1990#discussion_r914479685
##########
arrow/src/compute/kernels/arity.rs:
##########
@@ -78,10 +82,120 @@ where
PrimitiveArray::<O>::from(data)
}
+macro_rules! unary_dict_op {
+ ($array: expr, $op: expr, $value_ty: ty) => {{
+ // Safety justification: Since the inputs are valid Arrow arrays, all values are
+ // valid indexes into the dictionary (which is verified during construction)
+
+ let array_iter = unsafe {
+ $array
+ .values()
+ .as_any()
+ .downcast_ref::<$value_ty>()
+ .unwrap()
+ .take_iter_unchecked($array.keys_iter())
Review Comment:
Ah, I remember it. It is feasible, but it has one con. Because `unary_dict` and `unary_dyn` must return `ArrayRef`, the compiler cannot infer `T` so the caller must specify it.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] viirya commented on a diff in pull request #1990: Support DictionaryArray in unary kernel
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #1990:
URL: https://github.com/apache/arrow-rs/pull/1990#discussion_r914509485
##########
arrow/src/compute/kernels/arity.rs:
##########
@@ -78,10 +83,114 @@ where
PrimitiveArray::<O>::from(data)
}
+/// A helper function that applies an unary function to a dictionary array with primitive value type.
+#[allow(clippy::redundant_closure)]
+fn unary_dict<K, F, T>(array: &DictionaryArray<K>, op: F) -> Result<ArrayRef>
+where
+ K: ArrowNumericType,
+ T: ArrowPrimitiveType,
+ F: Fn(T::Native) -> T::Native,
+{
+ let dict_values = array
+ .values()
+ .as_any()
+ .downcast_ref::<PrimitiveArray<T>>()
+ .unwrap();
+
+ let values = dict_values
+ .iter()
+ .map(|v| v.map(|value| op(value)))
Review Comment:
Although the clippy complains about this
```rust
error: redundant closure
--> arrow/src/compute/kernels/arity.rs:101:24
|
101 | .map(|v| v.map(|value| op(value)))
| ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `op`
|
```
But I cannot follow the suggestion. The compiler fails to compile because `op` cannot moved.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] viirya commented on a diff in pull request #1990: Support DictionaryArray in unary kernel
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #1990:
URL: https://github.com/apache/arrow-rs/pull/1990#discussion_r915171084
##########
arrow/src/compute/kernels/arity.rs:
##########
@@ -103,8 +103,22 @@ where
.collect::<PrimitiveArray<T>>();
let keys = array.keys();
- let new_dict = DictionaryArray::<K>::try_new(keys, &values).unwrap();
+ let mut data = ArrayData::builder(array.data_type().clone())
Review Comment:
I thought about it. I think for mostly usage in query execution, input/output types are the same for `unary`. A difficulty to specify output type for `unary_dict` is, unlike `unary` where we can bind output type on the produced `PrimitiveArray<O>` and `F: Fn(T::Native) -> T::Native`. In `unary_dict`, the output type `O` cannot be bound on the produce dictionary array. So the compiler cannot infer it. Then we need to specify `O` when invoking `unary_dict`/`unary_dyn`. I feel it is harder to use. So if we only use it for same input/output types, I just leave it without a separate output type parameter.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] viirya commented on a diff in pull request #1990: Support DictionaryArray in unary kernel
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #1990:
URL: https://github.com/apache/arrow-rs/pull/1990#discussion_r914464137
##########
arrow/src/compute/kernels/arity.rs:
##########
@@ -78,10 +82,120 @@ where
PrimitiveArray::<O>::from(data)
}
+macro_rules! unary_dict_op {
+ ($array: expr, $op: expr, $value_ty: ty) => {{
+ // Safety justification: Since the inputs are valid Arrow arrays, all values are
+ // valid indexes into the dictionary (which is verified during construction)
+
+ let array_iter = unsafe {
+ $array
+ .values()
+ .as_any()
+ .downcast_ref::<$value_ty>()
+ .unwrap()
+ .take_iter_unchecked($array.keys_iter())
+ };
+
+ let values = array_iter.map(|v| v.map(|value| $op(value))).collect();
+
+ Ok(values)
+ }};
+}
+
+/// A helper function that applies an unary function to a dictionary array with primitive value type.
+fn unary_dict<K, F, T>(array: &DictionaryArray<K>, op: F) -> Result<PrimitiveArray<T>>
Review Comment:
They will use `unary_dyn` directly.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] viirya commented on a diff in pull request #1990: Support DictionaryArray in unary kernel
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #1990:
URL: https://github.com/apache/arrow-rs/pull/1990#discussion_r914465665
##########
arrow/src/compute/kernels/arity.rs:
##########
@@ -78,10 +82,120 @@ where
PrimitiveArray::<O>::from(data)
}
+macro_rules! unary_dict_op {
+ ($array: expr, $op: expr, $value_ty: ty) => {{
+ // Safety justification: Since the inputs are valid Arrow arrays, all values are
+ // valid indexes into the dictionary (which is verified during construction)
+
+ let array_iter = unsafe {
+ $array
+ .values()
+ .as_any()
+ .downcast_ref::<$value_ty>()
+ .unwrap()
+ .take_iter_unchecked($array.keys_iter())
Review Comment:
I thought it too, but didn't try. Let me try if it is feasible.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] viirya merged pull request #1990: Support DictionaryArray in unary kernel
Posted by GitBox <gi...@apache.org>.
viirya merged PR #1990:
URL: https://github.com/apache/arrow-rs/pull/1990
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] viirya commented on a diff in pull request #1990: Support DictionaryArray in unary kernel
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #1990:
URL: https://github.com/apache/arrow-rs/pull/1990#discussion_r914480820
##########
arrow/src/compute/kernels/arity.rs:
##########
@@ -78,10 +82,120 @@ where
PrimitiveArray::<O>::from(data)
}
+macro_rules! unary_dict_op {
+ ($array: expr, $op: expr, $value_ty: ty) => {{
+ // Safety justification: Since the inputs are valid Arrow arrays, all values are
+ // valid indexes into the dictionary (which is verified during construction)
+
+ let array_iter = unsafe {
+ $array
+ .values()
+ .as_any()
+ .downcast_ref::<$value_ty>()
+ .unwrap()
+ .take_iter_unchecked($array.keys_iter())
Review Comment:
I made the change in latest commit. You can see if this is okay.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] viirya commented on a diff in pull request #1990: Support DictionaryArray in unary kernel
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #1990:
URL: https://github.com/apache/arrow-rs/pull/1990#discussion_r915127448
##########
arrow/src/compute/kernels/arity.rs:
##########
@@ -78,10 +83,114 @@ where
PrimitiveArray::<O>::from(data)
}
+/// A helper function that applies an unary function to a dictionary array with primitive value type.
+#[allow(clippy::redundant_closure)]
+fn unary_dict<K, F, T>(array: &DictionaryArray<K>, op: F) -> Result<ArrayRef>
+where
+ K: ArrowNumericType,
+ T: ArrowPrimitiveType,
+ F: Fn(T::Native) -> T::Native,
+{
+ let dict_values = array
+ .values()
+ .as_any()
+ .downcast_ref::<PrimitiveArray<T>>()
+ .unwrap();
+
+ let values = dict_values
+ .iter()
+ .map(|v| v.map(|value| op(value)))
+ .collect::<PrimitiveArray<T>>();
+
+ let keys = array.keys();
+ let new_dict = DictionaryArray::<K>::try_new(keys, &values).unwrap();
Review Comment:
Seems `DictionaryArrya` doesn't provide a method like `try_new_unchecked`. We can just inline the code of preparing dictionary array data here.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] viirya commented on a diff in pull request #1990: Support DictionaryArray in unary kernel
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #1990:
URL: https://github.com/apache/arrow-rs/pull/1990#discussion_r914465222
##########
arrow/src/compute/kernels/arity.rs:
##########
@@ -78,10 +82,120 @@ where
PrimitiveArray::<O>::from(data)
}
+macro_rules! unary_dict_op {
+ ($array: expr, $op: expr, $value_ty: ty) => {{
+ // Safety justification: Since the inputs are valid Arrow arrays, all values are
+ // valid indexes into the dictionary (which is verified during construction)
+
+ let array_iter = unsafe {
+ $array
+ .values()
+ .as_any()
+ .downcast_ref::<$value_ty>()
+ .unwrap()
+ .take_iter_unchecked($array.keys_iter())
+ };
+
+ let values = array_iter.map(|v| v.map(|value| $op(value))).collect();
+
+ Ok(values)
+ }};
+}
+
+/// A helper function that applies an unary function to a dictionary array with primitive value type.
+fn unary_dict<K, F, T>(array: &DictionaryArray<K>, op: F) -> Result<PrimitiveArray<T>>
+where
+ K: ArrowNumericType,
+ T: ArrowPrimitiveType,
+ F: Fn(T::Native) -> T::Native,
+{
+ unary_dict_op!(array, op, PrimitiveArray<T>)
Review Comment:
ha, seems so.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] viirya commented on pull request #1990: Support DictionaryArray in unary kernel
Posted by GitBox <gi...@apache.org>.
viirya commented on PR #1990:
URL: https://github.com/apache/arrow-rs/pull/1990#issuecomment-1175582963
cc @sunchao I forgot this is not merged yet, I need to use this when implementing dictionary and scalar arithmetic kernels.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] viirya commented on pull request #1990: Support DictionaryArray in unary kernel
Posted by GitBox <gi...@apache.org>.
viirya commented on PR #1990:
URL: https://github.com/apache/arrow-rs/pull/1990#issuecomment-1176688904
Thanks @sunchao
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] sunchao commented on a diff in pull request #1990: Support DictionaryArray in unary kernel
Posted by GitBox <gi...@apache.org>.
sunchao commented on code in PR #1990:
URL: https://github.com/apache/arrow-rs/pull/1990#discussion_r915213846
##########
arrow/src/compute/kernels/arity.rs:
##########
@@ -103,8 +103,22 @@ where
.collect::<PrimitiveArray<T>>();
let keys = array.keys();
- let new_dict = DictionaryArray::<K>::try_new(keys, &values).unwrap();
+ let mut data = ArrayData::builder(array.data_type().clone())
Review Comment:
I see, make sense.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] viirya commented on pull request #1990: Support DictionaryArray in unary kernel
Posted by GitBox <gi...@apache.org>.
viirya commented on PR #1990:
URL: https://github.com/apache/arrow-rs/pull/1990#issuecomment-1175603958
Unrelated error:
```
################# FAILURES #################
FAILED TEST: datetime Java producing, C# consuming
```
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] sunchao commented on a diff in pull request #1990: Support DictionaryArray in unary kernel
Posted by GitBox <gi...@apache.org>.
sunchao commented on code in PR #1990:
URL: https://github.com/apache/arrow-rs/pull/1990#discussion_r915164844
##########
arrow/src/compute/kernels/arity.rs:
##########
@@ -103,8 +103,22 @@ where
.collect::<PrimitiveArray<T>>();
let keys = array.keys();
- let new_dict = DictionaryArray::<K>::try_new(keys, &values).unwrap();
+ let mut data = ArrayData::builder(array.data_type().clone())
Review Comment:
another question: similar to `unary`, do we plan to support different output value type than `T`?
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] sunchao commented on a diff in pull request #1990: Support DictionaryArray in unary kernel
Posted by GitBox <gi...@apache.org>.
sunchao commented on code in PR #1990:
URL: https://github.com/apache/arrow-rs/pull/1990#discussion_r914389732
##########
arrow/src/compute/kernels/arity.rs:
##########
@@ -78,10 +82,120 @@ where
PrimitiveArray::<O>::from(data)
}
+macro_rules! unary_dict_op {
+ ($array: expr, $op: expr, $value_ty: ty) => {{
+ // Safety justification: Since the inputs are valid Arrow arrays, all values are
+ // valid indexes into the dictionary (which is verified during construction)
+
+ let array_iter = unsafe {
+ $array
+ .values()
+ .as_any()
+ .downcast_ref::<$value_ty>()
+ .unwrap()
+ .take_iter_unchecked($array.keys_iter())
+ };
+
+ let values = array_iter.map(|v| v.map(|value| $op(value))).collect();
+
+ Ok(values)
+ }};
+}
+
+/// A helper function that applies an unary function to a dictionary array with primitive value type.
+fn unary_dict<K, F, T>(array: &DictionaryArray<K>, op: F) -> Result<PrimitiveArray<T>>
Review Comment:
does this need to be public so it can be used by other mods like `arithmetic.rs`?
##########
arrow/src/compute/kernels/arity.rs:
##########
@@ -78,10 +82,120 @@ where
PrimitiveArray::<O>::from(data)
}
+macro_rules! unary_dict_op {
+ ($array: expr, $op: expr, $value_ty: ty) => {{
+ // Safety justification: Since the inputs are valid Arrow arrays, all values are
+ // valid indexes into the dictionary (which is verified during construction)
+
+ let array_iter = unsafe {
+ $array
+ .values()
+ .as_any()
+ .downcast_ref::<$value_ty>()
+ .unwrap()
+ .take_iter_unchecked($array.keys_iter())
Review Comment:
Hmm, is it possible to directly apply the `op` on dictionary values? if values are large strings, the current approach will need to first decode the dictionary and convert it to a "plain" array, and then apply the `op` to each value in there, which is expensive.
##########
arrow/src/compute/kernels/arity.rs:
##########
@@ -78,10 +82,120 @@ where
PrimitiveArray::<O>::from(data)
}
+macro_rules! unary_dict_op {
+ ($array: expr, $op: expr, $value_ty: ty) => {{
+ // Safety justification: Since the inputs are valid Arrow arrays, all values are
+ // valid indexes into the dictionary (which is verified during construction)
+
+ let array_iter = unsafe {
+ $array
+ .values()
+ .as_any()
+ .downcast_ref::<$value_ty>()
+ .unwrap()
+ .take_iter_unchecked($array.keys_iter())
+ };
+
+ let values = array_iter.map(|v| v.map(|value| $op(value))).collect();
+
+ Ok(values)
+ }};
+}
+
+/// A helper function that applies an unary function to a dictionary array with primitive value type.
+fn unary_dict<K, F, T>(array: &DictionaryArray<K>, op: F) -> Result<PrimitiveArray<T>>
+where
+ K: ArrowNumericType,
+ T: ArrowPrimitiveType,
+ F: Fn(T::Native) -> T::Native,
+{
+ unary_dict_op!(array, op, PrimitiveArray<T>)
Review Comment:
do we need this macro? I think we can just inline it:
```rust
let array_iter = unsafe {
array
.values()
.as_any()
.downcast_ref::<PrimitiveArray<T>>()
.unwrap()
.take_iter_unchecked(array.keys_iter())
};
let values = array_iter.map(|v| v.map(|value| op(value))).collect();
Ok(values)
```
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] sunchao commented on a diff in pull request #1990: Support DictionaryArray in unary kernel
Posted by GitBox <gi...@apache.org>.
sunchao commented on code in PR #1990:
URL: https://github.com/apache/arrow-rs/pull/1990#discussion_r915072761
##########
arrow/src/compute/kernels/arity.rs:
##########
@@ -78,10 +83,114 @@ where
PrimitiveArray::<O>::from(data)
}
+/// A helper function that applies an unary function to a dictionary array with primitive value type.
+#[allow(clippy::redundant_closure)]
+fn unary_dict<K, F, T>(array: &DictionaryArray<K>, op: F) -> Result<ArrayRef>
+where
+ K: ArrowNumericType,
+ T: ArrowPrimitiveType,
+ F: Fn(T::Native) -> T::Native,
+{
+ let dict_values = array
Review Comment:
👍
##########
arrow/src/compute/kernels/arity.rs:
##########
@@ -78,10 +83,114 @@ where
PrimitiveArray::<O>::from(data)
}
+/// A helper function that applies an unary function to a dictionary array with primitive value type.
+#[allow(clippy::redundant_closure)]
+fn unary_dict<K, F, T>(array: &DictionaryArray<K>, op: F) -> Result<ArrayRef>
+where
+ K: ArrowNumericType,
+ T: ArrowPrimitiveType,
+ F: Fn(T::Native) -> T::Native,
+{
+ let dict_values = array
+ .values()
+ .as_any()
+ .downcast_ref::<PrimitiveArray<T>>()
+ .unwrap();
+
+ let values = dict_values
+ .iter()
+ .map(|v| v.map(|value| op(value)))
+ .collect::<PrimitiveArray<T>>();
+
+ let keys = array.keys();
+ let new_dict = DictionaryArray::<K>::try_new(keys, &values).unwrap();
Review Comment:
```suggestion
let new_dict = DictionaryArray::<K>::try_new(keys, &values)?;
```
##########
arrow/src/compute/kernels/arity.rs:
##########
@@ -78,10 +83,114 @@ where
PrimitiveArray::<O>::from(data)
}
+/// A helper function that applies an unary function to a dictionary array with primitive value type.
+#[allow(clippy::redundant_closure)]
+fn unary_dict<K, F, T>(array: &DictionaryArray<K>, op: F) -> Result<ArrayRef>
+where
+ K: ArrowNumericType,
+ T: ArrowPrimitiveType,
+ F: Fn(T::Native) -> T::Native,
+{
+ let dict_values = array
+ .values()
+ .as_any()
+ .downcast_ref::<PrimitiveArray<T>>()
+ .unwrap();
+
+ let values = dict_values
+ .iter()
+ .map(|v| v.map(|value| op(value)))
+ .collect::<PrimitiveArray<T>>();
+
+ let keys = array.keys();
+ let new_dict = DictionaryArray::<K>::try_new(keys, &values).unwrap();
Review Comment:
is there anyway to avoid the `validate` calls for key & values in `try_new`? it seems not that necessary for this case.
##########
arrow/src/compute/kernels/arity.rs:
##########
@@ -78,10 +83,114 @@ where
PrimitiveArray::<O>::from(data)
}
+/// A helper function that applies an unary function to a dictionary array with primitive value type.
+#[allow(clippy::redundant_closure)]
+fn unary_dict<K, F, T>(array: &DictionaryArray<K>, op: F) -> Result<ArrayRef>
+where
+ K: ArrowNumericType,
+ T: ArrowPrimitiveType,
+ F: Fn(T::Native) -> T::Native,
+{
+ let dict_values = array
+ .values()
+ .as_any()
+ .downcast_ref::<PrimitiveArray<T>>()
+ .unwrap();
+
+ let values = dict_values
+ .iter()
+ .map(|v| v.map(|value| op(value)))
Review Comment:
yea I tried the same too - this should be fine.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] codecov-commenter commented on pull request #1990: Support DictionaryArray in unary kernel
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1990:
URL: https://github.com/apache/arrow-rs/pull/1990#issuecomment-1172926966
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1990?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1990](https://codecov.io/gh/apache/arrow-rs/pull/1990?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fdca684) into [master](https://codecov.io/gh/apache/arrow-rs/commit/ccddaa83c989c23c9af819b2dc60c47dd0abe526?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ccddaa8) will **decrease** coverage by `0.00%`.
> The diff coverage is `69.38%`.
```diff
@@ Coverage Diff @@
## master #1990 +/- ##
==========================================
- Coverage 83.57% 83.57% -0.01%
==========================================
Files 221 222 +1
Lines 57482 57544 +62
==========================================
+ Hits 48043 48090 +47
- Misses 9439 9454 +15
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/1990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [arrow/src/compute/kernels/arity.rs](https://codecov.io/gh/apache/arrow-rs/pull/1990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy9hcml0eS5ycw==) | `76.05% <69.38%> (-14.86%)` | :arrow_down: |
| [...row/src/array/builder/string\_dictionary\_builder.rs](https://codecov.io/gh/apache/arrow-rs/pull/1990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2J1aWxkZXIvc3RyaW5nX2RpY3Rpb25hcnlfYnVpbGRlci5ycw==) | `91.36% <0.00%> (-0.37%)` | :arrow_down: |
| [parquet\_derive/src/parquet\_field.rs](https://codecov.io/gh/apache/arrow-rs/pull/1990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldF9kZXJpdmUvc3JjL3BhcnF1ZXRfZmllbGQucnM=) | `65.75% <0.00%> (-0.23%)` | :arrow_down: |
| [arrow/src/array/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/1990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L21vZC5ycw==) | `100.00% <0.00%> (ø)` | |
| [arrow/src/array/array\_decimal.rs](https://codecov.io/gh/apache/arrow-rs/pull/1990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2FycmF5X2RlY2ltYWwucnM=) | `93.67% <0.00%> (ø)` | |
| [arrow/src/array/array\_primitive.rs](https://codecov.io/gh/apache/arrow-rs/pull/1990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2FycmF5X3ByaW1pdGl2ZS5ycw==) | `95.81% <0.00%> (+0.03%)` | :arrow_up: |
| [arrow/src/array/array\_string.rs](https://codecov.io/gh/apache/arrow-rs/pull/1990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2FycmF5X3N0cmluZy5ycw==) | `97.76% <0.00%> (+0.03%)` | :arrow_up: |
| [arrow/src/array/data.rs](https://codecov.io/gh/apache/arrow-rs/pull/1990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2RhdGEucnM=) | `84.54% <0.00%> (+0.13%)` | :arrow_up: |
| [arrow/src/array/array\_binary.rs](https://codecov.io/gh/apache/arrow-rs/pull/1990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2FycmF5X2JpbmFyeS5ycw==) | `94.20% <0.00%> (+0.16%)` | :arrow_up: |
| [arrow/src/array/builder/primitive\_builder.rs](https://codecov.io/gh/apache/arrow-rs/pull/1990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2J1aWxkZXIvcHJpbWl0aXZlX2J1aWxkZXIucnM=) | `93.11% <0.00%> (+0.32%)` | :arrow_up: |
| ... and [2 more](https://codecov.io/gh/apache/arrow-rs/pull/1990/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1990?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1990?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ccddaa8...fdca684](https://codecov.io/gh/apache/arrow-rs/pull/1990?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org