Port row_ids to arrow1 #8657
Port row_ids to arrow1 #8657
Conversation
emilk
added
🏹 arrow
|
Web viewer built successfully. If applicable, you should also test it:
Note: This comment is updated whenever you push a commit. |
emilk
force-pushed
the
emilk/arrow-row-ids
branch
from
61e99f2 to
910269c
Compare
|
@rerun-bot full-check |
|
Started a full build: https://github.com/rerun-io/rerun/actions/runs/12726236739 |
| @@ -943,7 +934,11 @@ impl Chunk { | |||
| entity_path, | |||
| heap_size_bytes: Default::default(), | |||
| is_sorted: true, | |||
| row_ids: Arrow2StructArray::new_empty(RowId::arrow2_datatype()), | |||
| row_ids: arrow::array::StructBuilder::from_fields( | |||
There was a problem hiding this comment.
Here and elsewhere, it feels odd to me that we're going through a builder type just to build an empty thing.
Any particular reason we're not calling https://docs.rs/arrow/latest/arrow/array/fn.new_empty_array.html ?
There was a problem hiding this comment.
new_empty_array returns an ArrayRef, meaning we would need .as_any().downcast_ref::<StructArray>().unwrap().clone() here.
StructBuilder returns what we want.
| @@ -11,13 +11,18 @@ use crate::{DeserializationError, Loggable}; | |||
|
|
|||
| // --- | |||
|
|
|||
| // TODO(emilk): This is a bit ugly… but good enough for now? | |||
| pub fn tuid_arrow_fields() -> Fields { | |||
There was a problem hiding this comment.
Can't you get rid of this if you get rid of using the builder type for an empty array?
There was a problem hiding this comment.
Yes, but at the cost of some ugly dynamic casting and unwraps that I'd rather avoid
| #[inline] | ||
| fn heap_size_bytes(&self) -> u64 { | ||
| self.get_array_memory_size() as u64 | ||
| } | ||
| } | ||
|
|
||
| impl<T: ArrowPrimitiveType> SizeBytes for PrimitiveArray<T> { | ||
| impl<T: Array> SizeBytes for &T { |
There was a problem hiding this comment.
Method already takes &self:
| impl<T: Array> SizeBytes for &T { | |
| impl<T: Array> SizeBytes for T { |
Related
re_arrow2toarrow#3741