Core: Read content stats from v4 Manifest by nastra · Pull Request #17433 · apache/iceberg

nastra · 2026-07-30T15:30:51Z

V4ManifestReader now reads the content_stats column, so each TrackedFile comes
back with per-column bounds and counts.
The stats schema is derived from the table schema, so builder() takes it as a second
argument. Stats for every column are read by default, which is what copying entries into
a new manifest needs. Callers that want less can narrow:

projectStats(fieldIds) reads stats only for the given columns, projectStats() reads none
forScanPlanning() reads only what the filter needs
stats referenced by the filter are always read, even when select/project omits them

Reading stats surfaced two bugs in the copy path, fixed here:

ContentStatsStruct.copy() threw an NPE when a projected column had no stats in the
manifest
FieldStatsStruct copied StructLike bounds by reference. Geometry and geography bound
a bounding-box struct, so under reuseContainers() every entry reported the last row's
bounds. Bounds are now deep-copied through StructLikeUtil.copy, and StructCopy is
Serializable so a copied bound still survives serialization.

Used Claude for the initial prototyping but reviewed and adjusted the code manually

anoopj · 2026-07-30T19:28:36Z

+      readBuilder.setCustomType(TrackedFile.CONTENT_STATS_ID, ContentStatsStruct.class);
+      // content_stats holds one stats struct per projected column
+      for (Types.NestedField fieldStats : statsField.type().asStructType().fields()) {
+        readBuilder.setCustomType(fieldStats.fieldId(), FieldStatsStruct.class);


Does geo and variant need any special handling here? Can we add a test to make sure those types work?

good point, I've added tests for geo + variant types with single/multiple files and this uncovered a bug around Geo types copying, which I've fixed in FieldStatsStruct

anoopj

Is the file pruning coming in a followup?

dramaticlly · 2026-08-04T00:12:27Z

+     * fields referenced by the {@link #filter(Expression) filter} are always read.
+     */
+    Builder projectStats(Iterable<Integer> fieldIds) {
+      Preconditions.checkArgument(fieldIds != null, "Invalid stats projection for field IDs: null");


I think we have 4 modes relevant to stats so far, the default/CDC, scan planing, select column by name and project schema and we conditionally add required column depends on the filter. I am wondering if we want to add coverage for

empty projectStats and filter (ok to use precondition to check if this combination does not make sense)

valid projectStats and filter with default mode ( I think scan planning is already covered in projectStatsAndFilterStatsAreCombined)

dramaticlly · 2026-08-04T00:53:32Z

+      Set<Integer> requiredFieldIds = requiredStatsProjectionForFieldIds();
+      Schema fullSchema = fullSchema(requiredFieldIds);


nit: curious if we shall build requiredStatsType for reading stats regardless of caller's column projection ahead of time instead of passing them to calculate multiple times later? Can potentially pass requiredStatsType to fullSchema() and addRequiredColumns().

stevenzwu · 2026-08-06T19:06:12Z

+    /**
+     * Reads content stats for the given table field IDs instead of for every field. Stats for
+     * fields referenced by the {@link #filter(Expression) filter} are always read.
+     */


nit: worth spelling out the null-vs-empty distinction contract. Something like: Call with no arguments or an empty iterable to opt out of stats reads entirely; a null argument is rejected. This applies symmetrically to the Iterable<Integer> overload below.

stevenzwu · 2026-08-06T20:30:58Z

+
+    /** Returns the stats type to read, which is empty when no stats are needed. */
+    private Types.StructType contentStatsType(Set<Integer> requiredStatsProjectionForFieldIds) {
+      if (scanPlanning || statsProjectionForFieldIds != null) {


Is the asymmetry intended? A caller doing join/aggregate pushdown has to explicitly narrow via projectStats(...), but stats for row-filter-referenced columns come along for free — without any opt-in from the caller — because the default (non-scan-planning, no projectStats) reads full stats and the filter only forces its refs to be included on top of a narrower projection.

The check itself (statsProjectionForFieldIds != null) is right — requiredStatsProjectionForFieldIds may be non-empty due to the row filter, but we only want to narrow when the caller explicitly asks via projectStats(fieldIds).

stevenzwu · 2026-08-07T00:29:13Z

+      }
+
+      return StatsUtil.statsReadSchema(
+          tableSchema, TypeUtil.indexById(tableSchema.asStruct()).keySet());


This walks the full table schema for every build() — TypeUtil.indexById once, then statsReadSchema walks again (plus indexParents and per-field isScalar climbs to the root). Fine per manifest, but this is on the default path (no projectStats, no forScanPlanning) taken for every manifest read that copies entries forward, so the cost multiplies across a scan's fan-out on wide tables.

Compounding this: manifests only store stats for a capped prefix of columns (default ~100 via MetricsConfig), so on a table with e.g. 5,000 columns the default "read all stats" builds a stats schema with ~5,000 slots and registers 5,000 FieldStatsStruct custom types — but ~4,900 of them resolve to null at decode time because the manifest never stored them. We're paying construction cost for stats we know aren't there.

But I don't have a good solution. Neither option below is clean:

Using current MetricsConfig.metricsFieldIds() at read time is per-table, not per-manifest. If the cap narrowed since the manifest was written, we silently drop stats the manifest actually holds — no correctness impact (InclusiveMetricsEvaluator treats absent stats as "may match"), but pruning gets coarser on copy-forward and scans open more files at query time. If it widened, we still over-ask for the extra columns and get the same null-resolution waste. Not a sound signal either way.

The only truthful source is the manifest's own content_stats schema. But peeking at that before configuring the projection means either an extra file open per manifest (drop below InternalData to Avro.read/Parquet.read for a header/footer peek, then reopen via InternalData with the intersection), or extending InternalData.ReadBuilder with a fileSchema() accessor so the projection can be picked after the header is read. Both cost something.

Flagging this to see if we can explore good alternatives — not blocking this PR.

And orthogonally, I am also wondering if we should caching the full stats read schema keyed off the Schema (like Schema.lazyIdToField)?

Idea worth exploring: version MetricsConfig in TableMetadata alongside schemas and partition specs, and stamp each manifest file (root or leaf) with the metrics-config-id in effect when the manifest was written. Readers resolve the id and recover the exact write-time stats field IDs, and build the read schema off the intersection with tableSchema. Correct across config drift; no extra file peek at read time.

Trade-offs: spec change (metrics-configs list + current-metrics-config-id on TableMetadata, MetricsConfigParser, metrics-config-id on manifest file entries); MetricsConfig shifts from a runtime property derivation to a first-class immutable artifact; retention/dedup matters since property tweaks churn more than schema/spec changes. With metrics-config-id, schema-id, sort-id, reader should be able to faithfully reconstruct the writer MetricsConfig and content stats write schema. This also requires move the metrics config from table properties to properly versioned struct in table metadata.

Alternatives — field-IDs list in each manifest's header, or on the manifest-list entry — either force a pre-read I/O per manifest or bloat the manifest list on wide tables, so the versioned direction seems the cleanest long-term shape.

stevenzwu · 2026-08-07T02:49:16Z

+  void statsAreNullForColumnsWithoutStoredStats(FileFormat format) throws IOException {
+    // the manifest stores stats for id alone, while the reader reads stats for every column
+    Types.StructType storedStatsType =
+        StatsUtil.statsReadSchema(TABLE_SCHEMA, List.of(ID_FIELD_ID));


nit: we are constructing write schema using StatsUtil.statsReadSchema. just looks a bit weird.

stevenzwu · 2026-08-07T03:01:59Z

+
+  @ParameterizedTest
+  @FieldSource("MANIFEST_FORMATS")
+  void statsAreCorrectWithContainerReuse(FileFormat format) throws IOException {


I don't quite get this test. V4ManifestReader always copy first before return, although it configures InternalData.reuseContainers().

return CloseableIterable.transform(entries, TrackedFile::copy).iterator();

yes it does copy but there were bugs which were uncovered by this and the other containerReuse tests

details can be seen in 5e3733c and 5651b63

should the copy testing be covered in TestFieldsStatsStruct, TestContentStatsStruct, or TestTrackedFileStruct?

stevenzwu · 2026-08-07T17:17:32Z

+
+  @ParameterizedTest
+  @FieldSource("MANIFEST_FORMATS")
+  void requestedStatsAreProjectedWhenOmittedByCaller(FileFormat format) throws IOException {


nit: maybe the method name should be requestedStatsProjectedButOmittedBySchemaProjection?

github-actions Bot added the core label Jul 30, 2026

nastra marked this pull request as draft July 30, 2026 15:45

nastra force-pushed the read-content-stats-from-manifest branch 3 times, most recently from c62305e to 6cf0fd0 Compare July 30, 2026 18:25

anoopj reviewed Jul 30, 2026

View reviewed changes

stevenzwu added this to V4: metadata tree Jul 30, 2026

nastra force-pushed the read-content-stats-from-manifest branch 3 times, most recently from 4975ce5 to 33d52b2 Compare July 31, 2026 10:38

nastra moved this to In review in V4: metadata tree Jul 31, 2026

nastra marked this pull request as ready for review July 31, 2026 10:54

nastra requested review from amogh-jahagirdar, danielcweeks, rdblue and stevenzwu July 31, 2026 10:54

vishnuprakaz mentioned this pull request Aug 3, 2026

Core: Fix geo bounds in v4 legacy stat maps #17493

Open

dramaticlly reviewed Aug 4, 2026

View reviewed changes

nastra added 10 commits August 6, 2026 19:54

Core: Read content stats from v4 Manifest

ca3ac43

Pass table schema to manifest reader builder()

8b19706

improvements

07d8327

add test with container reuse + fix NPE

5e3733c

more tests

31daad3

move tests into their own class

93313d3

add tests for geo types and fix container reuse bug

5651b63

add tests for variant

e288639

improvements

471422b

Make Geo types copy serializable

b8612ba

nastra force-pushed the read-content-stats-from-manifest branch from 33d52b2 to b8612ba Compare August 6, 2026 17:59

stevenzwu reviewed Aug 7, 2026

View reviewed changes

		Set<Integer> requiredFieldIds = requiredStatsProjectionForFieldIds();
		Schema fullSchema = fullSchema(requiredFieldIds);

Uh oh!

Conversation

nastra commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anoopj 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevenzwu Aug 7, 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!

stevenzwu Aug 7, 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nastra commented Jul 30, 2026 •

edited

Loading

stevenzwu Aug 7, 2026 •

edited

Loading

stevenzwu Aug 7, 2026 •

edited

Loading