Repository navigation
Write Widen arrays through an array plugin - #11002
connortsui20 wants to merge 2 commits into
Conversation
Merging this PR will degrade performance by 7.89%1 warning
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing |
de8a7a6 to
f3d53cc
Compare
Signed-off-by: Connor Tsui <[email protected]>
Signed-off-by: Connor Tsui <[email protected]>
f3d53cc to
72749e5
Compare
| fn serialize( | ||
| &self, | ||
| array: &ArrayRef, | ||
| session: &VortexSession, | ||
| ) -> VortexResult<Option<ArraySerialization>> { | ||
| // TODO(connor)[Widen]: Add `vortex.widen` to both ID lists when it gets a wire form | ||
| // (#10733), and write it only where the writer permits it. This method cannot see the | ||
| // permitted IDs today. | ||
|
|
||
| // The child validity can be lazy, so the writer needs recursive canonicalization to | ||
| // produce nodes that it can store. | ||
| let canonical = array | ||
| .clone() | ||
| .execute::<RecursiveCanonical>(&mut session.create_execution_ctx())? | ||
| .0 | ||
| .into_array(); | ||
|
|
||
| session.array_serialize(&canonical) | ||
| } |
There was a problem hiding this comment.
Here is an open question: instead of doing this, do we want to have a more generalized version of this fix?
Not saying the PR should be blocked on this (as I do need to make progress on this stack), but there exist other encodings that we only have in-memory that we would want to ensure do not get serialized to disk (either because of an edition, or because we do not want them to be serialized ever).
Some examples:
FilterSlicePatched(the LLM is even telling me that this is a bug, andPatchedneeds a similar change as this one)ScalarFn(which we serialize now but imo we might want a different name for serialized scalar functions, see Tracking Issue: Encodings vs Scalar Functions #9156)- Any encoding that might easily be created in memory but we disallow via editions
Tracking Issue: #10701
Stacked on #11057. #10946 builds on it. No release gate.
Summary
Widenhas no wire form yet (#10733), but IPC and writes without a compressor still need to store it. A writer plugin makes that work through the same session API used by direct metadata callers.Changes
An integer
Widenwrites its canonical primitive form. Recursive canonicalization makes lazy child validity storable, while keeping theWidenas the serialization source preserves statistics in its logical dtype. Restricted contexts need only the existing primitive wire ID. Decimal writes follow in #10946.API Changes
Adds public
vortex_array::arrays::widen::WidenPluginto the defaultArraySession. Empty array sessions can register it explicitly. The plugin registers no reader becausevortex.widenhas no wire form.