Skip to content

refactor: validate scalar Iceberg results with native values - #710

Draft
alexanderbianchi wants to merge 11 commits into
datafusion-contrib:iceberg/test-statistics-casesfrom
alexanderbianchi:iceberg/test-native-query-results
Draft

refactor: validate scalar Iceberg results with native values#710
alexanderbianchi wants to merge 11 commits into
datafusion-contrib:iceberg/test-statistics-casesfrom
alexanderbianchi:iceberg/test-native-query-results

Conversation

@alexanderbianchi

@alexanderbianchi alexanderbianchi commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Native scalar validation, with less test code

Stacked on #705 (iceberg/test-statistics-cases, base f8dcd0c18f2420602f7ee62524ff88e667c50519). This covers every scalar query-result test in the current Iceberg suite, plus the empty-result case—not just the initial three-function pilot.

Net −14 lines across the whole PR: 7 files, 91 insertions / 105 deletions, including the native query API and shared assertion helper. The initial pilot was +28 lines; the expanded version is 42 lines smaller. All existing test cases remain.

Before / after

Test / behavior Before After
Explicit metadata override ASCII count table One Int64(0) row, alias trips
Explicit snapshot selection ASCII count table One Int64(175000) row, alias trips
Metadata-only COUNT(*), both statistics settings Plan and result snapshots Native ProjectionExec -> PlaceholderRowExec checks and Int64(175000) result
Compound predicate pushdown Plan and count-table snapshots Same plan snapshot, typed count 9891
Null predicate pushdown Plan and count-table snapshots Same plan snapshot, typed count 8829
Unsupported residual filter after safe pushdown Plan and count-table snapshots Same plan snapshot, typed count 10516
Wholly unsupported filter Plan and count-table snapshots Same plan snapshot, typed count 2757
Computed MAX(trip_distance * fare_amount) Plan and scalar-table snapshots Same plan snapshot, Float64(207508.9584) result
LIMIT 0 EmptyExec and empty-table snapshots Native EmptyExec check and zero total result rows

Eight scalar-result functions (nine named cases) share the same assertion helper. The empty-result case makes ten migrated cases overall. Multi-row/multi-column results, DESCRIBE, errors, statistics, and meaningful plan snapshots remain intact; they are not replaced merely to remove snapshot macros.

Structure

  • IcebergTestHarness::query_raw returns the executed native plan and Arrow batches, using one physical plan and its DataFrame task context.
  • Existing query() -> Result<(String, String)> stays compatible. It captures plan text before executing that same plan, preserving existing snapshots without resetting runtime state or creating a second physical plan.
  • The shared integration-only scalar assertion checks one total row across batches, one column, its alias, and the expected typed value. It does not assume one physical batch. #[track_caller] points failures at the calling test.
  • Typed i64/f64 expectations distinguish values that would print identically as table text. No public assertion API, dependencies, feature flags, benchmarks, or root comparator changes. No performance claim.

Validation

  • cargo test -p datafusion-distributed-iceberg --locked — 84 tests and 1 doctest passed; no cases removed
  • cargo clippy -p datafusion-distributed-iceberg --tests --locked -- -D warnings — passed
  • cargo fmt --all -- --check and git diff --check — passed
  • Negative value check: expecting Int64(1) for the empty table fails against actual Int64(0)
  • Negative type check: expecting Int32(9891) fails against actual Int64(9891) at the filter test's call site, even though both values print as 9891
  • Both negative controls were reverted; no snapshot expectations were updated

Validation is scoped to the Iceberg crate; distributed integration/gRPC features were not enabled. #709 remains the independent result-comparator bug fix.

@alexanderbianchi
alexanderbianchi force-pushed the iceberg/test-statistics-cases branch from f8dcd0c to ea6a73e Compare September 8, 2026 19:20
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