You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kawadakk (via GitHub)" <gi...@apache.org> on 2023/07/31 09:04:29 UTC

[GitHub] [arrow-rs] kawadakk commented on a diff in pull request #4598: fix(data): map type ID to child index before indexing a union child array

kawadakk commented on code in PR #4598:
URL: https://github.com/apache/arrow-rs/pull/4598#discussion_r1279000554


##########
arrow-data/src/transform/union.rs:
##########
@@ -48,14 +51,18 @@ pub(super) fn build_extend_dense(array: &ArrayData) -> Extend {
                 .extend_from_slice(&type_ids[start..start + len]);
 
             (start..start + len).for_each(|i| {
-                let type_id = type_ids[i] as usize;
+                let type_id = type_ids[i];
+                let child_index = src_fields

Review Comment:
   Actually I implemented that way in [an earlier version](https://github.com/apache/arrow-rs/commit/5c9c004443c8244614111160c2f579245b15fa75), but then realized that it might be counterproductive and excessively memory-intensive if the number of rows per input array is very low, and the number of input arrays is high (as in my use case). [`equal_dense`](https://github.com/apache/arrow-rs/blob/95683439fa4108c036e48b334f8bed898b87a9b9/arrow-data/src/equal/union.rs#L41-L50) is implemented in the naïve way as well.



##########
arrow-data/src/transform/union.rs:
##########
@@ -48,14 +51,18 @@ pub(super) fn build_extend_dense(array: &ArrayData) -> Extend {
                 .extend_from_slice(&type_ids[start..start + len]);
 
             (start..start + len).for_each(|i| {
-                let type_id = type_ids[i] as usize;
+                let type_id = type_ids[i];
+                let child_index = src_fields

Review Comment:
   Actually I implemented that way in [an earlier version](https://github.com/apache/arrow-rs/commit/5c9c004443c8244614111160c2f579245b15fa75), but then realized that it might be counterproductive and excessively memory-intensive if the number of rows per input array is very low, and the number of input arrays is high (as in my use case). [`equal_dense`](https://github.com/apache/arrow-rs/blob/95683439fa4108c036e48b334f8bed898b87a9b9/arrow-data/src/equal/union.rs#L41-L50) is implemented in the naïve way as well.



-- 
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