Parquet: Fix initial-default rows dropped when filtering on the defaulted column by cbb330 · Pull Request #16692 · apache/iceberg

cbb330 · 2026-06-05T09:29:00Z

What

A column added by schema evolution with an initial-default is backfilled with the default at read
time, but ParquetMetricsRowGroupFilter evaluates predicates against a column that is absent from
a data file as if it were all-null. For a file written before the column existed, a predicate
referencing the column — including the IsNotNull engines infer for null-intolerant predicates —
skipped the row group, silently dropping exactly the rows the default backfills. The full scan
was correct, so the case was untested and unobserved.

Fixes #16690.

Repro (Spark 3.5, Parquet, format-version 3)

CREATE TABLE t (id bigint, name string) USING iceberg TBLPROPERTIES ('format-version'='3');
INSERT INTO t VALUES (1, 'Alice');                          -- written before column c exists
-- table.updateSchema().addColumn("c", StringType, lit("US")).commit();
INSERT INTO t VALUES (2, 'Bob', 'US');                      -- c present

Query	Before	After
`SELECT id, c`	`(1,US),(2,US)` ✅	`(1,US),(2,US)`
`WHERE c = 'US'`	`(2)` ❌	`(1),(2)` ✅
`WHERE upper(c) = 'US'`	`(2)` ❌	`(1),(2)` ✅
`WHERE c IS NOT NULL`	`(2)` ❌	`(1),(2)` ✅
`WHERE c = 'CA'`	`()`	`()` ✅ (absent file still excluded)
`WHERE c IS NULL`	`()`	`()` ✅

Fix

The drop happens in ParquetMetricsRowGroupFilter, whose per-predicate handlers treat a column
missing from the file as all-null (// the column is not present and is all nulls). The filter is
already constructed with the table read schema, which carries initialDefault, so it has everything
it needs to be correct.

Override the single predicate dispatch point: when a predicate references a column that is missing
from the file but has an initialDefault, evaluate it against the default value
(pred.test(default)) instead of falling through to the null-assuming handlers. c = 'US' →
ROWS_MIGHT_MATCH, c = 'CA' → ROWS_CANNOT_MATCH, IsNotNull(c) → match, IsNull(c) → skip.
Columns present in the file, and missing columns without a default, are unchanged — so present-column
files still prune on real stats, and existing missing-column behavior is preserved.

This mirrors how the filter already gets the schema; it's a fix at the source rather than a rewrite
of the predicate at the call site.

Testing

TestMetricsRowGroupFilter#testColumnNotInFileWithInitialDefault (Parquet) — unit coverage for the
fold: match/non-match/IsNull/IsNotNull/notEqual against the default.
TestDefaultValuesFilteredRead (Spark 3.5) — end-to-end; fails on main without the fix.
Full :iceberg-parquet:test, :iceberg-data:test TestMetricsRowGroupFilter (Parquet + ORC), and
:iceberg-arrow:test pass.

Scope / notes

ParquetReader and VectorizedParquetReader both share this filter via ReadConf, so the row and
vectorized read paths are both fixed.
The dictionary and bloom row-group filters do not skip a column missing from the file, so they need
no change (verified by the end-to-end test).
Avro applies no file-level filtering (residual is applied by the engine after the default is
injected), so it is already correct. ORC throws UnsupportedOperationException on reading an
initial-default column, so there is no silent drop there.
The record-level filter path (ParquetFilters.convert) shares the same missing-column assumption
but is only reached via the @Deprecated readSupport reader, not by any engine read; it can get
the same treatment as a follow-up if desired.

…lted column A column added by schema evolution with an initial-default is backfilled with the default at read time, but ParquetMetricsRowGroupFilter evaluates predicates against a column that is absent from a data file as if it were all-null. For a file written before the column existed, a predicate referencing the column -- including the IsNotNull engines infer for null-intolerant predicates -- skipped the row group, silently dropping exactly the rows the default backfills. The full scan was correct, so the case was untested and unobserved. Make the row-group metrics filter default-aware: when a predicate references a column that is missing from the file but carries an initialDefault, evaluate it against the default value instead of assuming null. The filter already receives the table schema carrying the default, so this is a single check at the predicate dispatch point. Columns present in the file, and missing columns without a default, are unchanged. Closes apache#16690 Signed-off-by: Christian Bush <chbush@linkedin.com>

amogh-jahagirdar · 2026-06-15T04:00:57Z

I've gone ahead and added this to the 1.10.3 milestone since it's a correctness fix. I do think though this needs to be fixed differently by using the parquet schema as a source of truth rather than checking the stats.

…e in row-group filter Determine whether a column is missing from a file from the Parquet file schema rather than the column statistics. A column can be present in a file yet lack stats, so the stats maps cannot decide presence. A predicate on a column absent from the file with a non-null initial-default is evaluated against the default; a column present in the file is filtered on its real values. Adds a test that a present-but-no-stats column is filtered on stats rather than evaluated against its default.

amogh-jahagirdar · 2026-06-23T15:43:31Z

I published a PR to @cbb330 branch with a fix that uses the parquet schema and Iceberg schema fields with default values as a source of truth

…alue-fix Parquet pruning default value fix

nssalian · 2026-06-30T04:47:03Z

  }

+  @TestTemplate
+  public void testColumnNotInFileWithInitialDefault() {


Since the unit test only asserts eq/notEq/isNull/notNull against the default, it might be worth adding one range and one set case for the absent-default column, e.g. in("country", "US", "MX") -> read and in("country", "CA", "MX") -> skip, plus lessThan("country", "AA") -> skip.

Good call, I submitted a PR to @cbb330 branch cbb330#2 for this.

Would be great to land this in prior to merge since it's a small change to the test case. It looks good to me.

…ult column The unit test only asserted eq/notEq/isNull/notNull against the default. Range (lt/gtEq) and set (in/notIn) predicates dispatch through the same predicate() fold, so cover them explicitly to guard the dispatch path. Co-authored-by: Isaac

amogh-jahagirdar

I'll go ahead and approve this. I'll give it a day or so for any others to review cc @pvary @RussellSpitzer @stevenzwu @nssalian . If we're able to merge the test assertions by EOD today that's great, otherwise i think it's best to do a fast follow and in the backport we can include both. I'm largely trying to move the 1.10.3 release along as it's been some time.

…ter-tests Parquet: Add range and set predicate coverage for absent initial-default column

nssalian

thanks for fixing the tests @cbb330 and @amogh-jahagirdar.

stevenzwu · 2026-07-09T18:14:12Z

    StructType struct = schema.asStruct();
    this.expr = Binder.bind(struct, Expressions.rewriteNot(unbound), caseSensitive);
+    this.initialDefaults = Maps.newHashMap();
+    for (Types.NestedField field : schema.columns()) {


schema.columns() walks only top-level fields. nested field will still have the same problem. Consider iterating all fields via TypeUtil.indexById(struct).

I am also wondering if we should just call schema.findField(id) inside predicate(). Since findField(id) internally uses lazyIdToField() with a cached map, it is unnecessary to keep another <id, initialDefault> here.

Yeah we should just use the existing map to lookup by ID, no need for the extra mapping as it doesn't even save anything when we already need to keep all the field IDs in memory anyway. That addresses the nested field issue as well

stevenzwu · 2026-07-09T20:49:53Z

  }

+  @TestTemplate
+  public void testColumnNotInFileWithInitialDefault() {


A few paths that flow through the new predicate() override aren't exercised here. All three can reuse this test's 'field id not in the file' pattern with fresh ids — no writer setup, no new fixture:

startsWith / notStartsWith on country: startsWith("U") should match, startsWith("X") should skip; symmetric for notStartsWith.

One non-string default. pred.test((T) initialDefault) receives the internal Iceberg representation — Integer for date, Long for timestamp, BigDecimal for decimal, ByteBuffer for binary. String hides that contract because Java value and internal repr coincide; e.g. an IntegerType field (id 100) with Literal.of(42) against lt/gt/eq would guard the unchecked cast against a future representation change.

A DoubleType field (id 101) with a non-NaN default — enables isNaN (skip) and notNaN (match), which the current coverage skips entirely.

Thanks for the suggestion, Steven. Added all three:

startsWith/notStartsWith on country ("U" matches and "X" skips, with symmetric notStartsWith cases).

A DateType field using its raw Integer day-ordinal representation for lt/gt/eq, guarding the unchecked cast.

A DoubleType field with a non-NaN default covering isNaN (skip) and notNaN (match).

All follow the existing “field ID absent from the file schema” pattern with no new fixtures. Happy to add decimal or binary coverage if useful.

stevenzwu · 2026-07-09T20:58:49Z

+    table.updateSchema().addColumn("c", Types.StringType.get(), Expressions.lit("US")).commit();
+    sql("REFRESH TABLE %s", tableName);
+
+    sql("INSERT INTO %s VALUES (2, 'Bob', 'US')", tableName);


I would suggest also include a row with non-US values to cover the case that actual rows are read from the file

Added (3, 'Eve', 'CA') alongside the existing US row in the same insert, so filtering exercises physically stored matching and non-matching values in addition to the initial-default path.

…ter-tests fix nested field case

Cover additional predicate types and physical non-default values to guard row-group pruning behavior. Co-authored-by: Cursor <cursoragent@cursor.com>

Use a date field to verify predicate evaluation handles its internal integer representation. Co-authored-by: Cursor <cursoragent@cursor.com>

stevenzwu

LGTM. thanks for expanding the test coverage.

stevenzwu · 2026-07-10T04:29:18Z

@cbb330 can you fix the conflict?

cbb330 · 2026-07-10T06:42:17Z

@stevenzwu GitHub currently shows the PR as cleanly mergeable, and I can merge current main locally without conflicts. Are you still seeing a conflict? If so, could you point me to it?

pvary · 2026-07-10T08:13:30Z

+        Types.NestedField field = schema.findField(id);
+        if (field != null && field.initialDefault() != null && !valueCounts.containsKey(id)) {
+          return pred.test((T) field.initialDefault()) ? ROWS_MIGHT_MATCH : ROWS_CANNOT_MATCH;
+        }


How is this behaves with structs and defaults for structs?

github-actions Bot added spark parquet labels Jun 5, 2026

cbb330 force-pushed the fix-16690-default-filter-drop branch from dbc0dff to bbafc90 Compare June 5, 2026 09:40

pvary reviewed Jun 5, 2026

View reviewed changes

Comment thread parquet/src/main/java/org/apache/iceberg/parquet/ParquetFilters.java Outdated

cbb330 force-pushed the fix-16690-default-filter-drop branch from bbafc90 to 8326eb7 Compare June 5, 2026 20:33

github-actions Bot added the data label Jun 5, 2026

pvary reviewed Jun 9, 2026

View reviewed changes

Comment thread parquet/src/main/java/org/apache/iceberg/parquet/ParquetMetricsRowGroupFilter.java Outdated

amogh-jahagirdar requested review from amogh-jahagirdar and stevenzwu June 15, 2026 03:36

amogh-jahagirdar added this to the 1.10.3 milestone Jun 15, 2026

amogh-jahagirdar requested review from RussellSpitzer and rdblue June 16, 2026 14:30

amogh-jahagirdar added 2 commits June 25, 2026 13:28

address feedback

0c998b4

better naming

ec4fceb

cbb330 mentioned this pull request Jun 26, 2026

Parquet pruning default value fix cbb330/iceberg-apache#1

Merged

amogh-jahagirdar and others added 2 commits June 26, 2026 06:36

go back to using value_counts

b305710

Merge pull request #1 from amogh-jahagirdar/parquet-pruning-default-v…

1e92e6c

…alue-fix Parquet pruning default value fix

nssalian reviewed Jun 30, 2026

View reviewed changes

amogh-jahagirdar requested a review from pvary July 9, 2026 15:48

amogh-jahagirdar approved these changes Jul 9, 2026

View reviewed changes

Merge pull request #2 from amogh-jahagirdar/add-range-set-default-fil…

012c8a8

…ter-tests Parquet: Add range and set predicate coverage for absent initial-default column

nssalian approved these changes Jul 9, 2026

View reviewed changes

stevenzwu requested changes Jul 9, 2026

View reviewed changes

fix nested field case

9a5b599

stevenzwu reviewed Jul 9, 2026

View reviewed changes

cbb330 and others added 3 commits July 9, 2026 14:07

Merge pull request #3 from amogh-jahagirdar/add-range-set-default-fil…

1601926

…ter-tests fix nested field case

Parquet: Expand initial-default filter coverage

6c8dc4d

Cover additional predicate types and physical non-default values to guard row-group pruning behavior. Co-authored-by: Cursor <cursoragent@cursor.com>

Parquet: Test date initial-default representation

d428562

Use a date field to verify predicate evaluation handles its internal integer representation. Co-authored-by: Cursor <cursoragent@cursor.com>

stevenzwu approved these changes Jul 9, 2026

View reviewed changes

pvary reviewed Jul 10, 2026

View reviewed changes

Uh oh!

Conversation

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

Uh oh!

What

Repro (Spark 3.5, Parquet, format-version 3)

Fix

Testing

Scope / notes

Uh oh!

Uh oh!

Uh oh!

amogh-jahagirdar commented Jun 15, 2026

Uh oh!

amogh-jahagirdar commented Jun 23, 2026

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!

amogh-jahagirdar left a comment

Choose a reason for hiding this comment

Uh oh!

nssalian left a comment

Choose a reason for hiding this comment

Uh oh!

stevenzwu Jul 9, 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!

stevenzwu left a comment

Choose a reason for hiding this comment

Uh oh!

stevenzwu commented Jul 10, 2026

Uh oh!

cbb330 commented Jul 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cbb330 commented Jun 5, 2026 •

edited

Loading

stevenzwu Jul 9, 2026 •

edited

Loading