ORC: fill initial defaults for missing id-bound fields - #263
Conversation
a888bee to
4db9002
Compare
bd16f75 to
d39cbfd
Compare
4db9002 to
cac8ff0
Compare
cac8ff0 to
1115244
Compare
d39cbfd to
4ca1b1e
Compare
Fill a field's initial-default when it is absent from an id-bearing ORC file, mirroring how the Parquet reader resolves defaults. buildOrcProjection omits an absent field that declares a default when the file's embedded ids are trustworthy, so the id-binding StructReader finds no column for it and materializes the declared default as a per-file constant. This reuses the reader's existing constant path, so a defaulted field consumes no column vector. A present column always wins, including one holding an explicit null. Id-less and name-mapped files keep legacy null synthesis, so a default is never name-matched onto a legacy or migrated file. Engines that have not opted into id binding are unaffected: default filling is enabled only by passing a constant converter, and the positional path keeps its strict missing-reader failure. Co-authored-by: Cursor <cursoragent@cursor.com>
1115244 to
0e7b860
Compare
| final Map<Integer, OrcField> icebergToOrc = icebergToOrcMapping("root", originalOrcSchema); | ||
| return buildOrcProjection(Integer.MIN_VALUE, schema.asStruct(), true, icebergToOrc); | ||
| return buildOrcProjection( | ||
| Integer.MIN_VALUE, schema.asStruct(), true, hasTrustedIds, icebergToOrc); |
There was a problem hiding this comment.
name is a bit vague so I changed it to a two-valued enum, which names where the field ids in the schema came from.
- EMBEDDED: the ids were read from iceberg.id column attributes persisted in the ORC file. When it is present it is great, a field with no id was genuinely never written, so its declared default can be filled safely.
- NAME_MAPPED: the file carried no ids of its own (e.g. was renamed from kafka ETL's Hive-migration), so an earlier step derived them at read time by matching column names. When file carries no ids, "absent" is not proof since e.g. an iceberg column can rename and the name mapping no longer covers.but it's data is physically in the file. Filling a default there would fabricate values over real data, so we don't fill the default on this path. We keep the original behavior here, in order to focus only on the important case: iceberg created files.
| this.readers[pos] = fileReader; | ||
| } else if (convertConstant != null && field.initialDefault() != null) { | ||
| this.isConstantOrMetadataField[pos] = true; | ||
| this.readers[pos] = |
mkuchenbecker
left a comment
There was a problem hiding this comment.
overall makes sense, datatypes tested, where there is a default and the default is not specified, then populate.
An intergration test in li-openhouse for trino and spark will be useful to capture actual behaviour.
Replace buildOrcProjection's boolean hasTrustedIds parameter with a FieldIdSource enum naming where a schema's Iceberg field ids came from: EMBEDDED for ids read from iceberg.id column attributes written into the file, NAME_MAPPED for ids derived at read time by matching column names. "Trusted" asserted a judgment without saying who trusts the ids or why, which a reviewer had to resolve by reading OrcIterable. The provenance is the checkable fact, and it is what decides whether an absent field may be read as "never written" and so have its declared default filled. Naming it at the call site also makes the two branches in OrcIterable read as the matched pair they are, rather than one passing a bare true and the other omitting the argument entirely. Each constant documents its own hazard: under NAME_MAPPED a field can look absent merely because its name did not match, for example after a rename the name mapping no longer covers, while its data is physically present in the file, so filling a default would fabricate values over real data. No behavior change. Also record why isOmittableDefault need not check that the field is scalar: Types.NestedField#castDefault already rejects a default on any nested type at construction time.
|
I added an explicit Previously, the shared ORC projection omitted missing defaulted columns whenever a file had embedded field IDs. That inadvertently affected every Spark/Flink row and vectorized reader, even though only Generic and Spark 3.1 iterative readers know how to fill the omitted column today. This could break non-supported engines by giving their positional readers a shorter column list. Now a field is omitted only when all four conditions hold: supportsInitialDefaults
&& field.initialDefault() != null
&& !mapping.containsKey(field.fieldId())
&& fieldIdSource == FieldIdSource.EMBEDDEDGeneric and Spark 3.1 iterative readers explicitly opt in. Other engines retain the previous projection and behavior. Spark 3.1 also avoids vectorized ORC reads when the projected schema contains This keeps default filling isolated to readers that implement it, while preserving backward compatibility everywhere else. |
3fd044f to
5175788
Compare
5175788 to
540604c
Compare
60115f6 to
d5893e8
Compare
d5893e8 to
ca18b89
Compare
Summary
Fills a field's
initial-defaultwhen it is absent from an id-bearing ORC file, using the same fallback order as the Parquet reader.buildOrcProjectionomits an absent defaulted field only when the file has embedded IDs and the configured reader explicitly declaressupportsInitialDefaults().StructReaderfrom ORC: bind struct fields by Iceberg field id in generic and Spark 3.1 readers #265 then materializes the declared default as a per-file constant, consuming no column vector.Why reader opt-in is required
OrcIterableis shared by Generic, Spark, and Flink row and vectorized readers. Embedded field IDs establish that an absent column was never written, but do not establish that the selected reader can fill it. The newORC.ReadBuilder.supportsInitialDefaults()capability keeps omission local to readers that implement the missing-field contract.Generic row reads opt in here. Spark 3.1 row reads opt in in #264. This branch also provides the recursive Spark 3.1 scan-routing guard required before the row-reader implementation is enabled. Other engines and vectorized readers do not opt in.
Provenance
This is net new rather than a backport. Apache Iceberg 1.2 has the schema model for defaults but does not implement ORC default reads. The resolution logic mirrors Parquet's field-reader fallback: physical reader → declared default → null/failure.
Supported scope
Generic ORC row reader; scalar defaults at any struct nesting level, including structs inside list elements and map values.
Vectorized ORC default materialization remains deferred. Predicates on omitted defaulted fields use a conservative ORC SearchArgument so they are evaluated after iterative default materialization.
Stack
StructReaderbackportTesting Done
iceberg-orcandiceberg-datatest suites pass