add support for Optional/None values to vector/vam#6700
Merged
Conversation
This commit changes the fields of vector.Record back to vector.Any
and introduces the concept of vector.Optional and vector.None.
An Optional is simply a two-typed Dynamic of the original values
plus the nones where the nones are represnted by vector.None which
wraps a vector.Error("missing"). This means that None expresses
itself as Missing when referenced unless asserted to None and
treated otherwise.
There is also a performance issue in that we always load tags
of Optionals when reading optional fields in vcache and we always
compute the tags from the runlens in recordbuidler. In a future PR,
we will compute the tags on demand under a lock.
There are still problems in both sam/vam with put/cut and
problems in vam with None turning into Missing in aggregate functions.
These issues will be addressed in subsequent PRs.
mattnibs
reviewed
Mar 6, 2026
| } | ||
|
|
||
| func (u *unionBuilder) Build() Any { | ||
| func (u *unionBuilder) Build(sctx *super.Context) Any { |
Collaborator
There was a problem hiding this comment.
nit: If the argument isn't being used we typically do this but whateves:
Suggested change
| func (u *unionBuilder) Build(sctx *super.Context) Any { | |
| func (u *unionBuilder) Build(_ *super.Context) Any { |
Collaborator
Author
There was a problem hiding this comment.
It's used here in the recursive call...
mattnibs
approved these changes
Mar 6, 2026
mattnibs
reviewed
Mar 6, 2026
vector/record.go
Outdated
| return &Optional{NewDynamic(tags, []Any{vec, &None{NewMissing(sctx, noneLen)}})} | ||
| } | ||
|
|
||
| // An Optional value is a special Dynamic that has two tags comprosing the |
Collaborator
There was a problem hiding this comment.
Suggested change
| // An Optional value is a special Dynamic that has two tags comprosing the | |
| // An Optional value is a special Dynamic that has two tags comprising the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit changes the fields of vector.Record back to vector.Any and introduces the concept of vector.Optional and vector.None. An Optional is simply a two-typed Dynamic of the original values plus the nones where the nones are represnted by vector.None which wraps a vector.Error("missing"). This means that None expresses itself as Missing when referenced unless asserted to None and treated otherwise.
There is also a performance issue in that we always load tags of Optionals when reading optional fields in vcache and we always compute the tags from the runlens in recordbuidler. In a future PR, we will compute the tags on demand under a lock.
There are still problems in both sam/vam with put/cut and problems in vam with None turning into Missing in aggregate functions. These issues will be addressed in subsequent PRs.