Add UnionVariants type and embed it in DType::Union#8703
Conversation
Merging this PR will not alter performance
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | slice_empty_vortex |
310.3 ns | 368.6 ns | -15.83% |
| ❌ | Simulation | rebuild_naive |
91.4 µs | 102.1 µs | -10.46% |
| ❌ | Simulation | chunked_varbinview_opt_canonical_into[(100, 100)] |
305.4 µs | 340.5 µs | -10.31% |
| ❌ | Simulation | encode_varbin[(1000, 512)] |
190.4 µs | 212 µs | -10.19% |
| ⚡ | Simulation | chunked_varbinview_canonical_into[(1000, 10)] |
191 µs | 154.6 µs | +23.56% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing ct/union-variants-v2 (1de4f39) with develop (a649d75)
Footnotes
-
42 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
| /// assert_eq!(variants.len(), 2); | ||
| /// assert_eq!(variants.type_ids(), &[0, 1]); | ||
| /// ``` | ||
| #[allow( |
There was a problem hiding this comment.
for some reason expect doesn't work here
| impl<'de> DeserializeSeed<'de> for DTypeSerde<'_, UnionVariants> { | ||
| type Value = UnionVariants; |
There was a problem hiding this comment.
this is all llm generated, I have no idea if it is correct
Replaces `DType::Union(Nullability)` with `DType::Union(UnionVariants, Nullability)`. `UnionVariants` carries a `FieldNames` list, per-variant `FieldDType`s, and an `i8` type-id vector (supporting non-consecutive tags like `[0, 5, 7]` so children can be removed without renumbering). The flatbuffer and protobuf `Union` schemas are extended with the `names`/`dtypes`/`type_ids` fields, and serde, flatbuffers, and protobuf paths are filled in with round-trip tests and nullability-constraint checks. The flatbuffer `Union` table uses explicit field ids so that `nullable` keeps the id it had before this change, keeping the schema conformant with develop. Existing `DType::Union(..)` match arms that reject or skip unions are left in place; concrete implementations land alongside the array work. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
connortsui20
force-pushed
the
ct/union-variants-v2
branch
from
93cfc14 to
1de4f39
Compare
2 participants
Rationale for this change
Tracking Issue: #7882
We want to make
DType::Unionusable. This PR has been resurrected from #7902.What changes are included in this PR?
Replaces
DType::Union(Nullability)withDType::Union(UnionVariants, Nullability).UnionVariantscarries aFieldNameslist, per-variantFieldDTypes, and ani8type-id vector (supporting non-consecutive tags like[0, 5, 7]so children can be removed without renumbering).The flatbuffer and protobuf
Unionschemas are extended with thenames/dtypes/type_idsfields, and serde, flatbuffers, and protobuf paths are filled in with round-trip tests and nullability-constraint checks. The flatbufferUniontable uses explicit field ids so thatnullablekeeps the id it had before this change, keeping the schema conformant with develop.Existing
DType::Union(..)match arms that reject or skip unions are left in place, and concrete implementations will come later.What APIs are changed? Are there any user-facing changes?
DType::Unionwill now have a type parameterUnionVariants.