You are viewing a plain text version of this content. The canonical link for it is here.

Posted to github@arrow.apache.org by "izveigor (via GitHub)" <gi...@apache.org> on 2023/07/11 09:37:37 UTC

[GitHub] [arrow-rs] izveigor opened a new issue, #4506: Support `into_builder` and `builder` methods for all Arrays

izveigor opened a new issue, #4506:
URL: https://github.com/apache/arrow-rs/issues/4506

   **Is your feature request related to a problem or challenge? Please describe what you are trying to do.**
   In my opinion, the methods `into_builder` and `builder` are pretty good at production. For instance, if we want to change or insert some element in an array, then we can translate it into the appropriate builder and perform these operations.
   
   Table of implemented and non-implemented methods:
   | Array | builder | into_builder |
   | ----- | ------- | ------------ |
   | ArrayData | Yes | Yes |
   | BooleanArray | Yes |No|
   | DictionaryArray | No | No |
   | FixedSizeBinaryArray | No | No |
   | FixedSizeListArray | No | No |
   | GenericByteArray | No | Yes |
   | GenericListArray | No | No |
   | MapArray | No | No |
   | MutableArrayData | No | No |
   | NullArray | Yes | No |
   | PrimitiveArray | Yes | Yes |
   | RunArray | No | No |
   | StructArray | No | No |
   | UnionArray | No | No |
   
   
   **Describe the solution you'd like**
   <!--
   A clear and concise description of what you want to happen.
   -->
   
   **Describe alternatives you've considered**
   <!--
   A clear and concise description of any alternative solutions or features you've considered.
   -->
   
   **Additional context**
   <!--
   Add any other context or screenshots about the feature request 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.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-rs] izveigor commented on issue #4506: Support `into_builder` and `builder` methods for all Arrays

Posted by "izveigor (via GitHub)" <gi...@apache.org>.

izveigor commented on issue #4506:
URL: https://github.com/apache/arrow-rs/issues/4506#issuecomment-1633028638

   Thanks for answer, @tustvold. Now I understand some aspects.
   Perhaps I need to learn more about many features of Arrow :)
   > The array into_builder methods are fallible and can only be used where it can be guaranteed there are no shared references to the underlying buffers, I suspect they will therefore be of limited utility to DataFusion which is not setup to provide these guarantees
   
   The argument is clear to me, but there are some arrays for which this function is not implemented and is not dangerous: `BooleanArray`, `FixedSizeBinaryArray`, `NullArray`. Do we need to implement these methods for them, or is it still not worth adding extra code?


-- 
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] izveigor commented on issue #4506: Support `into_builder` and `builder` methods for all Arrays

Posted by "izveigor (via GitHub)" <gi...@apache.org>.

izveigor commented on issue #4506:
URL: https://github.com/apache/arrow-rs/issues/4506#issuecomment-1632812987

   Hello, @tustvold!
   
   > I'm not really sure of the utility of adding builder methods, it seems to just add code?
   
   I can't quite call it adding code.
   The thing is that with further implementations of array functions in Arrow DataFusion, several methods may be required, in particular, transfer from `Array` to `Builder`.
   
   Here is a list of possible methods:
   - `builder` (not sure);
   - `into_builder`;
   - all possible `append_` functions;
   - `from_value`;
   
   And also a list of possible arrays in which these methods should be implemented:
   - `BooleanArray`;
   - `FixedSizeList`;
   - `GenericListArray`;
   - `GenericByteArray`;
   - `FixedSizeBinary`;
   - `PrimitiveArray`;
   - `StructArray`;
   - `NullArray` (it is a special case);
   
   Probably, I incorrectly set the goals of the issue and hastened to generalize. If needed, I can create a separate issue with a clearer wording.
   P.S. I can't know for sure if all of these methods can come in handy.
   


-- 
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] tustvold commented on issue #4506: Support `into_builder` and `builder` methods for all Arrays

Posted by "tustvold (via GitHub)" <gi...@apache.org>.

tustvold commented on issue #4506:
URL: https://github.com/apache/arrow-rs/issues/4506#issuecomment-1632875068

   So a couple of comments in no particular order:
   
   * The destructing APIs added as part of #3879 likely supercede many of these APIs, including from_value
   * ArrayData::into_builder is different from the array into_builder, in that it copies the data
   * The array into_builder methods are fallible and can only be used where it can be guaranteed there are no shared references to the underlying buffers, I suspect they will therefore be of limited utility to DataFusion which is not setup to provide these guarantees
   * I'm not sure into_builder is tractable for ListBuilder, etc... as the builders are generic on the child builders, whereas the arrays are not
   * I don't really see the value of having `*Array::builder` methods, they are imo a historical artifact, I'm not sure why you wouldn't just use the builder constructors directly
   
   > further implementations of array functions in Arrow DataFusion, several methods may be required, in particular, transfer from Array to Builder.
   
   I would be surprised to see the array kernels making use of builders, instead I would expect the use of the arrow-select kernels or MutableArrayData. This would not only be simpler, but would avoid problems around recursive type definitions. Perhaps you could link to what you are trying to achieve and I can advise?


-- 
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] tustvold commented on issue #4506: Support `into_builder` and `builder` methods for all Arrays

Posted by "tustvold (via GitHub)" <gi...@apache.org>.

tustvold commented on issue #4506:
URL: https://github.com/apache/arrow-rs/issues/4506#issuecomment-1631770869

   I'm not really sure of the utility of adding builder methods, it seems to just add code?


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