Skip to content

refactor: consolidate and strengthen Iceberg statistics tests - #705

Open
alexanderbianchi wants to merge 4 commits into
datafusion-contrib:iceberg-0.10from
alexanderbianchi:iceberg/test-statistics-cases
Open

refactor: consolidate and strengthen Iceberg statistics tests#705
alexanderbianchi wants to merge 4 commits into
datafusion-contrib:iceberg-0.10from
alexanderbianchi:iceberg/test-statistics-cases

Conversation

@alexanderbianchi

@alexanderbianchi alexanderbianchi commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Less test code, stronger statistics coverage

Based directly on iceberg-0.10 at f9340ef, including merged #715 and #716. “Before” refers to that base. Only iceberg/tests/statistics.rs changes, with a net reduction of 24 lines.

Test / edge case Before After
Row count and full-scan byte size Separate enabled/disabled test bodies Ordinary named tests share assertions and retain both modes
Full-schema column statistics Vector length only, in another pair of tests Complete expected vector: populated metrics and unknown columns
Reordered projection / nonconsecutive selected field IDs Projected vector length and row count Exact association of metrics with reordered columns using IDs 4, 1, and 5
Null-count aggregation across files None Exact sum of 5
Column-size aggregation across files None Inexact sums of 400 and 600
Min/max bounds and scalar types None Int32 and Int64 bounds checked with their precision; opposing extrema order exercises both updating and retaining bounds
One file missing a column's null count None Aggregate stays Absent, rather than treating missing as zero
Entirely missing column metrics Only an all-empty-metrics fixture Unknown column alongside columns with known metrics
Column statistics disabled Fixture had no metrics even when enabled Populated fixture must return unknown column statistics when disabled
Explicit current-snapshot selection Separate duplicate row-total test Fixture selects the snapshot explicitly in every matrix case
Missing snapshot summary totals Present Retained
Filter and projection/sort propagation Present Retained
COUNT(*) skips the scan Duplicated enabled/disabled bodies and snapshots Both named cases retained, sharing assertions
Explain formatting Two identical snapshots plus weak label checks One diagnostic snapshot; computed values verified by the matrix

Structure

  • Four ordinary named Tokio tests cover full scan / reordered projection × column statistics enabled / disabled. Two more named tests cover COUNT(*) scan elimination. No test_case macros or dependency.
  • Session setup uses the merged harness's configure_session(...); tests never access its private context.
  • Native Iceberg writers create a manifest with two synthetic data-file entries and a matching manifest list. refactor: build Iceberg test metadata at runtime #700's with_file supplies the bytes to the harness.
  • Assertions observe query-output statistics, removing the recursive search/downcast helper and also checking projection propagation.
  • insta::allow_duplicates! is needed only for the shared, identical COUNT(*) inline snapshots.
  • The fixture is planning-only: synthetic Parquet paths are not opened. Multi-manifest merging, delete files, NDV, and verification against actual Parquet contents are not claimed here.
  • The genuinely historical-snapshot regression remains in refactor: construct historical Iceberg fixtures in tests #702; this PR consolidates the current-snapshot-only check from feat[iceberg]: table statistics suite #687.

Validation

  • cargo test -p datafusion-distributed-iceberg --test statistics --locked — all 10 cases passed
  • cargo test -p datafusion-distributed-iceberg --locked — 95 tests passed, including the doctest
  • cargo test -p datafusion-distributed-iceberg --features integration --locked — 96 tests passed, including the doctest
  • cargo clippy -p datafusion-distributed-iceberg --all-targets --locked -- -D warnings — passed, also with --all-features
  • cargo fmt --all -- --check
  • git diff --check
  • Mutation check: replacing computed column statistics with unknown values fails both enabled cases; the other eight tests pass. Mutation reverted.
  • Bounds mutation check: keeping the first minimum and taking the last maximum fails both enabled cases; the other eight tests pass. Mutation reverted.

@alexanderbianchi
alexanderbianchi force-pushed the iceberg/test-statistics-cases branch from cefbe81 to 8e6fd2d Compare September 6, 2026 01:51
@alexanderbianchi alexanderbianchi changed the title refactor: consolidate Iceberg statistics test cases refactor: consolidate and strengthen Iceberg statistics tests Sep 6, 2026
@alexanderbianchi
alexanderbianchi changed the base branch from iceberg-0.10 to codex/iceberg-runtime-metadata September 6, 2026 01:51

assert_eq!(stats.num_rows, Precision::Exact(TAXI_ROWS));
assert_eq!(stats.total_byte_size, Precision::Exact(TAXI_BYTES));
insta::allow_duplicates! {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we switch the harness to something more like

   async fn run(
       ctx: &SessionContext,
       query: &str,
   ) -> Result<(Arc<dyn ExecutionPlan>, Vec<RecordBatch>)> {
       let df = ctx.sql(query).await?;
       let plan = df.create_physical_plan().await?;
       let batches = collect(Arc::clone(&plan), ctx.task_ctx()).await?;
       Ok((plan, batches))
   }

from tests/multi_task_collect_join_repros.rs

Replace test-case matrices with named tests sharing assertions and configure column statistics through the merged harness API. Preserve manifest metrics coverage and shared COUNT(*) snapshots without exposing the context.

Remove the unused Iceberg test-case dependency after rebasing onto iceberg-0.10.
@alexanderbianchi
alexanderbianchi force-pushed the iceberg/test-statistics-cases branch from f8dcd0c to ea6a73e Compare September 8, 2026 19:21
@alexanderbianchi
alexanderbianchi changed the base branch from codex/iceberg-runtime-metadata to iceberg-0.10 September 8, 2026 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant