API, Core, Parquet: Model the file type as its own nested type - #17821
Draft
RussellSpitzer wants to merge 9 commits into
Draft
API, Core, Parquet: Model the file type as its own nested type#17821RussellSpitzer wants to merge 9 commits into
RussellSpitzer wants to merge 9 commits into
Conversation
Persist the type as "file" and expand it to a closed nested struct whose field IDs are derived from the enclosing field. Generated-by: Cursor Grok 4.6
ReassignDoc rebuilt every struct it visited, so reassigning docs turned a file column into a plain struct that no longer serializes as "file" or honors the format version gate. Return the file type unchanged there and in ReassignIds, matching the other assigners. The new two-argument GetID overload ignored the reservation request, so an implementation that did not override it could hand out IDs inside a file's derived block and produce duplicate field IDs with no error. Fail when the reservation cannot be honored. ReassignConflictingIds moved a field only when its own ID conflicted, so a file column kept an ID whose derived block overlapped IDs already in use. Move the column when any of its reserved IDs is unavailable. Also consolidate the helper that rebuilds a file type from a newly assigned ID into TypeUtil.assignedType, and drop the test prefix from the schema evolution tests added for this type. Generated-by: Cursor Claude Opus 5
Derived ID validation in the schema parser was only covered for struct fields and list elements. Add the map key and map value cases, along with a round trip for a file used as a map key. Add Parquet conversions for a required file column and for a file used as a list element and as a map value, plus a data round trip for a file inside a list. Record that reassigning a file column tracks only the enclosing ID, because the nested IDs are derived from it, and split the combined rename and delete test into one test per operation. Generated-by: Cursor Claude Opus 5
The file type had its own constant because the map was keyed by type ID and the file type reports STRUCT, so a STRUCT key would have gated every struct. Key the map by class instead, which identifies a logical type even when two of them share a type ID, and drop the separate constant so all minimum versions are declared in one place. Make the file type final so the class key is exact. Generated-by: Cursor Claude Opus 5
Passing zero reserved IDs used the argument as a sentinel for "do not reserve", which hid the fact that the overload exists only for types whose nested field IDs are derived. Branch on the type so the plain overload is used for everything else. Generated-by: Cursor Claude Opus 5
Tests for name resolution, Java serialization, format version gating, readability, projection, doc reassignment, accessors, and expression binding now live beside the code they exercise, so a change to those utilities surfaces the file type expectations. TestFileType keeps the type contract and the reserved ID block, which no existing class owns. Add coverage for selecting and filtering a file subfield. Drop tests that only re-exercised generic behavior: rejecting defaults applies to every nested type, and the list round trip is already covered by the list schema conversion plus the file round trip. Generated-by: Cursor
FileType.fieldId() returned the ID of the field that holds the type, not an ID of the type itself, which read as though it mirrored NestedField.fieldId(). Rename it to enclosingId() to match the name the parser already used for the same value. Report the short type name when a file and a struct are not interchangeable instead of formatting a whole struct into the error. Generated-by: Cursor
Reshape Types.FileType as a Type.NestedType sibling of StructType with its own TypeID.FILE, and add file() hooks to the schema visitor bases so that visitors opt in to file handling instead of inheriting struct behavior. Generated-by: Cursor (Claude Opus 4.6)
…iews Adding TypeID.FILE left the file type falling through switch defaults and failing unguarded asStructType() calls. Cover the reachable cases: - StructProjection threw when only some nested fields of a file were projected - JavaHash fell back to identity hashing instead of hashing nested fields - Comparators threw instead of comparing nested fields - IndexByName named list and map file elements with an extra element segment - SingleValueParser could not read or write a file default - PartitionData did not reject a file alongside other nested types - InternalRecordWrapper returned no wrapper for a file - the Avro read and write path threw on a file column Add TypeUtil.asStructType so the places that store and read a file as a group of its nested fields share one struct view. Generated-by: Cursor (Claude Opus 5)
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
1 participant
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.
An alternative implementation of the
filelogical type from #17808 which treats File as a new non-primitive special type. This is a little similar to Variant in that all visitors now have special handling for File type and you can't write a "bad" visitor which treats a File like a struct when it shouldn'd.Types.FileType extends StructType,typeId() == STRUCT.Visitors see a struct and behave correctly for free; the few places that must
not expand or rewrite a file carry
isFileType()guards.AI Disclosure
filelogical type as a standaloneNestedTypewith its own
TypeIDand per-visitorfile()hooks, as an alternative to theStructType-subclass approach in API, CORE, PARQUET: Implicit struct file type #17808, and measure the difference in filestouched, lines changed, engine breakage, and latent switch-statement risk.