ORC/Spark: forward-port default-value reads via idToConstant (LI #76) by cbb330 · Pull Request #267 · linkedin/iceberg

cbb330 · 2026-08-10T22:57:15Z

Summary

Forward-port of the Spark ORC default-value read path from LI #76 onto openhouse-1.2.0, adapted to initial-default and stacked on id-bound ORC structs (#265).

Absent defaulted fields are omitted from the per-file ORC projection and injected into idToConstant. The Spark row reader fills them. Omit is opt-in (supportsInitialDefaults) because ORCSchemaUtil is shared with Generic.

Vectorized ORC default-fill is out of scope. SparkBatchScan disables columnar ORC when the projection includes an initial-default. The vectorized-dir leftovers from #76 were dropped from this PR.

Mechanism (same as #76):

Omit absent defaulted fields from the per-file ORC projection
Inject their defaults into idToConstant
Existing ConstantReader materializes them

Inline notes mark each site as as-is or changed vs #76 (review).

Stack #270: this PR → #268 → #269.

Reviewer guide

As-is

Behavior	Where	Note
Overridable `visitRecord`	`OrcSchemaWithTypeVisitor`	as-is
Iceberg/ORC id alignment	`containsInOrder`	as-is
Row reader uses that visitor	`SparkOrcReader`	as-is
Nested `convertConstant` STRUCT/LIST/MAP	kept, unused	`convertConstant`

Changed

Divergence	This PR	Why	Note
Default API + omit	`initialDefault()`; omit gated	#250; `ORCSchemaUtil` is shared with Generic	omit · inject · gate
Who opts in	Spark row reader only	Generic does not fill yet; vectorized fill is out of scope	row · batch
Inject null-guard	Put only when `initialDefault() != null`	Avoid stuffing null constants	inject
Struct reader ctor	Id-bound `struct(record, ...)`	Required by #265	struct
Nested-typed defaults	Machinery kept; cannot declare yet	`castDefault` rejects non-null nested defaults (apache/iceberg#14611). Nested scalar defaults (e.g. `loc.country`) are covered	kept unused
Filter-on-defaulted-column tests	In #269	Needs file-level filter pushdown disabled for omitted columns	—

Follow-ups

#268: omit only for complete embedded field IDs
#269: skip file-level filters / empty-projection filters
Later: Generic reader; lift castDefault + nested-typed defaults

Replaces #263/#264 for the Spark fill path.

Testing Done

export JAVA_HOME=$(/usr/libexec/java_home -v 11)
./gradlew -DsparkVersions=3.1 -DscalaVersion=2.12 -DhiveVersions= -DflinkVersions= \
  :iceberg-spark:iceberg-spark-3.1_2.12:test \
  --tests 'org.apache.iceberg.spark.data.TestSparkOrcReaderForFieldsWithDefaultValue' \
  --tests 'org.apache.iceberg.spark.source.TestSparkBatchScanInitialDefaults' \
  -x generateGitProperties

testOrcScalarDefaultValues — pass
testOrcNestedScalarDefaultValues — pass
TestSparkBatchScanInitialDefaults — pass

cbb330

Inline annotations for the reviewer guide: forward-ported as-is vs forward-ported with changes. See the updated tables in the PR description for links back to each comment.

cbb330 · 2026-08-10T23:27:31Z

+     * from an ORC file. Disabled by default so existing readers retain null-synthesizing projection
+     * behavior. Spark ORC readers opt in (forward-port of LI #76).
+     */
+    public ReadBuilder supportsInitialDefaults() {


Raymond always omitted defaulted columns from the ORC projection, for every reader. This fork shares ORCSchemaUtil with Generic, so omit is opt-in via supportsInitialDefaults() — only the Spark row reader calls it.

"Raymond's omit was always-on." what does this mean?

sorry, the inline comments were confusing before, fixed them.

basically this PR is a "forward port" of Raymond Zhang's previous previous implementation which was utilized for hive. Bc the previous impl landed in production, we can feel safer to implement his work as is.

but his work as is has a couple gotchas. One is that it activated for every type of reader, not just spark's row reader. Because vectorized reads introduce another level of complexity, and I have found 0 tables in catalog with the vectorized read table property present*, I'm leaving it out of scope for the initial phase we are implementing (which is behind a feature gate).

there are two ways to activate vectorized reads: one is table property. other is spark session property. so maybe a few people are using spark session property, but I code searched "spark.sql.iceberg.vectorization.enabled" and didn't find any user repos that have enabled.

One is that it activated for every type of reader, not just spark's row reader.

Yeah, the blast radius for this is all tables vs just those that are enabled. The code seems safe though one sxisting codepath in the orc dir(famous last words)

mkuchenbecker · 2026-08-12T17:31:39Z

-  private static <T> T visitRecord(
+  /**
+   * Visits a struct. Overridden by Spark to inject {@code initial-default} values into {@code
+   * idToConstant} for fields omitted from the ORC projection (forward-port of LI #76 / {@code


do we need this forward port or can we consider this a reasonable limitation? Those datasets must rewrite themselves but are unblocked to migrate essentially.

I think this might be mixing two things.

This isn’t a compatibility path for files without field IDs. Those we don’t handle at all ( so name-mapped / incomplete IDs stay on the null-synthesizing projection as-is current status quo)

The production case we are targetting is the opposite: the file has Iceberg IDs, so we can omit an absent defaulted column from the ORC projection. This visitRecord looks at the Iceberg schema, notices the hole, and puts the default in idToConstant. Without that, fill-on-read does not work.

cbb330

Reviewer guide: as-is vs changed vs LI #76. Tables in the PR description link here.

cbb330 · 2026-08-12T20:31:19Z

+   * Visits a struct. Overridden by Spark to inject {@code initial-default} values into {@code
+   * idToConstant} for fields omitted from the ORC projection.
+   */
+  protected T visitRecord(


As-is. Protected visitRecord so Spark can inject defaults for omitted fields.

cbb330 · 2026-08-12T20:31:19Z

+   * Checks whether {@code list1} contains all integers from {@code list2} in the same relative
+   * order. {@code list1} may contain extra integers that {@code list2} does not.
+   */
+  private static boolean containsInOrder(List<Integer> list1, List<Integer> list2) {


As-is. Iceberg/ORC field-id alignment check.

cbb330 · 2026-08-12T20:31:19Z


-  private static class ReadBuilder extends OrcSchemaWithTypeVisitor<OrcValueReader<?>> {
-    private final Map<Integer, ?> idToConstant;
+  private static class ReadBuilder extends OrcSchemaWithTypeVisitorSpark<OrcValueReader<?>> {


As-is. Row reader extends the Spark visitor to pick up inject.

cbb330 · 2026-08-12T20:31:19Z


-  private static class ReadBuilder extends OrcSchemaWithTypeVisitor<Converter> {
-    private final Map<Integer, ?> idToConstant;
+  private static class ReadBuilder extends OrcSchemaWithTypeVisitorSpark<Converter> {


As-is wiring. Vectorized reader still extends the Spark visitor. Defaulted scans are forced onto the row reader, so this path does not fill.

Dropped. Vectorized fill leftovers from the #76 forward-port; this path is unchanged now. Defaulted scans still go through the row reader via SparkBatchScan.

cbb330 · 2026-08-12T20:31:19Z

+import org.apache.spark.unsafe.types.UTF8String;
+
+/** Constant array column vector. Unused until nested-typed defaults are legal in the API. */
+public class ConstantArrayColumnVector extends ConstantColumnVector {


As-is (unused). Nested constant vector kept; castDefault still rejects nested-typed defaults.

Dropped. Vectorized fill leftovers from the #76 forward-port; this path is unchanged now. Defaulted scans still go through the row reader via SparkBatchScan.

cbb330 · 2026-08-12T20:31:19Z

        orcType = TypeDescription.createStruct();
        for (Types.NestedField nestedField : type.asStructType().fields()) {
+          // Omit so the reader fills via idToConstant instead of a synthetic null column.
+          if (supportsInitialDefaults


Changed. Same omit, but initialDefault() != null and gated by supportsInitialDefaults because this util is shared with Generic.

cbb330 · 2026-08-12T20:31:20Z

+        if (MetadataColumns.nonMetadataColumn(iField.name())
+            && !idToConstant.containsKey(iField.fieldId())
+            && iField.initialDefault() != null) {
+          idToConstant.put(


Changed. Same id-mismatch inject, but initialDefault() and skip nulls.

cbb330 · 2026-08-12T20:31:20Z

        List<String> names,
        List<OrcValueReader<?>> fields) {
-      return SparkOrcValueReaders.struct(record, fields, expected, idToConstant);
+      return SparkOrcValueReaders.struct(record, fields, expected, getIdToConstant());


Changed. Id-bound struct(record, ...) from #265, not positional.

cbb330 · 2026-08-12T20:31:20Z

            .split(task.start(), task.length())
            .createReaderFunc(
                readOrcSchema -> new SparkOrcReader(readSchema, readOrcSchema, idToConstant))
+            .supportsInitialDefaults()


Changed. Spark row reader opts into omit+fill; Generic does not.

cbb330 · 2026-08-12T20:31:20Z

+    boolean hasNoInitialDefaults = hasNoInitialDefaults(expectedSchema);
+
+    // Defaulted projections stay on the row reader; batched ORC does not fill initial-defaults.
+    boolean batchReadOrc = hasNoDeleteFiles && allOrcFileScanTasks && hasNoInitialDefaults;


Changed. Defaulted projections stay on the row reader; batched ORC does not fill.

cbb330 · 2026-08-12T22:31:00Z

@mkuchenbecker carrying over your notes from #264 (and the Generic fill site on #263). Those PRs are closed; this is where the answers live.

Vectorized: no fill PR. Defaulted projections are forced onto the row reader (SparkBatchScan.hasNoInitialDefaults). BatchDataReader does not call supportsInitialDefaults(). Out of scope for this stack.

Spark 3.1 vs 3.5: this stack is openhouse-1.2.0 / Spark 3.1 (production OpenHouse). 3.5 is the 1.5.x line (id-binding is #257, separate).

convertConstant moved out of BaseDataReader: it was not. Option A introduced a new SparkValueConverters path. Here inject uses the existing BaseDataReader.convertConstant (same helper as partition constants).

Silent failure / is null an error: if we cannot prove the column was never written, we do not fill — we keep null (status quo). We do not throw and we do not invent a default. That is intentional. The case you wanted to avoid (silently overlaying a default on real data) is handled by refusing to omit, not by failing the read.

All data types: current coverage is int/string and a nested scalar (loc.country). castDefault still rejects nested-typed defaults (list/map/struct values). Broader scalar coverage (bool, decimal, timestamp, …) is a fair gap — not in this stack yet. Same for an OpenHouse Trino/Spark integration test; still outstanding.

Generic OrcValueReaders fill: not in this stack. Generic does not opt in. Fill is Spark row idToConstant → ConstantReader only.

Restore the production Spark ORC initial-default path from f200623: omit absent defaulted fields from the ORC projection and inject them into idToConstant so row and vectorized readers fill via existing constant readers. Adapted to the upstream initial-default API and gated behind supportsInitialDefaults so shared ORCSchemaUtil does not break Generic.

Vectorized ORC default-fill is out of scope. SparkBatchScan disables columnar ORC when the projection includes an initial-default, and the batched reader no longer opts into omission.

Keep the why (opt-in omit, row-only fill, nested defaults not API-legal). Drop stack TODOs, commit SHAs, and forward-port narration.

mkuchenbecker · 2026-08-14T00:54:49Z

+    /**
+     * Signals that the configured reader can fill {@code initial-default} values for fields omitted
+     * from an ORC file. Disabled by default so existing readers retain null-synthesizing projection
+     * behavior.


what is a "null-synthesizing projection" ?

Projection here means the columns this read asked for (the read schema), not the columns physically in the file.

If country is in the projection but missing from an old ORC file, the reader still has to return a country field. Today when it does that, it fills null. That's a null-synthesizing projection. It honors the requested schema by inventing nulls for columns the file doesn't have.

supportsInitialDefaults() is the new path. The scan still carries country, but we don't read that stream from this ORC file and we don't fill null. We omit it from this file's ORC columns and put initial-default in the scan's country slot.

The same existing behavior stays by default and returns nulls. Only the Spark row reader opts in, and only fields with initial-default take the new path.

mkuchenbecker

why is the vectorized dir needed?

Vectorized fill is out of scope; SparkBatchScan already keeps defaulted projections on the row reader. Restore the batched visitor and constant vectors to the pre-#76 path.

cbb330 · 2026-08-14T04:33:54Z

why is the vectorized dir needed?

@mkuchenbecker good catch, i removed them. they are dead code in this PR since I disabled vectorized.

these came forward from the original work from RZ from the forward port. but we aren't using it any time soon

github-actions Bot added SPARK ORC labels Aug 10, 2026

cbb330 commented Aug 10, 2026

View reviewed changes

mkuchenbecker reviewed Aug 12, 2026

View reviewed changes

cbb330 force-pushed the chbush/oh120-orc-defaults-raymond-pr1 branch from 3302e57 to 8c8d99b Compare August 12, 2026 17:40

This was referenced Aug 12, 2026

ORC: omit initial-defaults only for complete embedded field IDs #268

Open

ORC: don't push file filters for omitted initial-default fields #269

Open

cbb330 commented Aug 12, 2026

View reviewed changes

This was referenced Aug 12, 2026

ORC: Fill initial defaults for absent scalar columns on read #252

Closed

ORC: fill initial defaults for missing id-bound fields #263

Closed

Spark 3.1: fill ORC initial defaults on the row reader #264

Closed

cbb330 added 5 commits August 12, 2026 17:36

ORC/Spark: apply spotless formatting for CI

9c90f1a

ORC/Spark: keep defaulted projections on the row reader

006163b

Vectorized ORC default-fill is out of scope. SparkBatchScan disables columnar ORC when the projection includes an initial-default, and the batched reader no longer opts into omission.

ORC/Spark: trim process notes from source comments

7d2b5f0

Keep the why (opt-in omit, row-only fill, nested defaults not API-legal). Drop stack TODOs, commit SHAs, and forward-port narration.

ORC/Spark: fix spotless wrap in default-value test javadoc

95fbc27

cbb330 force-pushed the chbush/oh120-orc-defaults-raymond-pr1 branch from 526e934 to 95fbc27 Compare August 13, 2026 00:36

cbb330 mentioned this pull request Aug 13, 2026

[default values] Spark catalog itest for ReadBridge initial-default overlay linkedin/openhouse#681

Open

17 tasks

mkuchenbecker reviewed Aug 14, 2026

View reviewed changes

cbb330 added 2 commits August 13, 2026 21:31

ORC/Spark: drop unused vectorized default-fill leftovers

fd10372

Vectorized fill is out of scope; SparkBatchScan already keeps defaulted projections on the row reader. Restore the batched visitor and constant vectors to the pre-#76 path.

ORC/Spark: Spark inject visitor is row-reader only

609e8b9

Uh oh!

Conversation

cbb330 commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Reviewer guide

As-is

Changed

Follow-ups

Testing Done

Uh oh!

cbb330 left a comment

Choose a reason for hiding this comment

Uh oh!

cbb330 Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbb330 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbb330 commented Aug 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkuchenbecker left a comment

Choose a reason for hiding this comment

Uh oh!

cbb330 commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cbb330 commented Aug 10, 2026 •

edited

Loading

cbb330 Aug 10, 2026 •

edited

Loading

cbb330 commented Aug 14, 2026 •

edited

Loading