[PoC] Alternate implementation of Parquet ColumnOrder thrift union
#7930
+78
−20
Conversation
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
etseidl
commented
Jul 15, 2025
parquet/src/format.rs
Outdated
|
|
||
| impl ColumnOrderDisc { | ||
| pub const TYPE_ORDER : Self = Self(1); | ||
| pub const NO_SUCH_ORDER: Self = Self(2); |
There was a problem hiding this comment.
I added a fake order to demonstrate what a union with multiple fields would look like
etseidl
commented
Jul 15, 2025
| pub struct ColumnOrder { | ||
| pub disc: ColumnOrderDisc, | ||
| pub TYPE_ORDER: Option<TypeDefinedOrder>, | ||
| pub NO_SUCH_ORDER: Option<TypeDefinedOrder>, |
There was a problem hiding this comment.
Reused TypeDefinedOrder because I was too lazy to add another struct for the fake order.
|
closing in favor of #7995 to reduce clutter |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Which issue does this PR close?
Rationale for this change
Not for merging, but as a discussion point. This PR implements the thrift union
ColumnOrderas a rust struct rather than a rust enum, analogously to how the thrift rust generator handles thrift enums.The current implementation is not very ergonomic, but gets the point across.