API, Core, Parquet: Model the file type as its own nested type - #17821
Draft
RussellSpitzer wants to merge 27 commits into
Draft
API, Core, Parquet: Model the file type as its own nested type#17821RussellSpitzer wants to merge 27 commits into
RussellSpitzer wants to merge 27 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)
ORCSchemaUtil dispatches on Type.TypeID in two switches, one for writes and one for reads, and neither handled TypeID.FILE. A file column made every ORC write and every ORC read fail with "Unhandled type FILE" before a single row was processed. Map a file column to an ORC struct of its six derived fields tagged with an iceberg.struct-type value of FILE, mirroring how variant is tagged, so that OrcToIcebergVisitor can recover the file type from an ORC schema that has no accompanying table schema. Add the matching file() hooks to both ORC schema visitors and exclude file references from ORC search argument push down, as struct references already are. Generated-by: Cursor
HiveSchemaUtil.convertToTypeString had no arm for TypeID.FILE and fell through to a throwing default, so a HiveCatalog table with a file column could be neither created nor committed to: HiveOperationsBase rebuilds the storage descriptor on every commit. Render the file as the Hive struct of its six derived fields. Unlike variant, whose two binary fields are an opaque encoding that degrades to "unknown", every field of a file is a type Hive can represent, so the struct rendering keeps the Hive column readable. Generated-by: Cursor
RecordConverter.convertValue had no arm for TypeID.FILE, so a sink task failed with "Unsupported type: FILE" on any record carrying a value for a file column. Convert it as the struct of the file's nested fields, which is the value shape the generic writers already expect. Nested schema evolution reached a file column through an isStructType() guard that is false under design B and fell into the mismatched-type warning. State the skip explicitly instead: a file's nested fields are derived from the column, and SchemaUpdate rejects adding columns under one, so there is nothing to evolve. Generated-by: Cursor
IcebergToGlueConverter.toTypeString had no arm for TypeID.FILE, so its default recorded the Glue column type as the bare string "file". A Glue console user, or an Athena or Redshift Spectrum reader that consults the Glue column types, could not interpret the column. Render the struct of the file's nested fields, matching the struct arm. setTableInputInformation swallows RuntimeException and only logs, so a throwing arm would leave the Glue table silently missing its column metadata instead. Generated-by: Cursor
TypeToSparkType, Spark3Util.DescribeSchemaVisitor, and PruneColumnsWithoutReordering inherit a file() hook that throws, so any schema containing a file column failed to convert, describe, or prune. Each hook now delegates to its struct handling, using the file type's nested fields, because Spark has no type that can express a file. Generated-by: Cursor
The wrapper dispatches on the Spark type, so a file column reaches the struct branch and asStructType() throws Not a struct type: file. Because RowDataReader builds a SparkDeleteFilter for every task, and that filter builds a wrapper over the whole projection, this broke every row-based read of a file column even with no delete files. Generated-by: Cursor
AvroWithPartnerVisitor has no file() hook, so visitRecord passes a file partner into record(), where asStructType() throws Not a struct type: file. Use TypeUtil.asStructType, matching the fix already applied to GenericAvroReader and InternalReader in core. The new scan test covers a whole file column and a single nested field projection, in Parquet and Avro, end to end through Spark. Generated-by: Cursor
SparkTypeToType can only produce a struct for a Spark struct, so a write schema carries a plain struct where the table has a file. Id reassignment then failed with Not a struct: file, which named neither the column nor the reason. Reject the conversion up front with the column name and an explanation that Spark cannot express the file type. A write that omits the file column is still converted, so only writes that actually supply the column are rejected. Generated-by: Cursor
SparkUtil.internalToSpark fell through to its default arm for a file, returning a raw StructLike where Spark expects an InternalRow, and ConstantColumnVector cast a file straight to a struct. Neither is reachable today, because a file column can be neither a partition constant nor an initial default and it disables batch reads, but both would fail with a ClassCastException that names no column. Generated-by: Cursor
Flink has no logical type that carries file semantics, so TypeToFlinkType erases a file into a row of its six derived nested fields. RowDataWrapper now takes the struct view of a file instead of casting to StructType, and ReassignIds keeps a file type intact so that converting a Flink schema back to Iceberg against a reference schema recovers the file column. Generated-by: Cursor
FlinkSchemaVisitor routed a file into the primitive hook because its switch had no FILE case, and FlinkPlannedAvroReader asked a file for its struct type. Both now traverse the struct view of a file. RowDataProjection gains the same FILE case and accepts a struct project field for a file row field, which is what pruning a subset of a file's nested fields produces. Generated-by: Cursor
Flink erases a file into a row, so the dynamic sink compares an incoming struct against a file column of the table. isStructType is false for a file, so the visitor reported that a schema update was needed for a column that already matched, and the sink issued an UpdateSchema commit on every batch that could never converge. The struct hook now accepts a file partner and compares against its struct view, fieldPartner resolves a file parent, and a file hook handles an incoming schema that carries a file type of its own. Generated-by: Cursor
The struct hook asked a file partner for its struct type and threw. A file column is left alone instead: UpdateSchema rejects adding, reordering, and retyping the derived nested fields of a file, so a mismatch between those fields and the incoming struct cannot be evolved away and is reported with the column name rather than accumulated into a commit that always fails. Generated-by: Cursor
Flink cannot express a file type, so a Flink row carries a plain struct where the table has a file. Writing it would silently produce a struct or fail with an unqualified ClassCastException from RowDataWrapper, so the writer factory rejects the schema up front and names the file columns. Generated-by: Cursor
The pre-commit hook runs spotlessApply for the default Spark version only, so the 3.5 and 4.0 copies were left unformatted. Generated-by: Cursor
danielcweeks
self-requested a review
August 27, 2026 21:45
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.