nevi-me commented on a change in pull request #389:
URL: https://github.com/apache/arrow-rs/pull/389#discussion_r668163430



##########
File path: arrow/src/array/array_struct.rs
##########
@@ -85,12 +85,7 @@ impl From<ArrayData> for StructArray {
     fn from(data: ArrayData) -> Self {
         let mut boxed_fields = vec![];
         for cd in data.child_data() {
-            let child_data = if data.offset() != 0 || data.len() != cd.len() {

Review comment:
       > This seems to make sense -- my main interest was in getting the tests 
I added (concat on sliced struct arrays) to pass because I had run into 
problems with that. It seems like the tests are still here and passing, and it 
sounds like this may be more robust as well (addressing other issues I've hit 
when slicing StructArray such as the mentioned equality issues).
   
   Yes, that's correct. I relied on your tests to verify that my changes make 
sense.
   
   > For my own curiosity -- If I understand correctly, slicing the child data 
doesn't actually copy the data, but just creates a reference to the same data 
with different offsets, so this shouldn't affect performance significantly, 
correct?
   
   Yes. The cost should be negligible as we're cloning a bunch of arcs 
individually instead of all at once




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


Reply via email to