Skip to content

fix: preserve statistics for safe casts without extrema - #25517

Open
viirya wants to merge 2 commits into
apache:mainfrom
viirya:codex/preserve-safe-cast-statistics
Open

viirya wants to merge 2 commits into
apache:mainfrom
viirya:codex/preserve-safe-cast-statistics

Conversation

@viirya

@viirya viirya commented Sep 19, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

N/A

Rationale for this change

Projection statistics currently infer a CAST's source type from its minimum or
maximum value. When both extrema are absent, even a lossless widening cast such
as Int32 to Int64 is treated as unsafe.

This discards otherwise valid statistics, including exact null counts and
distinct counts, and can reduce the quality of optimizer estimates.

What changes are included in this PR?

  • Add ProjectionExprs::project_statistics_with_input_schema, which uses the
    input schema to identify lossless casts when typed extrema are unavailable.
  • Preserve the existing conservative behavior for casts that may fail, discard
    values, or change ordering.
  • Use the schema-aware method in ProjectionExec, extended statistics
    projection, and file scan statistics projection.
  • Add helper-level and ProjectionExec regression tests covering an Int32 to
    Int64 cast with absent extrema but exact null and distinct counts.

The existing project_statistics API remains available, so this is an
additive, non-breaking API change.

What is the testing strategy for this PR?

Added:

  • test_project_statistics_safe_cast_without_extrema
  • test_projection_statistics_safe_cast_without_extrema

Ablation testing confirmed that the old implementation changes
null_count = Exact(3) to Absent. Both the helper-level and production
ProjectionExec paths preserve the exact null and distinct counts after the
fix.

Also verified:

  • Targeted regression tests
  • cargo fmt --all -- --check
  • git diff --check

The full test suite is covered by CI.

Are there any user-facing changes?

Optimizer statistics are now preserved through known-lossless casts even when
minimum and maximum statistics are unavailable.

This PR adds a public method but does not remove or change the existing API.

@github-actions github-actions Bot added physical-expr Changes to the physical-expr crates datasource Changes to the datasource crate physical-plan Changes to the physical-plan crate labels Sep 19, 2026
@codecov-commenter

codecov-commenter commented Sep 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.07843% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.42%. Comparing base (710c748) to head (64aa719).
⚠️ Report is 20 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/datasource/src/file_scan_config/mod.rs 80.00% 2 Missing ⚠️
...usion/physical-plan/src/operator_statistics/mod.rs 80.00% 1 Missing ⚠️
datafusion/physical-plan/src/projection.rs 96.29% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25517      +/-   ##
==========================================
+ Coverage   82.38%   82.42%   +0.03%     
==========================================
  Files        1138     1138              
  Lines      434313   435405    +1092     
  Branches   434313   435405    +1092     
==========================================
+ Hits       357805   358868    +1063     
+ Misses      54876    54833      -43     
- Partials    21632    21704      +72     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@viirya
viirya requested a review from sunchao September 19, 2026 22:08

@alamb alamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@haohuaijin can you help review this PR?

@alamb

alamb commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Thanks @viirya -- would it be possible to get a end to end test for this fix? Specifically create a parquet file and then review the stats that come out. As an example, I think @haohuaijin did this in #25227

@haohuaijin haohuaijin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @viirya , LGTM!

I reproduced the null-count loss on main use below sql(because memtable only have null count, do not have min/max):

SET datafusion.explain.format = 'indent';
SET datafusion.explain.show_statistics = true;

CREATE TABLE t(a INT) AS VALUES (1), (NULL);
EXPLAIN SELECT CAST(a AS BIGINT) FROM t;

The scan reports Null=Exact(1), but the CAST projection drops it. This PR preserves that statistic.

@alamb alamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the review @haohuaijin

@github-actions github-actions Bot added the sqllogictest SQL Logic Tests (.slt) label Sep 20, 2026
@viirya

viirya commented Sep 20, 2026

Copy link
Copy Markdown
Member Author

Thanks for the suggestion! I added an end-to-end regression test based on the reproducer and verified it fails without the fix and passes with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

datasource Changes to the datasource crate physical-expr Changes to the physical-expr crates physical-plan Changes to the physical-plan crate sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants