fix: surface numRecords from struct-stats-only checkpoints via ScanFile.stats

[BoazC-MSFT]

What changes are proposed in this pull request?

Delta tables configured with delta.checkpoint.writeStatsAsJson=false and delta.checkpoint.writeStatsAsStruct=true store file statistics only in add.stats_parsed (a native parquet struct). The add.stats JSON string is absent from the checkpoint. During log replay, get_add_transform_expr reads only add.stats, so ScanFile.stats is null for all checkpoint entries. Data skipping via stats_parsed is unaffected — this only impacts consumers
reading the ScanFile.stats JSON string (e.g. for num_records).

When has_stats_parsed is true, get_add_transform_expr now uses COALESCE(add.stats, ToJson(add.stats_parsed)) instead of just add.stats. This populates the JSON stats string from the struct when JSON stats are absent, with zero overhead when they already exist. Only numRecords is surfaced through this path — minValues/maxValues/nullCount are schema-dependent and already consumed directly as structs by the data-skipping path.

Closes #2541. Partially addresses #1075.

How was this change tested?

Added an integration test.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.67%. Comparing base (2e06c7f) to head (97b2f7c).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2542   +/-   ##
=======================================
  Coverage   88.67%   88.67%           
=======================================
  Files         182      182           
  Lines       61722    61730    +8     
  Branches    61722    61730    +8     
=======================================
+ Hits        54729    54737    +8     
  Misses       4863     4863           
  Partials     2130     2130           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[DrakeLin]

Thanks for raising this, had a comment:

stats_parsed is materialized in the scan output whenever stats output is requested via coalesce(stats_parsed, parseJson(stats). So stats_parsed is always materialized when the caller opted in, regardless of how the checkpoint was written.

Given that, would it be cleaner to teach ScanFileVisitor to read num_records from stats_parsed.numRecords directly when stats output is requested? This way connectors can just read stats from the structured object, rather than having to parse the json themselves after

[BoazC-MSFT]

Thanks for raising this, had a comment:

stats_parsed is materialized in the scan output whenever stats output is requested via coalesce(stats_parsed, parseJson(stats). So stats_parsed is always materialized when the caller opted in, regardless of how the checkpoint was written.

Given that, would it be cleaner to teach ScanFileVisitor to read num_records from stats_parsed.numRecords directly when stats output is requested? This way connectors can just read stats from the structured object, rather than having to parse the json themselves after

Thanks for the suggestion. I looked into reading numRecords directly from stats_parsed in ScanFileVisitor, but ran into a couple of issues. First, ScanFileVisitor implements FilteredRowVisitor, whose selected_column_names_and_types returns &'static references, so it can't conditionally request stats_parsed.numRecords depending on whether it's present in the schema. Working around that would mean either changing the trait's &'static constraint (which cascades through the RowVisitor bridge into 25+ impls) or adding a second pass over the data to patch stats in after the visitor runs. Second, without a fix at the transform level, ScanFile.stats is populated for tables that write JSON stats but silently None for struct-stats-only checkpoints, even though the underlying data exists. The COALESCE approach avoids both issues -- it runs during log replay when processing checkpoint batches, only fires when add.stats is actually null, and makes the behavior consistent regardless of how the checkpoint was written.

[DrakeLin]

I ran a bench framework locally with a 2000-add v2 sidecar checkpoint (writeStatsAsStruct=true, writeStatsAsJson=false) and a predicate spec:

Workload	Main	PR #2542	Δ	p
readWithPredicate (hits new branch)	2.73 ms	2.99 ms	+9.5%	0.00
readNoPredicate (control)	2.53 ms	2.52 ms	-0.4%	0.18

Could we put this behind a flag for now then, similar to with_output_stats ?

We can add with_output_json_stats(bool) on ScanBuilder, threaded into the transform so the COALESCE+ToJson only runs when the caller asked for it. Default false means no regression for callers who don't need it.

Long-term we should still surface num_records from stats_parsed.numRecords typed through the visitor, but the flag is a clean stopgap and trivially deletable later.

We can also just tag this as an issue and merge this PR first if you'd prefer that

@BoazC-MSFT

[Jameson-Crate]

we might want to mention the coalesce behavior in the doc comment

[BoazC-MSFT]

Fixed

[Jameson-Crate]

lgtm

[BoazC-MSFT]

I ran a bench framework locally with a 2000-add v2 sidecar checkpoint (writeStatsAsStruct=true, writeStatsAsJson=false) and a predicate spec:

Workload Main PR #2542 Δ p
readWithPredicate (hits new branch) 2.73 ms 2.99 ms +9.5% 0.00
readNoPredicate (control) 2.53 ms 2.52 ms -0.4% 0.18
Could we put this behind a flag for now then, similar to with_output_stats ?

We can add with_output_json_stats(bool) on ScanBuilder, threaded into the transform so the COALESCE+ToJson only runs when the caller asked for it. Default false means no regression for callers who don't need it.

Long-term we should still surface num_records from stats_parsed.numRecords typed through the visitor, but the flag is a clean stopgap and trivially deletable later.

We can also just tag this as an issue and merge this PR first if you'd prefer that

@BoazC-MSFT

Thanks for checking that, @DrakeLin.

The ~0.26ms/2000 files overhead seems acceptable for log replay given the correctness fix it provides.

Regarding the flag suggestion — kernel is usually consumed indirectly via engines (delta-rs in our case), so the flag would need to be threaded through the engine layer and exposed in its API. Looking a bit in delta-rs, that's does not seem like a trivial change for a short-term fix.

[Jameson-Crate]

/bench

[github-actions]

Benchmark for 97b2f7c

tags: `base`

Test	Base	PR	%
101kAdds1kCommitsSinceChkpt1Chkpt/readLatest/readMetadata/serial	340.0±7.79ms	333.7±9.69ms	-1.85%
101kAdds1kCommitsSinceChkpt1Chkpt/readV10/readMetadata/serial	1583.9±59.37µs	1567.2±36.92µs	-1.05%
101kAdds1kCommitsSinceChkpt1Chkpt/readV110/readMetadata/serial	35.2±1.36ms	34.9±0.79ms	-0.85%
101kAdds1kCommitsSinceChkpt1Chkpt/readV210/readMetadata/serial	77.7±4.34ms	71.1±5.77ms	-8.49%
101kAdds1kCommitsSinceChkpt1Chkpt/readV510/readMetadata/serial	171.4±3.96ms	165.3±3.66ms	-3.56%
101kAdds1kCommitsSinceChkpt1Chkpt/readV60/readMetadata/serial	18.2±0.43ms	17.8±0.32ms	-2.20%
101kAdds1kCommitsSinceChkpt1Chkpt/snapshotLatest/snapshotConstruction	119.1±2.97ms	117.6±5.44ms	-1.26%
101kAdds1kCommitsSinceChkpt1Chkpt/snapshotV10/snapshotConstruction	30.1±0.52ms	30.7±1.41ms	+1.99%
101kAdds1kCommitsSinceChkpt1Chkpt/snapshotV110/snapshotConstruction	39.5±0.55ms	40.0±0.66ms	+1.27%
101kAdds1kCommitsSinceChkpt1Chkpt/snapshotV210/snapshotConstruction	48.5±0.86ms	48.9±0.88ms	+0.82%
101kAdds1kCommitsSinceChkpt1Chkpt/snapshotV510/snapshotConstruction	75.0±1.94ms	75.7±1.79ms	+0.93%
101kAdds1kCommitsSinceChkpt1Chkpt/snapshotV60/snapshotConstruction	35.7±0.29ms	35.1±0.68ms	-1.68%