Repository navigation
Avoid inlining non deterministic CTE #10337
Description
Describe the bug
Currently Datafusion will inline all CTE, a non-deterministic expression can be executed multiple times producing different results
To Reproduce
Consider the following query which uses the aggregate_test_100 data from datafusion-examples. Here, column c11 is a Float64
WITH cte as (
SELECT sum(c4 * c11) as total
FROM aggregate_test_100
GROUP BY c1)
SELECT total
FROM cte
WHERE total = (select max(total) from cte)
The optimized plan generated will inline the CTE and thus execute it twice
Projection: cte.total
Inner Join: cte.total = __scalar_sq_1.MAX(cte.total)
SubqueryAlias: cte
Projection: SUM(aggregate_test_100.c4 * aggregate_test_100.c11) AS total
Aggregate: groupBy=[[aggregate_test_100.c1]], aggr=[[SUM(CAST(aggregate_test_100.c4 AS Float64) * aggregate_test_100.c11)]]
TableScan: aggregate_test_100 projection=[c1, c4, c11]
SubqueryAlias: __scalar_sq_1
Aggregate: groupBy=[[]], aggr=[[MAX(cte.total)]]
SubqueryAlias: cte
Projection: SUM(aggregate_test_100.c4 * aggregate_test_100.c11) AS total
Aggregate: groupBy=[[aggregate_test_100.c1]], aggr=[[SUM(CAST(aggregate_test_100.c4 AS Float64) * aggregate_test_100.c11)]]
TableScan: aggregate_test_100 projection=[c1, c4, c11]
Expected behavior
Since summation here is dependent on ordering, I believe it is incorrect to inline the CTE here and execute it more than once.
Additional context
Related issue, which talks about possible advantages on not inlining CTE in some cases: #8777
Activity
tgujar commentedon May 3, 2024
I can work on this if we can confirm in this is indeed a correctly reported bug. Let me know what you think, thanks!
jonahgao commentedon May 6, 2024
I worry that judging whether a query is non-deterministic may be not easy. Perhaps we can first leave this judgment to the user, only do this when the user specifies Materialized and there are multiple references.
DuckDB may have taken a similar approach.
tgujar commentedon May 7, 2024
I think this would push the responsibility to the user to figure out what may be non-deterministic. I am not sure if this would be a good approach
jonahgao commentedon May 8, 2024
Due to parallel execution, non-deterministic behavior occurs more frequently than in traditional databases. I'm not sure if it's appropriate to disable inline CTE for all non-deterministic queries, as inline has advantages in some scenarios.
Additionally, I believe that detecting non-deterministic behavior is more difficult than in traditional databases, as it is affected by whether the input is multi-partitioned, the configuration of target_partitions, whether a Repartition node has been added, and some queries, like MIN, may be immune to unordered execution, etc.
So I think it might be a good starting point to have the user specify explicitly. When users want to avoid non-deterministic behavior or recomputation, they can explicitly request no inlining.
neilconway commentedon Jun 5, 2026
On defining which expressions are non-deterministic: I believe we can just use the existing concept of volatile functions. CTEs containing such expressions should not be inlined by default.
fix: don't duplicate volatile expressions when pushing projection int…
fix: don't duplicate volatile expressions when pushing projection int…
[branch-54] fix: don't duplicate volatile expressions when pushing pr…
feat: support `WITH ... AS [NOT] MATERIALIZED` CTEs (apache#25580)
feat: support `WITH ... AS [NOT] MATERIALIZED` CTEs (apache#25580)
feat: support `WITH ... AS [NOT] MATERIALIZED` CTEs (apache#25580)