Skip to content

Commit a204864

Browse files
committed
fix: amend RecursiveQuery structure and schema handling
- Remove schema field from RecursiveQuery - Update recompute_schema() to re-widen recursive CTE children as necessary - Retain widened schema through child projections post recompute drift - Adjust logical-plan SLT expectation for newly inserted projection
1 parent 7606c81 commit a204864

4 files changed

Lines changed: 42 additions & 43 deletions

File tree

datafusion/expr/src/logical_plan/builder.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,13 +215,11 @@ impl LogicalPlanBuilder {
215215
Arc::unwrap_or_clone(self.plan),
216216
Arc::clone(&output_schema),
217217
)?;
218-
let recursive_term =
219-
plan_with_schema(coerced_recursive_term, Arc::clone(&output_schema))?;
218+
let recursive_term = plan_with_schema(coerced_recursive_term, output_schema)?;
220219
Ok(Self::from(LogicalPlan::RecursiveQuery(RecursiveQuery {
221220
name,
222221
static_term: Arc::new(static_term),
223222
recursive_term: Arc::new(recursive_term),
224-
schema: output_schema,
225223
is_distinct,
226224
})))
227225
}

datafusion/expr/src/logical_plan/plan.rs

Lines changed: 38 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ use arrow::datatypes::{DataType, Field, FieldRef, Schema, SchemaRef};
5454
use datafusion_common::cse::{NormalizeEq, Normalizeable};
5555
use datafusion_common::format::ExplainFormat;
5656
use datafusion_common::metadata::check_metadata_with_storage_equal;
57+
use datafusion_common::recursive_schema::recursive_query_output_schema;
5758
use datafusion_common::tree_node::{
5859
Transformed, TreeNode, TreeNodeContainer, TreeNodeRecursion,
5960
};
@@ -353,7 +354,11 @@ impl LogicalPlan {
353354
LogicalPlan::Copy(CopyTo { output_schema, .. }) => output_schema,
354355
LogicalPlan::Ddl(ddl) => ddl.schema(),
355356
LogicalPlan::Unnest(Unnest { schema, .. }) => schema,
356-
LogicalPlan::RecursiveQuery(RecursiveQuery { schema, .. }) => schema,
357+
LogicalPlan::RecursiveQuery(RecursiveQuery { static_term, .. }) => {
358+
// The static term is coerced to the recursive output schema when
359+
// building a RecursiveQuery.
360+
static_term.schema()
361+
}
357362
}
358363
}
359364

@@ -737,7 +742,33 @@ impl LogicalPlan {
737742
};
738743
Ok(LogicalPlan::Distinct(distinct))
739744
}
740-
LogicalPlan::RecursiveQuery(_) => Ok(self),
745+
LogicalPlan::RecursiveQuery(RecursiveQuery {
746+
name,
747+
static_term,
748+
recursive_term,
749+
is_distinct,
750+
}) => {
751+
let output_schema = recursive_query_output_schema(
752+
static_term.schema(),
753+
recursive_term.schema(),
754+
)?;
755+
if output_schema == *static_term.schema() {
756+
Ok(LogicalPlan::RecursiveQuery(RecursiveQuery {
757+
name,
758+
static_term,
759+
recursive_term,
760+
is_distinct,
761+
}))
762+
} else {
763+
LogicalPlanBuilder::from(Arc::unwrap_or_clone(static_term))
764+
.to_recursive_query(
765+
name,
766+
Arc::unwrap_or_clone(recursive_term),
767+
is_distinct,
768+
)?
769+
.build()
770+
}
771+
}
741772
LogicalPlan::Analyze(_) => Ok(self),
742773
LogicalPlan::Explain(_) => Ok(self),
743774
LogicalPlan::TableScan(_) => Ok(self),
@@ -1073,20 +1104,13 @@ impl LogicalPlan {
10731104
Ok(LogicalPlan::Distinct(distinct))
10741105
}
10751106
LogicalPlan::RecursiveQuery(RecursiveQuery {
1076-
name,
1077-
schema,
1078-
is_distinct,
1079-
..
1107+
name, is_distinct, ..
10801108
}) => {
10811109
self.assert_no_expressions(expr)?;
10821110
let (static_term, recursive_term) = self.only_two_inputs(inputs)?;
1083-
Ok(LogicalPlan::RecursiveQuery(RecursiveQuery {
1084-
name: name.clone(),
1085-
static_term: Arc::new(static_term),
1086-
recursive_term: Arc::new(recursive_term),
1087-
schema: Arc::clone(schema),
1088-
is_distinct: *is_distinct,
1089-
}))
1111+
LogicalPlanBuilder::from(static_term)
1112+
.to_recursive_query(name.clone(), recursive_term, *is_distinct)?
1113+
.build()
10901114
}
10911115
LogicalPlan::Analyze(a) => {
10921116
self.assert_no_expressions(expr)?;
@@ -2247,7 +2271,7 @@ impl PartialOrd for EmptyRelation {
22472271
/// intermediate table, then empty the intermediate table.
22482272
///
22492273
/// [Postgres Docs]: https://www.postgresql.org/docs/current/queries-with.html#QUERIES-WITH-RECURSIVE
2250-
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
2274+
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Hash)]
22512275
pub struct RecursiveQuery {
22522276
/// Name of the query
22532277
pub name: String,
@@ -2256,33 +2280,11 @@ pub struct RecursiveQuery {
22562280
/// The recursive term (evaluated on the contents of the working table until
22572281
/// it returns an empty set)
22582282
pub recursive_term: Arc<LogicalPlan>,
2259-
/// Output schema for the recursive query. This preserves the static term's
2260-
/// names, types, and metadata, with nullability widened by the recursive term.
2261-
pub schema: DFSchemaRef,
22622283
/// Should the output of the recursive term be deduplicated (`UNION`) or
22632284
/// not (`UNION ALL`).
22642285
pub is_distinct: bool,
22652286
}
22662287

2267-
// Manual implementation needed because of `schema` field.
2268-
impl PartialOrd for RecursiveQuery {
2269-
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
2270-
(
2271-
&self.name,
2272-
&self.static_term,
2273-
&self.recursive_term,
2274-
self.is_distinct,
2275-
)
2276-
.partial_cmp(&(
2277-
&other.name,
2278-
&other.static_term,
2279-
&other.recursive_term,
2280-
other.is_distinct,
2281-
))
2282-
.filter(|cmp| *cmp != Ordering::Equal || self == other)
2283-
}
2284-
}
2285-
22862288
/// Values expression. See
22872289
/// [Postgres VALUES](https://www.postgresql.org/docs/current/queries-values.html)
22882290
/// documentation for more details.

datafusion/expr/src/logical_plan/tree_node.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -328,15 +328,13 @@ impl TreeNode for LogicalPlan {
328328
name,
329329
static_term,
330330
recursive_term,
331-
schema,
332331
is_distinct,
333332
}) => (static_term, recursive_term).map_elements(f)?.update_data(
334333
|(static_term, recursive_term)| {
335334
LogicalPlan::RecursiveQuery(RecursiveQuery {
336335
name,
337336
static_term,
338337
recursive_term,
339-
schema,
340338
is_distinct,
341339
})
342340
},

datafusion/sqllogictest/test_files/cte.slt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1196,8 +1196,9 @@ logical_plan
11961196
03)----RecursiveQuery: is_distinct=false
11971197
04)------Projection: Int64(0) AS k, Int64(0) AS v
11981198
05)--------EmptyRelation: rows=1
1199-
06)------Sort: r.v ASC NULLS LAST, fetch=1
1200-
07)--------TableScan: r projection=[k, v]
1199+
06)------Projection: r.k, r.v
1200+
07)--------Sort: r.v ASC NULLS LAST, fetch=1
1201+
08)----------TableScan: r projection=[k, v]
12011202
physical_plan
12021203
01)GlobalLimitExec: skip=0, fetch=5
12031204
02)--RecursiveQueryExec: name=r, is_distinct=false

0 commit comments

Comments
 (0)