Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]

### In Development — v0.5.0 (Rust Core, `rust-core` branch)
- `WITH` (read-path projection) now lowers and executes (#814, slice 1) — a
`MATCH … WITH … RETURN` pipeline projects/renames mid-query and chains, where
before `GraphOp::With` hit "operator not yet lowered". The binder lowers each
`WITH` item's expression in the current scope, mints an `out_var` for its
alias, then **resets the scope** to exactly those aliases (Cypher semantics):
a variable not carried forward is out of scope afterwards and referencing it is
an error, not a silently-wrong result. An optional `WITH … WHERE` filters over
the projected aliases; `ORDER BY`/`SKIP`/`LIMIT`/`DISTINCT` after `WITH` work
via their existing ops. The lowerer projects each item to its alias column and
registers the `out_var` in the var-map so downstream clauses resolve.
Deliberately deferred (rejected with a clear error rather than mishandled):
carrying a **whole node/path** through `WITH` (`WITH n` — a node spans multiple
columns) and **aggregation in `WITH`** (`WITH count(*) …`); write-result
`RETURN` and mid-statement reads-after-writes remain #814's later slices.
- Unlabelled `MATCH (n) RETURN n` materializes a whole node value (#889, slice 1)
— building on #785 (which required the label in the pattern), a bare match with
no label now returns the node with its **real label name and properties**, not
Expand Down
65 changes: 65 additions & 0 deletions crates/gf-api/tests/e2e_baseline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,71 @@ fn count_aggregate() {
assert_eq!(total, 5);
}

#[test]
fn with_projects_and_renames() {
// #814: WITH is a mid-pipeline projection; the alias carries to RETURN.
let gf = forge();
let r = rows(&gf, "MATCH (n:Person) WITH n.name AS nm RETURN nm");
assert_eq!(
r.stats.rows_produced, 5,
"all five names project through WITH"
);
}

#[test]
fn with_where_filters_post_projection() {
// #814: a WITH ... WHERE filters on the projected alias.
let gf = forge();
let r = rows(
&gf,
"MATCH (n:Person) WITH n.name AS nm WHERE nm = 'Alice' RETURN nm",
);
assert_eq!(
r.stats.rows_produced, 1,
"only Alice survives the WITH WHERE"
);
}

#[test]
fn with_resets_scope_dropping_unprojected_vars() {
// #814: WITH resets the scope to exactly its projected aliases — a variable
// not carried forward is out of scope afterwards, so referencing it is an
// error (not a silently-wrong result). `b` is dropped by `WITH a.name AS x`.
let gf = forge();
let res = gf.execute("MATCH (a:Person)-[:KNOWS]->(b:Person) WITH a.name AS x RETURN b.name");
assert!(
res.is_err(),
"`b` is out of scope after WITH a.name AS x; expected an error, got {res:?}"
);
}

#[test]
fn with_node_passthrough_is_deferred() {
// #814 deferral: carrying a whole node through WITH (`WITH n`) is not yet
// supported — a node var spans multiple columns. The binder rejects it with
// a clear error rather than producing a wrong result; projecting the node's
// properties (`WITH n.age AS age`) is the supported path.
let gf = forge();
let res = gf.execute("MATCH (n:Person) WITH n WHERE n.age > 28 RETURN n.name");
assert!(
res.is_err(),
"WITH of a whole node is deferred; expected an error, got {res:?}"
);
}

#[test]
fn with_nested_aggregate_is_rejected() {
// #814 deferral: aggregation in WITH is not yet supported. The guard is
// recursive, so a nested aggregate (`count(n) + 1`) is rejected too, not
// only a bare top-level `WITH count(n) AS c`.
let gf = forge();
let res = gf.execute("MATCH (n:Person) WITH count(n) + 1 AS c RETURN c");
assert!(
res.is_err(),
"a nested aggregate in WITH should be rejected, got {res:?}"
);
}

// ---------------------------------------------------------------------------
// Read — traversal
// ---------------------------------------------------------------------------
Expand Down
109 changes: 105 additions & 4 deletions crates/gf-ir/src/binder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -744,10 +744,88 @@ impl Binder {
}

fn lower_with(&self, w: &WithClause, s: &mut BinderState) {
// WITH is an intermediate pipeline projection: a bare node var must stay
// a node reference so downstream property access (`n.age`) resolves — do
// NOT materialize it as a whole node value (that is terminal-RETURN only).
let items = self.lower_return_items(&w.items, s, false);
// WITH projects/renames the pipeline and then *resets the scope*: only
// the projected aliases are visible to subsequent clauses (#814).
//
// Each item's expression is lowered in the CURRENT scope (it references
// upstream vars), then a fresh `out_var` is minted for its alias and
// becomes the downstream scope. The lowerer maps `out_var` to the
// projected column.
let mut items: Vec<ProjectItem> = Vec::with_capacity(w.items.len());
let mut new_scope: Vec<(String, VarId)> = Vec::new();
for item in &w.items {
// Reject (rather than silently mishandle) the parts not yet
// supported in WITH — these would otherwise produce wrong results.
// The aggregate check is recursive: `WITH count(n) + 1 AS c` must be
// caught too, not only a top-level `WITH count(n) AS c`.
if expr_contains_aggregate(&item.expr) {
s.errors.push(BindError::new(
BindErrorKind::UnsupportedClause,
item.span,
"aggregation in WITH is not yet supported (#814 follow-up)".to_string(),
));
}
if let Expr::Var(VarRef { name, .. }) = &item.expr {
if s.vars
.get(name)
.is_some_and(|v| s.node_vars.contains_key(v))
{
s.errors.push(BindError::new(
BindErrorKind::UnsupportedClause,
item.span,
format!(
"carrying a whole node (`WITH {name}`) through WITH is not yet \
supported — project its properties instead (#814 follow-up)"
),
));
}
if s.path_vars.contains_key(name) {
s.errors.push(BindError::new(
BindErrorKind::UnsupportedClause,
item.span,
format!(
"carrying a path (`WITH {name}`) through WITH is not yet supported \
(#814 follow-up)"
),
));
}
}

let expr_id = self.lower_expr(&item.expr, item.span, s);
// Alias: explicit (`expr AS name`), or the variable's own name for a
// bare-variable pass-through (`WITH x`).
let alias = item.alias.clone().or_else(|| match &item.expr {
Expr::Var(VarRef { name, .. }) => Some(name.clone()),
_ => None,
});
let Some(alias) = alias else {
s.errors.push(BindError::new(
BindErrorKind::UnsupportedClause,
item.span,
"a non-variable WITH item must be aliased (`expr AS name`)".to_string(),
));
continue;
};
let out_var = alloc_anon_var(s);
new_scope.push((alias.clone(), out_var));
items.push(ProjectItem {
expr: expr_id,
alias: Some(alias),
out_var: Some(out_var),
});
}

// Scope reset: drop everything, then introduce only the WITH aliases.
s.vars.clear();
s.node_vars.clear();
s.edge_vars.clear();
s.path_vars.clear();
for (name, v) in new_scope {
s.vars.insert(name, v);
}

// WITH's WHERE / ORDER BY reference the projected aliases — lower them
// against the new scope.
let where_pred = w
.where_clause
.as_ref()
Expand Down Expand Up @@ -860,6 +938,8 @@ impl Binder {
}
_ => None,
}),
// RETURN items are terminal — they introduce no downstream scope.
out_var: None,
})
.collect()
}
Expand Down Expand Up @@ -1464,6 +1544,27 @@ fn agg_func_of(expr: &Expr) -> Option<AggFunc> {
}
}

/// Whether `expr` contains an aggregate function *anywhere* — `count(n) + 1`,
/// `(sum(n.x))`, `[max(n.a)]`, etc., not only a bare top-level call. Used to
/// reject aggregation inside a WITH item, which is not yet supported (#814).
fn expr_contains_aggregate(expr: &Expr) -> bool {
if agg_func_of(expr).is_some() {
return true;
}
match expr {
Expr::BinaryOp(b) => expr_contains_aggregate(&b.left) || expr_contains_aggregate(&b.right),
Expr::UnaryOp(u) => expr_contains_aggregate(&u.expr),
Expr::Parenthesized { inner, .. } => expr_contains_aggregate(inner),
Expr::FunctionCall(c) => c.args.iter().any(expr_contains_aggregate),
Expr::Property(p) => expr_contains_aggregate(&p.object),
Expr::List(l) => l.elements.iter().any(expr_contains_aggregate),
// Other variants (Map, Case, comprehensions, …) are not recursed into;
// an aggregate slipping through still fails at lowering (UnknownFunction),
// so the worst case is a less specific error, never a wrong result.
_ => false,
}
}

fn ensure_var(name: Option<&String>, s: &mut BinderState) -> VarId {
if let Some(n) = name {
ensure_var_name(n, s)
Expand Down
6 changes: 6 additions & 0 deletions crates/gf-ir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,12 @@ pub struct ProjectItem {
pub expr: ExprId,
/// Optional output column name.
pub alias: Option<String>,
/// For a `WITH` item only: the variable this item introduces into the
/// downstream scope (#814). The lowerer maps it to the projected column so a
/// later clause referencing the alias resolves. `None` for terminal `RETURN`
/// items, which introduce no scope.
#[serde(default)]
pub out_var: Option<VarId>,
}

/// An aggregation expression inside an AGGREGATE operator.
Expand Down
4 changes: 4 additions & 0 deletions crates/gf-ir/src/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ mod tests {
items: vec![ProjectItem {
expr: var_r,
alias: Some("n".into()),
out_var: None,
}],
distinct: false,
},
Expand Down Expand Up @@ -485,6 +486,7 @@ mod tests {
items: vec![ProjectItem {
expr: ref_b,
alias: Some("b".into()),
out_var: None,
}],
distinct: false,
},
Expand Down Expand Up @@ -618,6 +620,7 @@ mod tests {
items: vec![ProjectItem {
expr: e0,
alias: None,
out_var: None,
}],
distinct: true,
},
Expand Down Expand Up @@ -682,6 +685,7 @@ mod tests {
items: vec![ProjectItem {
expr: e0,
alias: Some("x".into()),
out_var: None,
}],
where_predicate: Some(e0),
},
Expand Down
6 changes: 4 additions & 2 deletions crates/gf-ir/tests/golden.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,10 @@ fn order_by_limit() {

#[test]
fn with_pipeline() {
// WITH introduces a pipeline; return n directly to keep binder scope simple.
let plan = bind_query("MATCH (n:Person) WITH n WHERE n.age > 30 RETURN n.name");
// WITH projects/renames mid-pipeline and resets the scope to its aliases
// (#814): `nm` is introduced (its `out_var`) and the WHERE + RETURN resolve
// against it. Carrying a whole node (`WITH n`) is deferred — #814 follow-up.
let plan = bind_query("MATCH (n:Person) WITH n.name AS nm WHERE nm = 'Alice' RETURN nm");
golden_settings().bind(|| {
insta::assert_json_snapshot!("with_pipeline", plan);
});
Expand Down
3 changes: 2 additions & 1 deletion crates/gf-ir/tests/ir_goldens/golden__filtered_scan.snap
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ source: crates/gf-ir/tests/golden.rs
"items": [
{
"expr": 5,
"alias": null
"alias": null,
"out_var": null
}
],
"distinct": false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,23 @@ source: crates/gf-ir/tests/golden.rs
"items": [
{
"expr": 1,
"alias": "hops"
"alias": "hops",
"out_var": null
},
{
"expr": 5,
"alias": "ns"
"alias": "ns",
"out_var": null
},
{
"expr": 8,
"alias": "rels"
"alias": "rels",
"out_var": null
},
{
"expr": 16,
"alias": "p"
"alias": "p",
"out_var": null
}
],
"distinct": false
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
---
source: crates/gf-ir/tests/golden.rs
assertion_line: 133
---
{
"ir_version": {
Expand Down Expand Up @@ -41,15 +40,18 @@ assertion_line: 133
"items": [
{
"expr": 1,
"alias": "hops"
"alias": "hops",
"out_var": null
},
{
"expr": 2,
"alias": "rels"
"alias": "rels",
"out_var": null
},
{
"expr": 5,
"alias": "ns"
"alias": "ns",
"out_var": null
}
],
"distinct": false
Expand Down
6 changes: 4 additions & 2 deletions crates/gf-ir/tests/ir_goldens/golden__one_hop_expand.snap
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,13 @@ source: crates/gf-ir/tests/golden.rs
"items": [
{
"expr": 1,
"alias": null
"alias": null,
"out_var": null
},
{
"expr": 3,
"alias": null
"alias": null,
"out_var": null
}
],
"distinct": false
Expand Down
6 changes: 4 additions & 2 deletions crates/gf-ir/tests/ir_goldens/golden__optional_match.snap
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,13 @@ source: crates/gf-ir/tests/golden.rs
"items": [
{
"expr": 1,
"alias": null
"alias": null,
"out_var": null
},
{
"expr": 3,
"alias": null
"alias": null,
"out_var": null
}
],
"distinct": false
Expand Down
3 changes: 2 additions & 1 deletion crates/gf-ir/tests/ir_goldens/golden__order_by_limit.snap
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ source: crates/gf-ir/tests/golden.rs
"items": [
{
"expr": 1,
"alias": null
"alias": null,
"out_var": null
}
],
"distinct": false
Expand Down
3 changes: 2 additions & 1 deletion crates/gf-ir/tests/ir_goldens/golden__parameter.snap
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ source: crates/gf-ir/tests/golden.rs
"items": [
{
"expr": 5,
"alias": null
"alias": null,
"out_var": null
}
],
"distinct": false
Expand Down
Loading
Loading