ArrayData Enumeration for Remaining Layouts #3769
Conversation
| @@ -270,7 +270,7 @@ impl<O: BytesOffset, B: Bytes> BytesArrayData<O, B> { | |||
|
|
|||
| /// Returns the offsets | |||
| #[inline] | |||
| pub fn value_offsets(&self) -> &[O] { | |||
| pub fn offsets(&self) -> &[O] { | |||
There was a problem hiding this comment.
The value_ prefix is redundant
| @@ -43,13 +43,13 @@ mod private { | |||
| } | |||
|
|
|||
| pub trait Primitive: private::PrimitiveSealed + ArrowNativeType { | |||
| const VARIANT: PrimitiveType; | |||
| const TYPE: PrimitiveType; | |||
There was a problem hiding this comment.
This makes it consistent with the other traits
| @@ -141,7 +140,7 @@ impl From<&DataType> for PhysicalType { | |||
| DataType::UInt64 => Self::Dictionary(DictionaryKeyType::UInt64), | |||
| d => panic!("illegal dictionary key data type {d}"), | |||
| }, | |||
| DataType::Map(_, _) => Self::Map, | |||
| DataType::Map(_, _) => Self::List(OffsetType::Int32), | |||
There was a problem hiding this comment.
MapArray is just a logical type over ListArray
tustvold
force-pushed
the
more-typed-array-data
branch
from
ad6d546
to
5aad042
Compare
tustvold
changed the title
| } | ||
|
|
||
| /// ArrayData for [run-end encoded arrays](https://arrow.apache.org/docs/format/Columnar.html#run-end-encoded-layout) | ||
| pub struct RunArrayData<E: RunEnd> { |
There was a problem hiding this comment.
This is a slight deviation from the spec as defined in https://arrow.apache.org/docs/format/Columnar.html#run-end-encoded-layout in that it "inlines" the run ends. This is consistent with RunEndArray and I think makes for a significantly less confusing abstraction.
tustvold
changed the title
|
Unless I hear anything further, I plan to merge this tomorrow morning. Please let me know if you need more time to review |
|
I will take some time looking at this today but if I cannot, it doesn't need to block it. |
arrow-data/src/data/dictionary.rs
Outdated
| } | ||
| } | ||
|
|
||
| /// Types of offset used by variable length byte arrays |
arrow-data/src/data/list.rs
Outdated
| } | ||
| } | ||
|
|
||
| /// Types of offset used by variable length byte arrays |
arrow-data/src/data/list.rs
Outdated
| /// # Safety | ||
| /// | ||
| /// - Each consecutive window of `offsets` must identify a valid slice of `child` | ||
| /// - `nulls.len() == offsets.len() + 1` |
There was a problem hiding this comment.
| /// - `nulls.len() == offsets.len() + 1` | |
| /// - `nulls.len() == offsets.len() - 1` |
?
arrow-data/src/data/union.rs
Outdated
| } | ||
| } | ||
|
|
||
| /// Returns the type ids for this array if this is a dense union |
There was a problem hiding this comment.
sparse union also has type ids.
| /// Returns the type ids for this array if this is a dense union | |
| /// Returns the type ids for this array |
|
Benchmark runs are scheduled for baseline = d440c24 and contender = 7852e76. 7852e76 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Part of #1799
Rationale for this change
Follow on to #3749 adds more types
What changes are included in this PR?
Are there any user-facing changes?