feat: add columnar Delta->AMT stats pivot for content_stats by emkornfield · Pull Request #3351 · delta-io/delta-kernel-rs

emkornfield · 2026-09-17T17:13:01Z

🥞 Stacked PR

Use this link to review incremental changes.

stack/stats_leaf_walker [Files changed]
- stack/amt_stats_pivot [Files changed] ← this PR

What changes are proposed in this pull request?

Add try_pre_convert_stats_column, which converts a batch's parsed
Delta-stats column (numRecords/tightBounds + nested minValues/maxValues/
nullCount) into the flat AMT content_stats layout by building a
StructPatch expression and evaluating it on the engine. Rewritten for the
flat schema (each leaf reads its nested Delta source path), reusing the
shared StatsLeafWalker and leaf_stats_field so the expression and output
schema align by construction. Conversion is attempted optimistically then
retried conservatively against only the declared stat columns.

tight_bounds is type-aware: string/TIMESTAMP/TIMESTAMP_NTZ leaves are
forced false (Delta may truncate their bounds); other leaves take the
file's tightBounds, defaulting to true when absent/null so a false file
value still forces false everywhere. Full DBR rules are a follow-up.

Co-authored-by: Isaac no-reply@databricks.com

How was this change tested?

emkornfield · 2026-09-17T17:52:27Z

+/// Evaluation is attempted optimistically (assuming every Delta stat source the AMT layout wants is
+/// present, since the data may carry more than `input_schema` declares) and, if that fails, retried
+/// conservatively against only the stat columns `input_schema` declares.
+pub(crate) fn try_pre_convert_stats_column(


nit: preconvert (no "_"

emkornfield · 2026-09-18T06:02:14Z

+
+/// Converts a batch's parsed Delta-stats column into the flat AMT `content_stats` layout in place.
+///
+/// `data` conforms to `input_schema`; `stats_column_name` names a struct column holding parsed


lets make this more concise with an example of input and output.

emkornfield · 2026-09-18T06:02:31Z

+/// Returns `Ok(None)` when `input_schema`'s stats column is not in Delta-stats shape (no
+/// `numRecords`) -- e.g. it is already AMT format or absent -- or when the built expression fails
+/// to evaluate, so the caller leaves `content_stats` null (manifest-level data skipping is then
+/// unavailable). Errors from *building* the pivot expression -- a precondition violation such as a


I don't think we need to document errors here.

emkornfield · 2026-09-18T06:06:30Z

+        Err(e) => debug!("AMT stats pivot optimistic pass failed, retrying conservatively: {e}"),
+    }
+
+    // Conservative pass: reference only the stat sources the input schema declares.


we should only do this pass. I don't think we need to do the full pass.

emkornfield · 2026-09-18T06:11:14Z

+
+/// Builds the `tight_bounds` expression for a leaf.
+///
+/// String, binary, `TIMESTAMP`, and `TIMESTAMP_NTZ` bounds may be truncated by Delta, so they are


we need variant and geography here also I think.

emkornfield · 2026-09-18T06:13:38Z

+        let engine = SyncEngine::new();
+        let data = engine
+            .evaluation_handler()
+            .create_one(Arc::new(data_schema.clone()), values)


need to fix this.

emkornfield · 2026-09-18T06:14:45Z

+
+    /// Extracts the `content_stats` struct column from a successful pivot result.
+    fn content_stats_of(result: Box<dyn EngineData>) -> StructArray {
+        let batch: RecordBatch = ArrowEngineData::try_from_engine_data(result)


ideally we wouldn't use arrow here but instead use a visitor or compare engine data to a preconstructed value.

emkornfield · 2026-09-18T06:16:32Z

+        let content_stats = run_pivot(&input_schema, values);
+
+        let id = leaf(&content_stats, "id");
+        assert_eq!(


we should do these comparison in bulck instead of unwrapping to types.

Replace StatsSchemaCollector with a generic StatsLeafWalker<'a, F> that drives the table-schema walk (struct descent, Delta stat projection threading, and the drop-if-no-category rule) and invokes an `on_leaf` sink per surviving leaf. `collect_stats_schema` drives it with a closure that pushes `leaf_stats_field` results. Behavior of `stats_schema` / `projected_stats_schema` is unchanged; this lets the upcoming Delta->AMT stats pivot reuse the same leaf enumeration instead of duplicating it. Co-authored-by: Isaac <no-reply@databricks.com>

Add try_pre_convert_stats_column, which converts a batch's parsed Delta-stats column (numRecords/tightBounds + nested minValues/maxValues/ nullCount) into the flat AMT content_stats layout by building a StructPatch expression and evaluating it on the engine. Rewritten for the flat schema (each leaf reads its nested Delta source path), reusing the shared StatsLeafWalker and leaf_stats_field so the expression and output schema align by construction. Conversion is attempted optimistically then retried conservatively against only the declared stat columns. tight_bounds is type-aware: string/TIMESTAMP/TIMESTAMP_NTZ leaves are forced false (Delta may truncate their bounds); other leaves take the file's tightBounds, defaulting to true when absent/null so a false file value still forces false everywhere. Full DBR rules are a follow-up. Co-authored-by: Isaac <no-reply@databricks.com>

emkornfield · 2026-09-18T07:04:05Z

+/// Drives the table-schema walk shared by AMT `content_stats` schema generation
+/// ([`collect_stats_schema`]) and the Delta -> AMT stats pivot ([`build_amt_flat_stats_expr`]): it
+/// descends structs, threads the optional Delta stat projection through each descent, and invokes
+/// `on_leaf` once for every non-struct leaf the projection did not drop, with the leaf field, its


too much detail?

codecov · 2026-09-18T07:07:16Z

Codecov Report

❌ Patch coverage is 96.69118% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.53%. Comparing base (c457674) to head (0a6e4a2).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
kernel/src/content_tree/stats.rs	96.69%	14 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3351      +/-   ##
==========================================
+ Coverage   90.51%   90.53%   +0.02%     
==========================================
  Files         255      255              
  Lines       91213    91772     +559     
  Branches    91213    91772     +559     
==========================================
+ Hits        82558    83086     +528     
- Misses       5642     5669      +27     
- Partials     3013     3017       +4

☔ View full report in Codecov by Harness.
📢 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.

emkornfield · 2026-09-18T07:08:14Z

+            // is non-absent (nulls count as present; a row where an enclosing nullable struct is
+            // null makes the leaf absent). For top-level leaves `numRecords` is exact; for a leaf
+            // under a nullable struct it over-approximates, but Delta records no better source.
+            VALUE_COUNT if field_exists(NUM_RECORDS, false) => {


double check if this is accurate.

emkornfield · 2026-09-18T07:09:03Z

+
+    let exprs = stats_struct.fields().map(|f| {
+        let expr = match f.name().as_str() {
+            // TODO: for float/double leaves, Delta excludes NaN from min/max, so a bound can


delta actually includes NaN, Iceberg doesn't update the comment.

emkornfield · 2026-09-18T07:14:08Z

+            // is non-absent (nulls count as present; a row where an enclosing nullable struct is
+            // null makes the leaf absent). For top-level leaves `numRecords` is exact; for a leaf
+            // under a nullable struct it over-approximates, but Delta records no better source.
+            VALUE_COUNT if field_exists(NUM_RECORDS, false) => {


can we eliminate the boolean inputs to exists and just call exists directly in these cases?

emkornfield · 2026-09-18T07:24:08Z

+    }
+
+    #[test]
+    fn tight_bounds_expr_by_type_and_presence() {


this is better tested in an end-to-end with a resulting values instead of testing the helper method.

emkornfield · 2026-09-18T07:26:18Z

+    fn pivot_round_trip_maps_categories_and_counts() {
+        let input_schema = delta_stats_input_schema(true, true, true);
+        // numRecords=10, tightBounds=true, min{1,"aaa"}, max{5,"zzz"}, nullCount{0,2}.
+        let values: &[Scalar] = &[


might be clearer if we did create_many instead of the flattened schema.

emkornfield · 2026-09-18T07:28:11Z

+    }
+
+    #[test]
+    fn pivot_round_trip_nested_leaf_reads_nested_source_path() {


these tests seem to have a common pattern can we convert to rs_test?

emkornfield mentioned this pull request Sep 17, 2026

refactor: extract shared StatsLeafWalker for AMT content_stats walk #3350

Open

github-actions Bot assigned emkornfield Sep 17, 2026

emkornfield commented Sep 17, 2026

View reviewed changes

emkornfield commented Sep 18, 2026

View reviewed changes

emkornfield and others added 2 commits September 18, 2026 06:55

emkornfield force-pushed the stack/amt_stats_pivot branch from 8d30f7f to 0a6e4a2 Compare September 18, 2026 07:02

emkornfield commented Sep 18, 2026

View reviewed changes

Conversation

emkornfield commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🥞 Stacked PR

What changes are proposed in this pull request?

How was this change tested?

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!

codecov Bot commented Sep 18, 2026

Codecov Report

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!

1 participant

emkornfield commented Sep 17, 2026 •

edited

Loading