Skip to content

feat: report statistics of materialized CTE scans - #26271

Merged
alamb merged 2 commits into
apache:mainfrom
adriangb:mcte-scan-properties
Oct 27, 2026
Merged

alamb merged 2 commits into
apache:mainfrom
adriangb:mcte-scan-properties

Conversation

@adriangb

@adriangb adriangb commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

A reference to a MATERIALIZED CTE reported unknown statistics. A query could then get a worse plan than the same query with the CTE inlined.

SET datafusion.sql_parser.dialect = 'PostgreSQL';
CREATE TABLE small AS SELECT value AS k FROM generate_series(1, 5);
CREATE TABLE huge  AS SELECT value AS k FROM generate_series(1, 100000);

EXPLAIN WITH c AS MATERIALIZED (SELECT k FROM small)
SELECT count(*) FROM huge JOIN c ON huge.k = c.k;

Before, the hash table is built on the 100000 rows of huge:

HashJoinExec: mode=CollectLeft, join_type=Inner, on=[(k@0, k@0)], projection=[]
  DataSourceExec: partitions=1, partition_sizes=[13]
  MaterializedCteScanExec: name=c

After, it is built on the 5 rows of the CTE:

HashJoinExec: mode=CollectLeft, join_type=Inner, on=[(k@0, k@0)], projection=[]
  MaterializedCteScanExec: name=c
  DataSourceExec: partitions=1, partition_sizes=[13]

What changes are included in this PR?

The scan is a leaf and the body is a child of MaterializedCteExec, so the planner copies the statistics to the scan in DefaultPhysicalPlanner::optimize_physical_plan, before the optimizer runs. The optimizer runs one time, as before. The EXPLAIN paths use the same function.

Each scan gets the statistics of the body, computed with StatisticsContext. A body that reads another CTE gets the statistics of that CTE first. A rewrite of the body does not change its rows, so the statistics stay correct. The statistics of one scan partition are known only when the scan has one partition. The statistics API is not changed.

Other plan changes that follow from the statistics: an exact row count of the body can answer count(*), and the body then does not run. One pinned plan in cte_materialized.slt has a new join order.

The ordering and the hash partitioning of the scans are a follow-up: #26424.

The scan gets the complete statistics of the body, not only the row count: column min, max, null count, distinct count and byte size. Thus the outer query uses the column statistics of the CTE. For example, a filter above the scan derives its bounds from the column bounds of the body, the same as in the inlined plan. The statistics of one partition are given only when the scan has one partition. Otherwise they are unknown, because scan partitions do not map to body partitions.

Comparison with PostgreSQL and DuckDB. This is what PostgreSQL and DuckDB do. PostgreSQL gives a CTE scan the row estimate of the CTE plan (allpaths.c). DuckDB copies the statistics of the body to each reference (propagate_cte.cpp).

Are these changes tested?

Yes. cte_materialized.slt: the join-order plan, the statistics in EXPLAIN, count(*) from exact statistics, a chain of two CTEs, a scalar subquery and a view. A unit test for the statistics of a scan.

Are there any user-facing changes?

Plans of queries with MATERIALIZED CTEs can change. New public functions: MaterializedCteScanExec::with_body_statistics and MaterializedCteExec::body.

@github-actions github-actions Bot added sql SQL Planner logical-expr Logical plan and expressions optimizer Optimizer rules core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) physical-plan Changes to the physical-plan crate labels Oct 13, 2026
@codecov-commenter

codecov-commenter commented Oct 13, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.44371% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.92%. Comparing base (484183d) to head (a95795d).
⚠️ Report is 18 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/core/src/physical_planner.rs 76.81% 10 Missing and 6 partials ⚠️
datafusion/physical-plan/src/materialized_cte.rs 89.02% 2 Missing and 7 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #26271      +/-   ##
==========================================
+ Coverage   82.89%   82.92%   +0.02%     
==========================================
  Files        1154     1155       +1     
  Lines      457630   458327     +697     
  Branches   457630   458327     +697     
==========================================
+ Hits       379370   380055     +685     
+ Misses      55256    55222      -34     
- Partials    23004    23050      +46     

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

@adriangb
adriangb force-pushed the mcte-scan-properties branch 2 times, most recently from 2273e33 to f5ab4f7 Compare October 16, 2026 03:46
@github-actions github-actions Bot removed sql SQL Planner logical-expr Logical plan and expressions optimizer Optimizer rules labels Oct 16, 2026
@adriangb
adriangb marked this pull request as ready for review October 16, 2026 16:57
@adriangb

Copy link
Copy Markdown
Contributor Author

A query with a MATERIALIZED CTE whose body has an ordering or a hash partitioning runs the physical optimizer two times, or up to four times for a chain of CTEs.

Hmm not so sure about this. We should consider if this makes sense.

@adriangb
adriangb force-pushed the mcte-scan-properties branch from f5ab4f7 to a442b87 Compare October 16, 2026 19:46
@adriangb adriangb changed the title feat: report statistics, ordering and partitioning of materialized CTE scans feat: report statistics of materialized CTE scans Oct 16, 2026
@adriangb
adriangb force-pushed the mcte-scan-properties branch 2 times, most recently from db48d4e to 19fa23e Compare October 17, 2026 21:58

@asolimando asolimando left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, it feels like landing #26424 would make our life easier on a few places (I left two comments), but nothing major/blocking.

Happy to see better CTE support landing, thanks for your work!

id: u64,
name: String,
schema: SchemaRef,
/// The statistics of all partitions of the body.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Today we don't push-down filters, limit or other stats-affecting operators down into CTEs (or, more probable, their scans), so copying the stats as-is is safe, but if we introduce something like this limit pushdown for MemorySourceConfig data source, that wouldn't be safe anymore, and I have seen/fixed such cases elsewhere, very easy to miss.

So I wonder if we should state this invariant as a comment, in case someone adds a similar pushdown later?

Comment thread datafusion/core/src/physical_planner.rs Outdated
///
/// Before the optimizers run, each [`MaterializedCteScanExec`] is bound
/// to the buffer of its [`MaterializedCteExec`] and gets the statistics
/// of the CTE body. A rewrite of the body does not change its rows, so

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is only strictly true for perfect statistics propagation (unhandled cases today can turn stats into Absent for some rewrites) and for row count at most, many column stats are not guaranteed to stay the same upon rewrites.

Consider a filter pushdown rewriting Filter(Join(A,B)) into Join(Filter(A),B), the interval-derived min/max and NDV are very probably going to be different.

It seems to me that we should optimize the body first, and then snapshot as #26424 proposes for ordering/partitioning. Statistics are finally just another metadata exactly like those and would benefit from the same treatment.

WDYT?

/// plan of the inlined CTE. For example, `JoinSelection` does not know
/// that a scan is small, and can put the other, larger input of a hash
/// join on the build side.
/// The statistics of one partition are known only when the scan has one

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We could again wait for #26424, which will give us all the missing information to handle the case with multiple partitions

query TT
EXPLAIN ANALYZE WITH c AS MATERIALIZED (SELECT k, v FROM t)
SELECT count(*) FROM (
SELECT sum(k) FROM (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Q: IIUC this query change is needed to avoid AggregateStatistics answering the count(*) part with the Exact row count, right?

adriangb and others added 2 commits October 24, 2026 09:08
A `MaterializedCteScanExec` is a leaf, and the CTE body is a child of `MaterializedCteExec`, so the scan reported unknown statistics.
`JoinSelection` could then build a hash table on a large table and not on a small CTE.

`optimize_physical_plan` now binds the scans and gives each scan the statistics of the CTE body, before the optimizer runs.
A body that reads another CTE gets the statistics of that CTE first.
A rewrite of the body does not change its rows, so these statistics stay correct.
The EXPLAIN paths use the same function, so they show the same plan.

Closes apache#26230.
where
F: FnMut(&dyn ExecutionPlan, &dyn PhysicalOptimizerRule),
{
let plan = bind_materialized_cte_scans(plan, session_state)?;

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.

Since the scans now carry exact statistics, AggregateStatistics often folds a count(*) over a scan into a constant and drops the scan. That scan was already bound here, though, and bind pushed its partition count into scan_partitions. So the replay reservation still includes readers that never run. With target_partitions = 1 and memory_limit = '300K', this fails while asking for 576 KB, nine scans' worth, even though only the sum scan is left:

WITH g AS MATERIALIZED (SELECT value AS x FROM generate_series(1, 200000))
SELECT count(*) FROM g UNION ALL SELECT count(*) FROM g UNION ALL SELECT count(*) FROM g
UNION ALL SELECT count(*) FROM g UNION ALL SELECT count(*) FROM g UNION ALL SELECT count(*) FROM g
UNION ALL SELECT count(*) FROM g UNION ALL SELECT count(*) FROM g UNION ALL SELECT sum(x) FROM g;

Main reserves the same amount for this query, so it isn't a regression, and a follow-up is fine. Could the partition counts be registered from the scans in the final plan instead, for example after the optimizers or when MaterializedCteExec::execute sets the body?

@adriangb

Copy link
Copy Markdown
Contributor Author

@asolimando Thank you for the review. I agree with your point about the statistics. I propose this:

  1. Merge feat: report the ordering and partitioning of materialized CTE scans #26428 into this PR.
  2. Optimize each CTE body one time, before the rest of the plan. Then give each scan the statistics, ordering and partitioning of the optimized body. Do not optimize the body again.
  3. Optimize the rest of the plan.

This way, the scan always reports the correct properties of the body. #26428 does not need its check and its fallback. The extra planning time is small, about 0.04 ms for one CTE (measured with #26428). I will keep the statistics and the properties in separate commits. Do you agree?

@alamb
alamb added this pull request to the merge queue Oct 27, 2026
@alamb

alamb commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Merging to keep things moving. Thank you everyone

Merged via the queue into apache:main with commit d48480e Oct 27, 2026
45 checks passed
@adriangb

Copy link
Copy Markdown
Contributor Author

@alamb i think there’s still some discussion to be had here. merging now might cause some churn. i’m happy to handle it that way, up to you. this is all new / unstable code so im not too worried either way.

@adriangb
adriangb deleted the mcte-scan-properties branch October 27, 2026 21:18
@alamb

alamb commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

@alamb i think there’s still some discussion to be had here. merging now might cause some churn. i’m happy to handle it that way, up to you. this is all new / unstable code so im not too worried either way.

Sorry -- I clicked the button. Should we revert this?

I am trying to work through our PR backlog but I maybe was overzealous

@adriangb

Copy link
Copy Markdown
Contributor Author

no worries, i think it’s fine. it only impacts people who explicitly upped to materialize CTE, which is a new unstable feature.

galaxytemple pushed a commit to galaxytemple/datafusion that referenced this pull request Nov 3, 2026
…27005)

## Which issue does this PR close?

- Related to apache#26424 and apache#26428. No
issue of its own: this PR only adds benchmarks.

## Rationale for this change

`MATERIALIZED` CTEs (apache#26271) are planned differently
from inlined CTEs: each body is optimized on its own before the rest of
the plan. apache#26428 adds more work to that step. No
planner benchmark plans a materialized CTE today, so there is no
standard way to see what this costs, or to compare it with inlining.

## What changes are included in this PR?

New `sql_planner` cases, which plan a query from SQL to a physical plan.
The PostgreSQL dialect is used, because sqlparser only parses `[NOT]
MATERIALIZED` for that dialect. For each kind of CTE (unmarked,
`MATERIALIZED`, `NOT MATERIALIZED`) there are three cases:

- `physical_cte_<kind>_one_ref`: the CTE is read once.
- `physical_cte_<kind>_two_refs`: the CTE is joined with itself.
- `physical_cte_<kind>_chain_6`: six CTEs, each joining the previous one
with itself.

`physical_cte_mixed` has one CTE of each kind, each read twice.

Results on `main` (Criterion means):

| Case | unmarked | `MATERIALIZED` | `NOT MATERIALIZED` |
| --- | --- | --- | --- |
| one reference | 242 µs | 264 µs | 245 µs |
| two references | 474 µs | 360 µs | 481 µs |
| chain of 6 | 8.25 ms | 0.92 ms | 8.33 ms |

`physical_cte_mixed` takes 1.36 ms.

An inlined chain copies the first CTE 32 times. So it takes about 9
times as long to plan as the materialized chain.

## What is the testing strategy for this PR?

This PR has only benchmarks. They run with:

```bash
cargo bench -p datafusion --bench sql_planner -- physical_cte
```

The `sql_planner` benchmark needs ClickBench data. The checked-in
fixture works: put
`datafusion/core/tests/data/clickbench_hits_10.parquet` in a directory,
and set `CLICKBENCH_DATA_PATH` to that directory.

## Are there any user-facing changes?

No.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate physical-plan Changes to the physical-plan crate sqllogictest SQL Logic Tests (.slt) v56.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Materialized CTE scans report unknown statistics, ordering and partitioning

5 participants