Skip to content

Commit 58071af

Browse files
committed
feat(recursive-cte): implement join-link based working-table iteration
Add a join_link field (collection_field, working_table_field) to RecursiveScan that drives proper tree-traversal CTEs. Each iteration now builds a hash-set of values from the frontier's working_field and finds collection rows whose collection_field is in that set, matching the SQL INNER JOIN ON semantics of standard recursive CTEs. Previously the recursive step applied filters to the full collection without any join relationship to the previous iteration, producing incorrect results for parent/child tree queries. Also handle strict (Binary Tuple) encoded collections in the recursive executor by converting through the schema before filter evaluation.
1 parent 08b1227 commit 58071af

9 files changed

Lines changed: 482 additions & 99 deletions

File tree

nodedb-sql/src/planner/cte.rs

Lines changed: 214 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,15 @@ use sqlparser::ast::{self, Query, SetExpr};
44

55
use crate::error::{Result, SqlError};
66
use crate::functions::registry::FunctionRegistry;
7+
use crate::parser::normalize::{normalize_ident, normalize_object_name};
78
use crate::types::*;
89

910
/// Plan a WITH RECURSIVE query.
11+
///
12+
/// Supports table-based recursive CTEs where the base case scans a real
13+
/// collection and the recursive step references both the collection and
14+
/// the CTE. Value-generating CTEs (no underlying collection) return an
15+
/// explicit unsupported error.
1016
pub fn plan_recursive_cte(
1117
query: &Query,
1218
catalog: &dyn SqlCatalog,
@@ -16,41 +22,227 @@ pub fn plan_recursive_cte(
1622
detail: "expected WITH clause".into(),
1723
})?;
1824

19-
// Get the CTE definition.
2025
let cte = with.cte_tables.first().ok_or_else(|| SqlError::Parse {
2126
detail: "empty WITH clause".into(),
2227
})?;
2328

29+
let cte_name = normalize_ident(&cte.alias.name);
30+
2431
let cte_query = &cte.query;
2532

2633
// The CTE body should be a UNION of base case and recursive case.
27-
match &*cte_query.body {
34+
let (left, right, set_quantifier) = match &*cte_query.body {
2835
SetExpr::SetOperation {
2936
op: ast::SetOperator::Union,
3037
left,
3138
right,
32-
..
39+
set_quantifier,
40+
} => (left, right, set_quantifier),
41+
_ => {
42+
return Err(SqlError::Unsupported {
43+
detail: "WITH RECURSIVE requires UNION in CTE body".into(),
44+
});
45+
}
46+
};
47+
48+
// UNION ALL → distinct = false; UNION → distinct = true.
49+
let distinct = !matches!(set_quantifier, ast::SetQuantifier::All);
50+
51+
// Plan the base case (should not reference the CTE name).
52+
let base = plan_cte_branch(left, catalog, functions)?;
53+
54+
// Extract the source collection from the base case.
55+
let collection = extract_collection(&base).unwrap_or_default();
56+
57+
// Plan the recursive branch. The recursive branch references the CTE
58+
// name in its FROM clause — either directly (value-gen) or via a JOIN
59+
// with a real table. We attempt to plan it; if it fails because the
60+
// CTE name isn't in the catalog, we try to extract the real table from
61+
// a JOIN and use it with the CTE self-reference as the recursive filter.
62+
let (recursive_filters, join_link) = match plan_cte_branch(right, catalog, functions) {
63+
Ok(plan) => (extract_filters(&plan), None),
64+
Err(_) => {
65+
// The recursive branch references the CTE name. Try to extract
66+
// the real collection, filters, and join link from the AST.
67+
extract_recursive_info(right, &cte_name)?
68+
}
69+
};
70+
71+
if collection.is_empty() {
72+
return Err(SqlError::Unsupported {
73+
detail: "WITH RECURSIVE requires a base case that scans a collection; \
74+
value-generating recursive CTEs are not yet supported"
75+
.into(),
76+
});
77+
}
78+
79+
Ok(SqlPlan::RecursiveScan {
80+
collection,
81+
base_filters: extract_filters(&base),
82+
recursive_filters,
83+
join_link,
84+
max_iterations: 100,
85+
distinct,
86+
limit: 10000,
87+
})
88+
}
89+
90+
/// Extract recursive info from the AST when normal planning fails
91+
/// because the FROM clause references the CTE name.
92+
///
93+
/// Returns `(filters, join_link)` where `join_link` is the
94+
/// `(collection_field, working_table_field)` pair for the working-table
95+
/// hash-join.
96+
///
97+
/// Handles the common tree-traversal pattern:
98+
/// `SELECT t.id FROM tree t INNER JOIN cte_name d ON t.parent_id = d.id`
99+
/// → join_link = `("parent_id", "id")`
100+
/// `(filters, join_link)` where `join_link` is `(collection_field, working_table_field)`.
101+
type RecursiveInfo = (Vec<Filter>, Option<(String, String)>);
102+
103+
fn extract_recursive_info(expr: &SetExpr, cte_name: &str) -> Result<RecursiveInfo> {
104+
let select = match expr {
105+
SetExpr::Select(s) => s,
106+
_ => {
107+
return Err(SqlError::Unsupported {
108+
detail: "recursive CTE branch must be SELECT".into(),
109+
});
110+
}
111+
};
112+
113+
let mut real_table_alias = None;
114+
let mut cte_alias = None;
115+
let mut join_on_expr = None;
116+
117+
for from in &select.from {
118+
let table_name = extract_table_name(&from.relation);
119+
let table_alias = extract_table_alias(&from.relation);
120+
121+
if let Some(name) = &table_name {
122+
if name.eq_ignore_ascii_case(cte_name) {
123+
cte_alias = table_alias.or_else(|| Some(name.clone()));
124+
} else {
125+
real_table_alias = table_alias.or_else(|| Some(name.clone()));
126+
}
127+
}
128+
129+
for join in &from.joins {
130+
let join_table = extract_table_name(&join.relation);
131+
let join_alias = extract_table_alias(&join.relation);
132+
if let Some(jt) = &join_table {
133+
if jt.eq_ignore_ascii_case(cte_name) {
134+
cte_alias = join_alias.or_else(|| Some(jt.clone()));
135+
if let Some(cond) = extract_join_on_condition(&join.join_operator) {
136+
join_on_expr = Some(cond.clone());
137+
}
138+
} else {
139+
real_table_alias = join_alias.or_else(|| Some(jt.clone()));
140+
if join_on_expr.is_none()
141+
&& let Some(cond) = extract_join_on_condition(&join.join_operator)
142+
{
143+
join_on_expr = Some(cond.clone());
144+
}
145+
}
146+
}
147+
}
148+
}
149+
150+
// Extract the join link from the ON condition.
151+
let join_link = if let (Some(real_alias), Some(cte_al), Some(on_expr)) =
152+
(&real_table_alias, &cte_alias, &join_on_expr)
153+
{
154+
extract_equi_link(on_expr, real_alias, cte_al)
155+
} else {
156+
None
157+
};
158+
159+
// Convert the WHERE clause to filters if present.
160+
let mut filters = Vec::new();
161+
if let Some(where_expr) = &select.selection {
162+
let converted = crate::resolver::expr::convert_expr(where_expr)?;
163+
filters.push(Filter {
164+
expr: FilterExpr::Expr(converted),
165+
});
166+
}
167+
168+
Ok((filters, join_link))
169+
}
170+
171+
/// Extract `(collection_field, cte_field)` from an equi-join ON clause.
172+
///
173+
/// Given `t.parent_id = d.id` where `t` is the real table alias and `d`
174+
/// is the CTE alias, returns `("parent_id", "id")`.
175+
fn extract_equi_link(
176+
expr: &ast::Expr,
177+
real_alias: &str,
178+
cte_alias: &str,
179+
) -> Option<(String, String)> {
180+
match expr {
181+
ast::Expr::BinaryOp {
182+
left,
183+
op: ast::BinaryOperator::Eq,
184+
right,
33185
} => {
34-
let base = plan_cte_branch(left, catalog, functions)?;
35-
let recursive = plan_cte_branch(right, catalog, functions)?;
36-
37-
// Extract collection and filters from base/recursive plans.
38-
let collection = extract_collection(&base)
39-
.or_else(|| extract_collection(&recursive))
40-
.unwrap_or_default();
41-
42-
Ok(SqlPlan::RecursiveScan {
43-
collection,
44-
base_filters: extract_filters(&base),
45-
recursive_filters: extract_filters(&recursive),
46-
max_iterations: 100,
47-
distinct: true,
48-
limit: 10000,
49-
})
186+
let left_parts = extract_qualified_column(left)?;
187+
let right_parts = extract_qualified_column(right)?;
188+
189+
// Determine which side is the real table and which is the CTE.
190+
if left_parts.0.eq_ignore_ascii_case(real_alias)
191+
&& right_parts.0.eq_ignore_ascii_case(cte_alias)
192+
{
193+
Some((left_parts.1, right_parts.1))
194+
} else if right_parts.0.eq_ignore_ascii_case(real_alias)
195+
&& left_parts.0.eq_ignore_ascii_case(cte_alias)
196+
{
197+
Some((right_parts.1, left_parts.1))
198+
} else {
199+
None
200+
}
50201
}
51-
_ => Err(SqlError::Unsupported {
52-
detail: "WITH RECURSIVE requires UNION in CTE body".into(),
53-
}),
202+
// For AND-combined conditions, take the first equi-link found.
203+
ast::Expr::BinaryOp {
204+
left,
205+
op: ast::BinaryOperator::And,
206+
right,
207+
} => extract_equi_link(left, real_alias, cte_alias)
208+
.or_else(|| extract_equi_link(right, real_alias, cte_alias)),
209+
_ => None,
210+
}
211+
}
212+
213+
/// Extract `(table_or_alias, column)` from a qualified column reference.
214+
fn extract_qualified_column(expr: &ast::Expr) -> Option<(String, String)> {
215+
match expr {
216+
ast::Expr::CompoundIdentifier(parts) if parts.len() == 2 => {
217+
Some((normalize_ident(&parts[0]), normalize_ident(&parts[1])))
218+
}
219+
_ => None,
220+
}
221+
}
222+
223+
fn extract_table_name(relation: &ast::TableFactor) -> Option<String> {
224+
match relation {
225+
ast::TableFactor::Table { name, .. } => Some(normalize_object_name(name)),
226+
_ => None,
227+
}
228+
}
229+
230+
fn extract_table_alias(relation: &ast::TableFactor) -> Option<String> {
231+
match relation {
232+
ast::TableFactor::Table { alias, .. } => alias.as_ref().map(|a| normalize_ident(&a.name)),
233+
_ => None,
234+
}
235+
}
236+
237+
fn extract_join_on_condition(op: &ast::JoinOperator) -> Option<&ast::Expr> {
238+
use ast::JoinOperator::*;
239+
let constraint = match op {
240+
Inner(c) | LeftOuter(c) | RightOuter(c) | FullOuter(c) => c,
241+
_ => return None,
242+
};
243+
match constraint {
244+
ast::JoinConstraint::On(expr) => Some(expr),
245+
_ => None,
54246
}
55247
}
56248

nodedb-sql/src/types.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,11 @@ pub enum SqlPlan {
197197
collection: String,
198198
base_filters: Vec<Filter>,
199199
recursive_filters: Vec<Filter>,
200+
/// Equi-join link for tree-traversal recursion:
201+
/// `(collection_field, working_table_field)`.
202+
/// e.g. `("parent_id", "id")` means each iteration finds rows
203+
/// where `collection.parent_id` matches a `working_table.id`.
204+
join_link: Option<(String, String)>,
200205
max_iterations: usize,
201206
distinct: bool,
202207
limit: usize,

nodedb/src/bridge/physical_plan/query.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,11 @@ pub enum QueryOp {
187187
base_filters: Vec<u8>,
188188
/// Recursive step filters (applied to working table each iteration).
189189
recursive_filters: Vec<u8>,
190+
/// Equi-join link for tree-traversal recursion:
191+
/// `(collection_field, working_table_field)`.
192+
/// Each iteration finds rows where `collection_field` value
193+
/// matches a `working_table_field` value from the previous iteration.
194+
join_link: Option<(String, String)>,
190195
/// Maximum iterations to prevent infinite loops. Default: 100.
191196
max_iterations: usize,
192197
/// Whether to deduplicate results (UNION vs UNION ALL).

nodedb/src/control/planner/sql_plan_convert/convert.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,15 +134,16 @@ pub(super) fn convert_one(
134134
right,
135135
on,
136136
join_type,
137+
condition,
137138
limit,
138139
projection,
139140
filters,
140-
..
141141
} => super::scan::convert_join(super::scan_params::JoinPlanParams {
142142
left,
143143
right,
144144
on,
145145
join_type,
146+
condition,
146147
limit,
147148
projection,
148149
filters,
@@ -275,18 +276,20 @@ pub(super) fn convert_one(
275276
collection,
276277
base_filters,
277278
recursive_filters,
279+
join_link,
278280
max_iterations,
279281
distinct,
280282
limit,
281-
} => super::scan::convert_recursive_scan(
283+
} => super::scan::convert_recursive_scan(super::scan_params::RecursiveScanParams {
282284
collection,
283285
base_filters,
284286
recursive_filters,
287+
join_link,
285288
max_iterations,
286289
distinct,
287290
limit,
288291
tenant_id,
289-
),
292+
}),
290293

291294
SqlPlan::Cte { definitions, outer } => {
292295
super::set_ops::convert_cte(definitions, outer, tenant_id, ctx)

0 commit comments

Comments
 (0)