[jackylee-ch]

Which issue does this PR close?

• Related to analysis.rs bounds check panic #10536, where planning panicked on a user's custom statistics; propagate error instead of panicking on out of bounds in physical-expr/src/analysis.rs  #10992 turned only that panic, in ExprBoundaries::try_from_column, into an error.

Rationale for this change

Extensions can report their own Statistics: a custom ExecutionPlan, such as a table provider's scan, or a StatisticsProvider registered on the session. column_statistics must have one entry per field of the plan's schema, but nothing validates this when statistics are reported, and it is easy to get wrong: starting from Statistics::default() gives no entries, and returning a child's statistics from a plan with fewer columns gives too many.

Three downstream projects have hit this:

• fix(datafusion): schema-size ProximaScanExec stats (DF54 aggregate panic) anvai-labs/proximaDB#76: after upgrading to DataFusion 54, aggregate queries over its scan panicked in AggregateExec (index out of bounds: the len is 0 but the index is 0), because the scan fell back to Statistics::default().
• build: DataFusion 53, Arrow 58, Lance 8.0.1, iceberg 0.10.1 SkardiLabs/skardi#276: when upgrading to DataFusion 53, KNN queries with a compound WHERE panicked in ProjectionExprs::project_statistics, because LanceKnnExec reported no column statistics.
• Caching acceleration: integer-column filter fails with Internal error (ExprBoundaries col_index out of bounds) spiceai/spiceai#14144: filters on integer columns of cache-accelerated tables failed with Internal error: Could not create ExprBoundaries: ... col_index has gone out of bounds, because a wrapper around the scan that hides internal columns passed on its input's column statistics unchanged (Fix SchemaCastScanExec's schema conversion in fn partition_statistics spiceai/spiceai#14258).

Each failure surfaced far from its cause, as an index out of bounds, with nothing naming the plan that reported the statistics. Rules and operators read a column's statistics by its index, so a mismatch stays hidden until some code reads that column, and a DataFusion upgrade can add such code. Too many entries can also give wrong answers: in a test where a scan of b got the statistics of both a and b, SELECT min(b), max(b) returned the min and max of a.

What changes are included in this PR?

The planner and optimizer rules get plan statistics through StatisticsContext. It now checks that the statistics of every plan and statistics provider have one entry per field, so a rule or operator that would read wrong statistics gets an error instead, naming the plan or provider that reported them, e.g. MyExec returned 1 column statistics, expected 2 (one per field of its schema). The default EnsureRequirements rule, which adds the repartitions and sorts a plan needs to run, computes the statistics of each operator's input, so even a plain SELECT * over such a plan fails at planning with this error. Every other query we tried, from filters to joins and aggregates, failed the same way, so an extension's own tests should catch the mistake. Padding with unknown statistics instead would hide it.

value_from_stats, which computes an aggregate from statistics alone, is public, so callers can pass it statistics that never went through this check, and it returns an Option, so it cannot report an error. The count, min and max implementations therefore return None on too few entries instead of panicking, as sum's already does.

Not covered: the table-level and per-file statistics that a table provider passes to a file scan. Files can legitimately lack statistics for some columns (virtual columns have none; delta-rs skips them when there is no predicate to prune with), so a missing entry there is not a bug, and the built-in file scans fill it in as unknown. That layer needs a different rule, allowing missing entries but not extra ones, so this PR keeps to one rule in one place and drops the extra-entries check an earlier version added there. The statistics the scan itself reports are still checked.

What is the testing strategy for this PR?

End-to-end tests in core/tests/physical_optimizer/aggregate_statistics.rs reproduce too few and too many entries from SQL (on main they panic or return the wrong min/max) and check the new error. Unit tests cover plans and providers reporting too few and too many entries. The check is one length comparison, about 5 ns, each time a plan's statistics are computed. The compute_statistics microbenchmarks, which only compute statistics, are up to 7% slower. Total planning time, this PR vs main (median of 3 runs): TPC-H (21 queries, all but q15) 15.24 vs 15.17 ms; TPC-DS (99 queries) 240.3 vs 240.1 ms.

Are there any user-facing changes?

Yes, breaking for extensions: a plan or statistics provider with the wrong number of column statistics now fails planning, even for queries such as SELECT * that worked before. The upgrade guide describes the fix: start from Statistics::new_unknown(&schema), or project a child's statistics to the plan's columns with Statistics::project. Please add the api change label.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.94%. Comparing base (3ee3e39) to head (9313de8).
⚠️ Report is 131 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #25694   +/-   ##
=======================================
  Coverage   82.93%   82.94%           
=======================================
  Files        1154     1154           
  Lines      458002   458075   +73     
  Branches   458002   458075   +73     
=======================================
+ Hits       379856   379930   +74     
+ Misses      55123    55122    -1     
  Partials    23023    23023           

☔ 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.

[kosiew]

@jackylee-ch,

Thanks for fixing this. Both COUNT and COUNT(DISTINCT) now handle missing column statistics safely, and the new regression tests cover both paths. The change looks good to me. I have one optional suggestion for additional end-to-end coverage.

[kosiew]

It would be useful to add an end-to-end regression test with incomplete column statistics, NULLs, and duplicates to verify that aggregation is preserved and returns the correct counts. Please test COUNT(b) and COUNT(DISTINCT b) separately, since the optimizer stops at the first unresolved expression, and make sure the column index remains out of bounds after any projection or remapping. This is optional since the existing unit tests already cover both guarded accesses.

[jackylee-ch]

Edit: superseded by d032a22. Short column statistics now fail planning with an error naming the provider, which also covers COUNT(DISTINCT b) (rewritten into a GROUP BY), and the end-to-end test short_column_statistics_are_reported checks that error for count(b) and a plain scan. An earlier version of this reply said test_count_distinct_optimization gained a short-statistics case; that was wrong: the case passed without the guard and was never pushed. The count unit test covers both branches.

[alamb]

Thank you for this PR @jackylee-ch and for the review @kosiew

I feel like this is just papering over a bug and by sliently ignoring the problem we will make debugging harder

Changing the code so it doesn't panic seems fine, but if the table provider returns the wrong number of statistics I think that should return a clear error

[jackylee-ch]

Agreed — it should be an error. Two candidates: the provider/source boundary, or StatisticsContext::compute_base, where provider and built-in statistics meet; that one also covers min_max and AggregateExec, which index it the same way.

One thing first: planning-time rules compute statistics (join_selection, limit_pushdown), so the broad gate also fails SELECT * ... LIMIT at planning time, not only count. With it the sqllogictests and core_integration pass; in physical-plan, two StatisticsProvider fixtures reporting no column statistics fail. Which do you prefer?

[alamb]

Which do you prefer?

I don't understand what you are asking me

What is the "broad gate"?

[jackylee-ch]

Sorry, that was unclear. "Broad gate" meant one check in StatisticsContext, which sees what every operator and provider returns. This PR adds it: unless column_statistics has one entry per field, planning fails with an error naming the culprit.

Too few entries can make filters, joins, GROUP BY, count, min/max panic, too many can give min/max another column's values; such plans now fail, plain scans too. File scans also reject extras but still pad short statistics (virtual columns, delta-rs). count/min/max use .get(idx)?: value_from_stats cannot return an error.

[alamb]

I am sorry -- I still don't get it.

How about we start with a clear and concise description of what problem you are solving (from the end user API perspective, not a code description)

Describe:

1. What scenario are you hitting
2. What happens today
3. What your proposed behavior ?

It seems like you are trying to add better error messages when a table provider provides incorrect statistics ? But that is not clear

[jackylee-ch]

I've updated the description; does it answer your questions? Your guess is right: a clear planning error instead of a panic or a wrong answer. In short:

1. Scenario: a table provider's scan reports statistics for the wrong number of columns. In fix(datafusion): schema-size ProximaScanExec stats (DF54 aggregate panic) anvai-labs/proximaDB#76, the scan fell back to Statistics::default(), which has no column entries; the description has two more cases.
2. Today: rules and operators read column statistics by position, and nothing checks the count when a plan reports them, so a mismatch surfaces wherever they are first read. DataFusion 54's AggregateExec indexes its input's column statistics, so after upgrading, proximaDB's aggregate queries panicked there (index out of bounds: the len is 0 but the index is 0), with nothing pointing at the scan. With too many entries, min/max can return another column's values.
3. Proposed: check the count whenever statistics are reported for a plan node, and fail planning with an error naming that node, e.g. MyExec returned 1 column statistics, expected 2 (one per field of its schema). Queries over such a scan then fail at planning, even SELECT *, which works today; the upgrade guide shows the fix.