neilconway commented on code in PR #22675: URL: https://github.com/apache/datafusion/pull/22675#discussion_r3363437836
########## datafusion/physical-plan/src/materialized_cte.rs: ########## @@ -0,0 +1,621 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Physical plan nodes for materialized CTEs. + +use std::fmt; +use std::future::Future; +use std::sync::Arc; + +use crate::coop::cooperative; +use crate::execution_plan::{Boundedness, EmissionType, collect_partitioned}; +use crate::joins::utils::{OnceAsync, OnceFut}; +use crate::memory::MemoryStream; +use crate::metrics::{ExecutionPlanMetricsSet, MetricsSet}; +use crate::operator_statistics::StatisticsRegistry; +use crate::stream::RecordBatchStreamAdapter; +use crate::{ + DisplayAs, DisplayFormatType, ExecutionPlan, ExecutionPlanProperties, PlanProperties, + SendableRecordBatchStream, Statistics, +}; + +use arrow::datatypes::SchemaRef; +use arrow::record_batch::RecordBatch; +use datafusion_common::tree_node::{Transformed, TransformedResult, TreeNode}; +use datafusion_common::{Result, internal_err}; +use datafusion_execution::TaskContext; +use datafusion_physical_expr::{EquivalenceProperties, Partitioning}; +use futures::TryStreamExt; + +/// A shared cache that stores the materialized CTE results. +/// The cache uses `OnceAsync` to ensure the CTE is only computed once, +/// while allowing multiple consumers to await the result concurrently. +#[derive(Debug)] +pub struct MaterializedCteCache { + /// Name of the CTE (for debugging) + #[expect(dead_code)] + name: String, + /// The shared one-time async computation of the CTE batches + once: OnceAsync<Vec<Vec<RecordBatch>>>, +} + +impl MaterializedCteCache { + /// Create a new empty cache for the given CTE name. + pub fn new(name: String) -> Self { + Self { + name, + once: OnceAsync::default(), + } + } + + /// Get or initialize the cached batches via `OnceAsync::try_once`. + /// The first caller triggers computation; subsequent callers share the result. + pub(crate) fn try_once<F, Fut>(&self, f: F) -> Result<OnceFut<Vec<Vec<RecordBatch>>>> + where + F: FnOnce() -> Result<Fut>, + Fut: Future<Output = Result<Vec<Vec<RecordBatch>>>> + Send + 'static, + { + self.once.try_once(f) + } +} + +/// Physical execution plan that materializes a CTE and then executes +/// a continuation plan. The CTE results are cached in a shared +/// `MaterializedCteCache` for use by `MaterializedCteReaderExec` nodes. +#[derive(Debug)] +pub struct MaterializedCteExec { + /// Name of the CTE + name: String, + /// The plan that computes the CTE + cte_plan: Arc<dyn ExecutionPlan>, + /// The continuation plan that uses the materialized CTE + continuation: Arc<dyn ExecutionPlan>, + /// Shared cache for the CTE results + cache: Arc<MaterializedCteCache>, + /// Execution metrics + metrics: ExecutionPlanMetricsSet, + /// Cache holding plan properties + properties: Arc<PlanProperties>, +} + +impl MaterializedCteExec { + /// Create a new MaterializedCteExec. + pub fn new( + name: String, + cte_plan: Arc<dyn ExecutionPlan>, + continuation: Arc<dyn ExecutionPlan>, + cache: Arc<MaterializedCteCache>, + ) -> Self { + let properties = Arc::clone(continuation.properties()); + Self { + name, + cte_plan, + continuation, + cache, + metrics: ExecutionPlanMetricsSet::new(), + properties, + } + } +} + +impl DisplayAs for MaterializedCteExec { + fn fmt_as(&self, t: DisplayFormatType, f: &mut fmt::Formatter) -> fmt::Result { + match t { + DisplayFormatType::Default | DisplayFormatType::Verbose => { + write!(f, "MaterializedCteExec: name={}", self.name) + } + DisplayFormatType::TreeRender => { + write!(f, "name={}", self.name) + } + } + } +} + +impl ExecutionPlan for MaterializedCteExec { + fn name(&self) -> &'static str { + "MaterializedCteExec" + } + + fn properties(&self) -> &Arc<PlanProperties> { + &self.properties + } + + fn children(&self) -> Vec<&Arc<dyn ExecutionPlan>> { + vec![&self.cte_plan, &self.continuation] + } + + fn with_new_children( + self: Arc<Self>, + children: Vec<Arc<dyn ExecutionPlan>>, + ) -> Result<Arc<dyn ExecutionPlan>> { + if children.len() != 2 { + return internal_err!( + "MaterializedCteExec expected 2 children, got {}", + children.len() + ); + } + let cte_plan = Arc::clone(&children[0]); + let partition_count = cte_plan.output_partitioning().partition_count(); + let statistics = materialized_cte_statistics(cte_plan.as_ref())?; + let continuation = replace_materialized_cte_readers( + Arc::clone(&children[1]), + &self.name, + &self.cache, + partition_count, + &statistics, + )?; + Ok(Arc::new(Self::new( + self.name.clone(), + cte_plan, + continuation, + Arc::clone(&self.cache), + ))) + } + + fn execute( + &self, + partition: usize, + context: Arc<TaskContext>, + ) -> Result<SendableRecordBatchStream> { + let output_partitions = self.properties.output_partitioning().partition_count(); + if partition >= output_partitions { + return internal_err!( + "MaterializedCteExec got partition {partition}, expected less than {output_partitions}" + ); + } + + let cte_plan = Arc::clone(&self.cte_plan); + let continuation = Arc::clone(&self.continuation); + let name = self.name.clone(); + let ctx = Arc::clone(&context); + let schema = Arc::clone(&self.continuation.schema()); + + // Use OnceAsync to ensure the CTE is materialized exactly once, + // even when multiple partitions call execute() concurrently. + let mut once_fut = self.cache.try_once(move || { + Ok(async move { + let partitions = collect_partitioned(cte_plan, ctx).await?; + + let num_partitions = partitions.len(); + let num_batches: usize = partitions.iter().map(Vec::len).sum(); + let num_rows: usize = + partitions.iter().flatten().map(|b| b.num_rows()).sum(); + log::info!( + "Materializing CTE '{name}': {num_partitions} partitions, {num_batches} batches, {num_rows} rows" + ); + + Ok(partitions) + }) + })?; + + let ctx = Arc::clone(&context); + let fut = async move { + // Wait for the CTE to be materialized + std::future::poll_fn(|cx| once_fut.get_shared(cx)).await?; + // Now execute the continuation + continuation.execute(partition, ctx) + }; + + let stream = futures::stream::once(fut).try_flatten(); + Ok(Box::pin(RecordBatchStreamAdapter::new(schema, stream))) + } + + fn metrics(&self) -> Option<MetricsSet> { + Some(self.metrics.clone_inner()) + } + + fn partition_statistics(&self, _partition: Option<usize>) -> Result<Arc<Statistics>> { + Ok(Arc::new(Statistics::new_unknown( + &self.continuation.schema(), + ))) + } Review Comment: Why `new_unknown`? ########## datafusion/core/src/materialized_cte_planner.rs: ########## @@ -0,0 +1,154 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Extension planner for materialized CTEs. +//! +//! This module provides [`MaterializedCtePlanner`] which connects the logical +//! plan nodes ([`MaterializedCteProducer`] and [`MaterializedCteReader`]) to +//! their physical execution counterparts. + +use std::collections::HashMap; +use std::sync::{Arc, Mutex}; + +use async_trait::async_trait; +use datafusion_common::Result; +use datafusion_expr::logical_plan::{MaterializedCteProducer, MaterializedCteReader}; +use datafusion_expr::{LogicalPlan, UserDefinedLogicalNode}; +use datafusion_physical_plan::materialized_cte::{ + MaterializedCteCache, MaterializedCteExec, MaterializedCteReaderExec, + materialized_cte_statistics, replace_materialized_cte_readers, +}; +use datafusion_physical_plan::{ExecutionPlan, ExecutionPlanProperties}; + +use crate::execution::context::SessionState; +use crate::physical_planner::{ExtensionPlanner, PhysicalPlanner}; + +/// An extension planner that handles materialized CTE logical nodes. +/// +/// It maintains a map of CTE name to shared cache, ensuring that +/// producers and readers for the same CTE share the same cache instance. +#[derive(Debug)] +pub struct MaterializedCtePlanner { + /// Map of CTE name to shared cache + caches: Mutex<HashMap<String, Arc<MaterializedCteCache>>>, Review Comment: CTE names might not be unique, given nesting. For example: ```sql SELECT a.cnt AS acnt, b.cnt AS bcnt FROM (WITH t AS (SELECT column1 AS a FROM foo WHERE column1 <= 2) SELECT count(*) cnt FROM t x JOIN t y ON x.a = y.a) a, (WITH t AS (SELECT column1 AS a FROM foo WHERE column1 >= 4) SELECT count(*) cnt FROM t x JOIN t y ON x.a = y.a) b ``` ########## datafusion/physical-plan/src/materialized_cte.rs: ########## @@ -0,0 +1,621 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Physical plan nodes for materialized CTEs. + +use std::fmt; +use std::future::Future; +use std::sync::Arc; + +use crate::coop::cooperative; +use crate::execution_plan::{Boundedness, EmissionType, collect_partitioned}; +use crate::joins::utils::{OnceAsync, OnceFut}; +use crate::memory::MemoryStream; +use crate::metrics::{ExecutionPlanMetricsSet, MetricsSet}; +use crate::operator_statistics::StatisticsRegistry; +use crate::stream::RecordBatchStreamAdapter; +use crate::{ + DisplayAs, DisplayFormatType, ExecutionPlan, ExecutionPlanProperties, PlanProperties, + SendableRecordBatchStream, Statistics, +}; + +use arrow::datatypes::SchemaRef; +use arrow::record_batch::RecordBatch; +use datafusion_common::tree_node::{Transformed, TransformedResult, TreeNode}; +use datafusion_common::{Result, internal_err}; +use datafusion_execution::TaskContext; +use datafusion_physical_expr::{EquivalenceProperties, Partitioning}; +use futures::TryStreamExt; + +/// A shared cache that stores the materialized CTE results. +/// The cache uses `OnceAsync` to ensure the CTE is only computed once, +/// while allowing multiple consumers to await the result concurrently. +#[derive(Debug)] +pub struct MaterializedCteCache { + /// Name of the CTE (for debugging) + #[expect(dead_code)] + name: String, + /// The shared one-time async computation of the CTE batches + once: OnceAsync<Vec<Vec<RecordBatch>>>, +} + +impl MaterializedCteCache { + /// Create a new empty cache for the given CTE name. + pub fn new(name: String) -> Self { + Self { + name, + once: OnceAsync::default(), + } + } + + /// Get or initialize the cached batches via `OnceAsync::try_once`. + /// The first caller triggers computation; subsequent callers share the result. + pub(crate) fn try_once<F, Fut>(&self, f: F) -> Result<OnceFut<Vec<Vec<RecordBatch>>>> + where + F: FnOnce() -> Result<Fut>, + Fut: Future<Output = Result<Vec<Vec<RecordBatch>>>> + Send + 'static, + { + self.once.try_once(f) + } +} + +/// Physical execution plan that materializes a CTE and then executes +/// a continuation plan. The CTE results are cached in a shared +/// `MaterializedCteCache` for use by `MaterializedCteReaderExec` nodes. +#[derive(Debug)] +pub struct MaterializedCteExec { + /// Name of the CTE + name: String, + /// The plan that computes the CTE + cte_plan: Arc<dyn ExecutionPlan>, + /// The continuation plan that uses the materialized CTE + continuation: Arc<dyn ExecutionPlan>, + /// Shared cache for the CTE results + cache: Arc<MaterializedCteCache>, + /// Execution metrics + metrics: ExecutionPlanMetricsSet, + /// Cache holding plan properties + properties: Arc<PlanProperties>, +} Review Comment: We can't reuse `ScalarSubqueryExec`, but there are some similarities between what that node is trying to do and what we're trying to do here. It might be worth comparing the two implementations and making them consistent, where appropriate. For example, the way SSE does `maintains_input_order`, `partition_statistics`, and `cardinality_effect` are all probably worth adopting here. For `benefits_from_input_partitioning`, we probably want to say that the CTE might benefit from partitioning but the continuation does not? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
