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/06/10 21:01:27 UTC
[GitHub] [arrow-rs] tustvold opened a new pull request, #1842: Strongly typed UnionBuilder
tustvold opened a new pull request, #1842:
URL: https://github.com/apache/arrow-rs/pull/1842
# Which issue does this PR close?
Follow on from #1591
# Rationale for this change
`mutable_buffer_to_builder` created an untyped back-door to creating a `BufferBuilder<T>` with arbitrary `MutableArray`. This makes it hard to reason about the invariants of the `MutableArray` belonging to a `BufferBuilder`.
# What changes are included in this PR?
This instead implements UnionArray using type-erasure, which is safer and less code.
# Are there any user-facing changes?
No
--
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 #1842: Implement UnionArray FieldData using Type Erasure
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1842:
URL: https://github.com/apache/arrow-rs/pull/1842#issuecomment-1152750422
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1842?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 [#1842](https://codecov.io/gh/apache/arrow-rs/pull/1842?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6b96f42) into [master](https://codecov.io/gh/apache/arrow-rs/commit/c396dfb5035d22e57717b6dd365486b76eb611bc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c396dfb) will **increase** coverage by `0.02%`.
> The diff coverage is `87.50%`.
> :exclamation: Current head 6b96f42 differs from pull request most recent head 0b464c0. Consider uploading reports for the commit 0b464c0 to get more accurate results
```diff
@@ Coverage Diff @@
## master #1842 +/- ##
==========================================
+ Coverage 83.48% 83.50% +0.02%
==========================================
Files 201 201
Lines 56838 56806 -32
==========================================
- Hits 47452 47437 -15
+ Misses 9386 9369 -17
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/1842?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/buffer/mutable.rs](https://codecov.io/gh/apache/arrow-rs/pull/1842/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-YXJyb3cvc3JjL2J1ZmZlci9tdXRhYmxlLnJz) | `90.53% <ø> (ø)` | |
| [arrow/src/array/builder.rs](https://codecov.io/gh/apache/arrow-rs/pull/1842/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-YXJyb3cvc3JjL2FycmF5L2J1aWxkZXIucnM=) | `87.75% <87.50%> (+0.87%)` | :arrow_up: |
| [parquet\_derive/src/parquet\_field.rs](https://codecov.io/gh/apache/arrow-rs/pull/1842/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.98% <0.00%> (-0.23%)` | :arrow_down: |
| [parquet/src/encodings/encoding.rs](https://codecov.io/gh/apache/arrow-rs/pull/1842/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-cGFycXVldC9zcmMvZW5jb2RpbmdzL2VuY29kaW5nLnJz) | `93.46% <0.00%> (-0.20%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1842?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/1842?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 [c396dfb...0b464c0](https://codecov.io/gh/apache/arrow-rs/pull/1842?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
[GitHub] [arrow-rs] alamb commented on a diff in pull request #1842: Implement UnionArray FieldData using Type Erasure
Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #1842:
URL: https://github.com/apache/arrow-rs/pull/1842#discussion_r899459271
##########
arrow/src/array/builder.rs:
##########
@@ -1909,101 +1886,65 @@ struct FieldData {
/// The Arrow data type represented in the `values_buffer`, which is untyped
data_type: DataType,
/// A buffer containing the values for this field in raw bytes
- values_buffer: Option<MutableBuffer>,
+ values_buffer: Box<dyn FieldDataValues>,
/// The number of array slots represented by the buffer
slots: usize,
/// A builder for the null bitmap
bitmap_builder: BooleanBufferBuilder,
}
+/// A type-erased [`BufferBuilder`] used by [`FieldData`]
+trait FieldDataValues: std::fmt::Debug {
+ fn as_mut_any(&mut self) -> &mut dyn Any;
+
+ fn append_null(&mut self);
+
+ fn finish(&mut self) -> Buffer;
+}
+
+impl<T: ArrowNativeType> FieldDataValues for BufferBuilder<T> {
+ fn as_mut_any(&mut self) -> &mut dyn Any {
+ self
+ }
+
+ fn append_null(&mut self) {
+ self.advance(1)
+ }
+
+ fn finish(&mut self) -> Buffer {
+ self.finish()
+ }
+}
+
impl FieldData {
/// Creates a new `FieldData`.
- fn new(type_id: i8, data_type: DataType) -> Self {
+ fn new<T: ArrowPrimitiveType>(type_id: i8, data_type: DataType) -> Self {
Self {
type_id,
data_type,
- values_buffer: Some(MutableBuffer::new(1)),
slots: 0,
+ values_buffer: Box::new(BufferBuilder::<T::Native>::new(1)),
bitmap_builder: BooleanBufferBuilder::new(1),
}
}
/// Appends a single value to this `FieldData`'s `values_buffer`.
- #[allow(clippy::unnecessary_wraps)]
- fn append_to_values_buffer<T: ArrowPrimitiveType>(
- &mut self,
- v: T::Native,
- ) -> Result<()> {
- let values_buffer = self
- .values_buffer
- .take()
- .expect("Values buffer was never created");
- let mut builder: BufferBuilder<T::Native> =
- mutable_buffer_to_builder(values_buffer, self.slots);
- builder.append(v);
- let mutable_buffer = builder_to_mutable_buffer(builder);
- self.values_buffer = Some(mutable_buffer);
+ fn append_value<T: ArrowPrimitiveType>(&mut self, v: T::Native) {
+ self.values_buffer
+ .as_mut_any()
+ .downcast_mut::<BufferBuilder<T::Native>>()
+ .unwrap()
Review Comment:
```suggestion
.expect("Tried to append unexpected type")
```
--
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 merged pull request #1842: Implement UnionArray FieldData using Type Erasure
Posted by GitBox <gi...@apache.org>.
tustvold merged PR #1842:
URL: https://github.com/apache/arrow-rs/pull/1842
--
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