Conversation
Codecov Report❌ Patch coverage is 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. 🚀 New features to boost your workflow:
|
alamb
left a comment
There was a problem hiding this comment.
@haohuaijin can you help review this PR?
|
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
left a comment
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
Thanks for the review @haohuaijin
|
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. |
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
Int32toInt64is 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?
ProjectionExprs::project_statistics_with_input_schema, which uses theinput schema to identify lossless casts when typed extrema are unavailable.
values, or change ordering.
ProjectionExec, extended statisticsprojection, and file scan statistics projection.
ProjectionExecregression tests covering anInt32toInt64cast with absent extrema but exact null and distinct counts.The existing
project_statisticsAPI remains available, so this is anadditive, non-breaking API change.
What is the testing strategy for this PR?
Added:
test_project_statistics_safe_cast_without_extrematest_projection_statistics_safe_cast_without_extremaAblation testing confirmed that the old implementation changes
null_count = Exact(3)toAbsent. Both the helper-level and productionProjectionExecpaths preserve the exact null and distinct counts after thefix.
Also verified:
cargo fmt --all -- --checkgit diff --checkThe 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.