add option types#6881
Merged
Merged
Conversation
This commit replaces the previous optional field design with
legit option types. This model follows the Rust pattern where
optional values must be unwrapped and dealt with when encountered.
Option types are implemented as a union type that contains type none.
A "some" value is a non-none value coded in an option union and a
"none" value is a type none value coded in an option value.
None values should never be serialized by themselves but the
formats support such serialization for debugging etc.
When we add strict type checking, option values that aren't unwrapped
will cause the query compilation fail with a type check error.
The BSUP and CSUP formats have changed to support this and their
version numbers bumped.
The SUP formatter formats record fields with optional types with
a trailing "?" and for types of the form "T|none", then union syntax
is dropped (even though a union is implied). If we so decide,
we can implement this for larger option unions and also for
fusions that embed an option supertype. For now, we will deal
with the noisier syntax and see how it feels.
The vector.Option and CSUP/vcache counterparts are not finished and
instead this commit codes option values as full-blown unions. In
a subsequent PR, we will add support for run-length encoding of
option values, which is needed for decent performance by fjson etc
when fusion creates wide records. We left behind some comments and
stubs that will be utilized in the vector.Option PR.
Now that we have option types, we will reconsider how error("missing")
and error("quiet") and related support functionality works in
a forthcoming PR.
This change breaks the Parquet writer that used to work for optional
fields since it can't handle the none type. In a subsequent PR,
we will fix this by treating none as null in Parquet.
Docs updates for option types and the format changes will be
merged in a future PR.
Co-authored-by: Noah Treuhaft <noah.treuhaft@gmail.com>
nwt
approved these changes
May 1, 2026
| errs := c.popErrs() | ||
| if !valid { | ||
| c.keepErrs(errs) | ||
| c.keepErrs(errs[:1]) |
Member
There was a problem hiding this comment.
What's the reasoning behind this change?
Collaborator
Author
There was a problem hiding this comment.
Duplicate errors over multiple union types. Best to just report one. The checker needs another overall scrubbing with a bit of refactoring to make this all a bit easier.
mattnibs
reviewed
May 1, 2026
Comment on lines
+98
to
+99
| case id == super.IDNone: | ||
| return &Const{NewNone(length), length} |
Collaborator
There was a problem hiding this comment.
I'm sure you did this get test passing but I don't think we should be doing this
Collaborator
Author
There was a problem hiding this comment.
I agree and didn't want to put that there but I don't know that it's quite factored the right way. We can fix it later.
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
3 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 replaces the previous optional field design with legit option types. This model follows the Rust pattern where optional values must be unwrapped and dealt with when encountered. Option types are implemented as a union type that contains type none. A "some" value is a non-none value coded in an option union and a "none" value is a type none value coded in an option value. None values should never be serialized by themselves but the formats support such serialization for debugging etc.
When we add strict type checking, option values that aren't unwrapped will cause the query compilation fail with a type check error.
The BSUP and CSUP formats have changed to support this and their version numbers bumped.
The SUP formatter formats record fields with optional types with a trailing "?" and for types of the form "T|none", then union syntax is dropped (even though a union is implied). If we so decide, we can implement this for larger option unions and also for fusions that embed an option supertype. For now, we will deal with the noisier syntax and see how it feels.
The vector.Option and CSUP/vcache counterparts are not finished and instead this commit codes option values as full-blown unions. In a subsequent PR, we will add support for run-length encoding of option values, which is needed for decent performance by fjson etc when fusion creates wide records. We left behind some comments and stubs that will be utilized in the vector.Option PR.
Now that we have option types, we will reconsider how error("missing") and error("quiet") and related support functionality works in a forthcoming PR.
This change breaks the Parquet writer that used to work for optional fields since it can't handle the none type. In a subsequent PR, we will fix this by treating none as null in Parquet.
Docs updates for option types and the format changes will be merged in a future PR.