fix: reject statistics whose column count does not match the schema by jackylee-ch · Pull Request #25694 · apache/datafusion

jackylee-ch · 2026-09-24T07:53:16Z

Which issue does this PR close?

N/A

Rationale for this change

Too few column statistics can make filters, joins, GROUP BY, count/min/max panic; too many can give min/max another column's values.

What changes are included in this PR?

Unless operators and providers return one ColumnStatistics per field, StatisticsContext fails planning naming them; FileScanConfig rejects more than one per column. count/min/max use .get(idx)? like sum: value_from_stats cannot return an error.

Not covered: file scans pad short statistics (virtual columns #22604, delta-io/delta-rs#4823 need it).

What is the testing strategy for this PR?

Unit tests per source, file scans, count/min/max; e2e tests. compute_statistics bench up to 10% slower; TPC-H/DS planning in noise.

Are there any user-facing changes?

Breaking for extensions (please add api change): such plans fail, plain scans too; see the upgrade guide.

count's statistics fast path indexed column_statistics[col.index()] directly, so a TableProvider reporting fewer column statistics than its schema made SELECT count(col) panic. Use .get(idx)? in both branches, matching the guard already in sum.rs.

codecov-commenter · 2026-09-24T08:18:11Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.87%. Comparing base (d93699c) to head (d1fb12f).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #25694   +/-   ##
=======================================
  Coverage   82.87%   82.87%           
=======================================
  Files        1153     1153           
  Lines      457043   457104   +61     
  Branches   457043   457104   +61     
=======================================
+ Hits       378777   378836   +59     
+ Misses      55268    55265    -3     
- Partials    22998    23003    +5

☔ 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 · 2026-10-14T07:31:30Z

        Ok(())
    }
+
+    #[test]


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.

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.

A statistics provider that drops the trailing column entry keeps the aggregated column's index out of bounds whatever the projection, so `count`'s statistics fast path is reached from SQL. The query panics on main and returns the correct count with the guard in place.

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 · 2026-10-15T08:06:52Z

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 · 2026-10-15T18:07:25Z

Which do you prefer?

I don't understand what you are asking me

What is the "broad gate"?

`Statistics::column_statistics` must hold one entry per schema field, and consumers index it by column. Check that where `StatisticsContext` receives statistics from an operator or a statistics provider, and fail with an error naming the culprit instead of panicking in a consumer.

`value_from_stats` is public, so a direct caller can still pass short statistics; fold the two `count` cases into one loop. Link `ColumnStatistics` in the `StatisticsProvider` docs.

`min` and `max` index `column_statistics` like `count` did, and `value_from_stats` is public, so use `.get(idx)?` there as well. The end-to-end test now also checks that a plain scan fails planning.

Extra entries never indexed out of bounds; only missing ones did.

`FilterExec` indexes its input's column statistics when it is built, so a table provider returning too few entries also panicked on `WHERE`.

A statistics provider returning the statistics of the whole table for a scan of only `b` used to make `min(b)` and `max(b)` return the minimum and maximum of `a`. Check end to end that planning fails instead, and that both statistics sources reject one entry too many as well as too few. Also pin what file scans accept: with a projection, as all built-in sources have, table and file group statistics of any length, missing trailing entries being unknown and extra ones ignored; without one, exactly one entry per table column. Say so in the upgrade guide.

`FileScanConfig` pads table and file group statistics it is given and ignores extra entries when its source has a projection, so a missing or extra entry before the last column shifts the statistics of the columns after it. Describe that as unchecked rather than as accepted.

`FileScanConfig` looks up the table and file group statistics it is given by column position, so an extra entry anywhere but at the end shifted the statistics of every later column by one, and `min`/`max` could be answered from another column's values. Return an internal error naming the statistics instead. Missing trailing entries are still unknown, which virtual columns and some table providers rely on.

jackylee-ch · 2026-10-16T14:18:16Z

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 · 2026-10-16T15:51:16Z

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:

What scenario are you hitting
What happens today
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

github-actions Bot added the functions Changes to functions implementation label Sep 24, 2026

Merge branch 'main' into fix-count-stats-guard

ccc8c84

kosiew approved these changes Oct 14, 2026

View reviewed changes

alamb requested changes Oct 14, 2026

View reviewed changes

jackylee-ch added 11 commits October 16, 2026 10:21

Merge remote-tracking branch 'upstream/main' into fix-count-stats-guard

7c95a05

test: describe why count still guards its statistics lookup

1720108

`value_from_stats` is public, so a direct caller can still pass short statistics; fold the two `count` cases into one loop. Link `ColumnStatistics` in the `StatisticsProvider` docs.

fix: guard min/max statistics lookup and test plain scans

d7dceba

`min` and `max` index `column_statistics` like `count` did, and `value_from_stats` is public, so use `.get(idx)?` there as well. The end-to-end test now also checks that a plain scan fails planning.

docs: say that too few column statistics caused the panics

7ac2405

Extra entries never indexed out of bounds; only missing ones did.

docs: list filters among the panics short statistics caused

6f5740f

`FilterExec` indexes its input's column statistics when it is built, so a table provider returning too few entries also panicked on `WHERE`.

Merge remote-tracking branch 'upstream/main' into fix-count-stats-guard

98ff84c

Merge remote-tracking branch 'upstream/main' into fix-count-stats-guard

7631f9a

github-actions Bot added documentation Improvements or additions to documentation core Core DataFusion crate datasource Changes to the datasource crate physical-plan Changes to the physical-plan crate labels Oct 16, 2026

jackylee-ch changed the title ~~fix: guard against out-of-bounds column statistics in count's fast path~~ fix: reject statistics whose column count does not match the schema Oct 16, 2026

Navigation Menu

Repository navigation

fix: reject statistics whose column count does not match the schema - #25694

fix: reject statistics whose column count does not match the schema#25694
jackylee-ch wants to merge 14 commits into
apache:mainfrom
jackylee-ch:fix-count-stats-guard

jackylee-ch commented Sep 24, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Sep 24, 2026 •

edited

Loading

Uh oh!

kosiew left a comment

Uh oh!

kosiew Oct 14, 2026

Uh oh!

jackylee-ch Oct 14, 2026 •

edited

Loading

Uh oh!

alamb left a comment

Uh oh!

jackylee-ch commented Oct 15, 2026

Uh oh!

alamb commented Oct 15, 2026

Uh oh!

jackylee-ch commented Oct 16, 2026

Uh oh!

alamb commented Oct 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

                       Ok(())
                   }
+                  #[test]

Conversation

jackylee-ch commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

What is the testing strategy for this PR?

Are there any user-facing changes?

Uh oh!

codecov-commenter commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kosiew left a comment

Choose a reason for hiding this comment

Uh oh!

kosiew Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

jackylee-ch Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

jackylee-ch commented Oct 15, 2026

Uh oh!

alamb commented Oct 15, 2026

Uh oh!

jackylee-ch commented Oct 16, 2026

Uh oh!

alamb commented Oct 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jackylee-ch commented Sep 24, 2026 •

edited

Loading

codecov-commenter commented Sep 24, 2026 •

edited

Loading

jackylee-ch Oct 14, 2026 •

edited

Loading