diff --git a/crates/ruvector-graph-node/src/cypher_exec.rs b/crates/ruvector-graph-node/src/cypher_exec.rs new file mode 100644 index 0000000000..45c587476b --- /dev/null +++ b/crates/ruvector-graph-node/src/cypher_exec.rs @@ -0,0 +1,759 @@ +//! Cypher `MATCH` execution over an in-memory [`GraphDB`]. +//! +//! Before this module, `GraphDatabase::query()` handled exactly one shape — +//! `MATCH (n:Label)` — by consulting the label index. Every other shape fell +//! through an empty `if` branch and returned zero rows: the label-less +//! `MATCH (n)`, any `WHERE` filter (the parser produced a `where_clause` the +//! executor dropped), and every relationship pattern (`result_edges` was +//! declared and never written to). See ruvnet/ruvector#879. +//! +//! Scope. This is a single-pattern matcher, not a planner: each pattern in the +//! `MATCH` is resolved independently against the graph and its rows unioned. +//! Cross-pattern joins, variable-length paths and aggregations remain +//! unimplemented — but they now report themselves as unsupported through +//! [`ExecOutcome::unsupported`] instead of silently returning an empty set, +//! which is the failure mode that made #879 hard to spot from the outside. +//! +//! Note on operator coverage: the evaluator implements `CONTAINS`, `STARTS +//! WITH`, `ENDS WITH`, `IN`, `IS NULL` and `=~` because [`BinaryOperator`] +//! declares them, but the lexer does not yet tokenise any of those, so no +//! parse can reach those arms today. They are kept so that extending the lexer +//! is a one-sided change; `=~` is the exception and is reported unsupported +//! because no regex engine is linked into this crate. + +use std::collections::{HashMap, HashSet}; + +use ruvector_graph::cypher::ast::{ + BinaryOperator, Expression, MatchClause, NodePattern, Pattern, PropertyMap, UnaryOperator, +}; +use ruvector_graph::edge::Edge; +use ruvector_graph::node::Node; +use ruvector_graph::types::PropertyValue; +use ruvector_graph::GraphDB; + +/// A value in the expression evaluator's own domain. +/// +/// Cypher literals and stored [`PropertyValue`]s are both lowered into this so +/// comparison has exactly one set of rules to follow. +#[derive(Debug, Clone, PartialEq)] +pub enum EvalValue { + Null, + Bool(bool), + Int(i64), + Float(f64), + Str(String), + List(Vec), +} + +impl EvalValue { + fn truthy(&self) -> bool { + matches!(self, EvalValue::Bool(true)) + } + + fn as_f64(&self) -> Option { + match self { + EvalValue::Int(i) => Some(*i as f64), + EvalValue::Float(f) => Some(*f), + _ => None, + } + } + + fn as_str(&self) -> Option<&str> { + match self { + EvalValue::Str(s) => Some(s.as_str()), + _ => None, + } + } +} + +impl From<&PropertyValue> for EvalValue { + fn from(value: &PropertyValue) -> Self { + match value { + PropertyValue::Null => EvalValue::Null, + PropertyValue::Boolean(b) => EvalValue::Bool(*b), + PropertyValue::Integer(i) => EvalValue::Int(*i), + PropertyValue::Float(f) => EvalValue::Float(*f), + PropertyValue::String(s) => EvalValue::Str(s.clone()), + PropertyValue::Array(items) | PropertyValue::List(items) => { + EvalValue::List(items.iter().map(EvalValue::from).collect()) + } + PropertyValue::FloatArray(items) => { + EvalValue::List(items.iter().map(|f| EvalValue::Float(*f as f64)).collect()) + } + // A map has no ordering or equality semantics we can honour here; + // treating it as Null makes every comparison against it false + // rather than accidentally true. + PropertyValue::Map(_) => EvalValue::Null, + } + } +} + +/// What a pattern variable is bound to for the duration of one candidate row. +#[derive(Debug, Clone)] +pub enum Bound { + Node(Node), + Edge(Edge), +} + +type Bindings = HashMap; + +/// Resolve `.` against the bound entity. +/// +/// `id` is resolved from the entity's identity field when no stored property +/// shadows it. That is the whole point of the exercise: `MATCH (n) WHERE +/// n.id = '...'` is the point-lookup shape #879 calls out, and in this data +/// model the id lives beside the property bag rather than inside it. +fn lookup_property(bound: &Bound, property: &str) -> EvalValue { + match bound { + Bound::Node(node) => { + if let Some(value) = node.properties.get(property) { + return EvalValue::from(value); + } + match property { + "id" => EvalValue::Str(node.id.clone()), + "labels" => EvalValue::List( + node.labels + .iter() + .map(|l| EvalValue::Str(l.name.clone())) + .collect(), + ), + _ => EvalValue::Null, + } + } + Bound::Edge(edge) => { + if let Some(value) = edge.properties.get(property) { + return EvalValue::from(value); + } + match property { + "id" => EvalValue::Str(edge.id.clone()), + "from" | "source" => EvalValue::Str(edge.from.clone()), + "to" | "target" => EvalValue::Str(edge.to.clone()), + "type" => EvalValue::Str(edge.edge_type.clone()), + _ => EvalValue::Null, + } + } + } +} + +/// Evaluate an expression to a value. Unresolvable references yield `Null`, +/// which makes every downstream comparison false — Cypher's own rule. +fn eval(expr: &Expression, bindings: &Bindings) -> EvalValue { + match expr { + Expression::Integer(i) => EvalValue::Int(*i), + Expression::Float(f) => EvalValue::Float(*f), + Expression::String(s) => EvalValue::Str(s.clone()), + Expression::Boolean(b) => EvalValue::Bool(*b), + Expression::Null => EvalValue::Null, + Expression::List(items) => { + EvalValue::List(items.iter().map(|i| eval(i, bindings)).collect()) + } + Expression::Variable(name) => match bindings.get(name) { + // A bare variable in a predicate position is only meaningful as an + // existence check; comparing it directly is not supported. + Some(_) => EvalValue::Bool(true), + None => EvalValue::Null, + }, + Expression::Property { object, property } => { + let Expression::Variable(name) = object.as_ref() else { + return EvalValue::Null; + }; + match bindings.get(name) { + Some(bound) => lookup_property(bound, property), + None => EvalValue::Null, + } + } + Expression::UnaryOp { op, operand } => { + let value = eval(operand, bindings); + match op { + UnaryOperator::Not => EvalValue::Bool(!value.truthy()), + UnaryOperator::Minus => match value { + EvalValue::Int(i) => EvalValue::Int(-i), + EvalValue::Float(f) => EvalValue::Float(-f), + _ => EvalValue::Null, + }, + UnaryOperator::Plus => value, + UnaryOperator::IsNull => EvalValue::Bool(matches!(value, EvalValue::Null)), + UnaryOperator::IsNotNull => EvalValue::Bool(!matches!(value, EvalValue::Null)), + } + } + Expression::BinaryOp { left, op, right } => eval_binary(left, *op, right, bindings), + // Functions, aggregations, CASE and pattern predicates are out of + // scope for this executor. + _ => EvalValue::Null, + } +} + +fn eval_binary( + left: &Expression, + op: BinaryOperator, + right: &Expression, + bindings: &Bindings, +) -> EvalValue { + // Short-circuit the logical operators before evaluating both sides. + match op { + BinaryOperator::And => { + return EvalValue::Bool(eval(left, bindings).truthy() && eval(right, bindings).truthy()) + } + BinaryOperator::Or => { + return EvalValue::Bool(eval(left, bindings).truthy() || eval(right, bindings).truthy()) + } + BinaryOperator::Xor => { + return EvalValue::Bool(eval(left, bindings).truthy() != eval(right, bindings).truthy()) + } + _ => {} + } + + let l = eval(left, bindings); + let r = eval(right, bindings); + + match op { + BinaryOperator::Equal => EvalValue::Bool(values_equal(&l, &r)), + BinaryOperator::NotEqual => EvalValue::Bool(!values_equal(&l, &r)), + BinaryOperator::LessThan + | BinaryOperator::LessThanOrEqual + | BinaryOperator::GreaterThan + | BinaryOperator::GreaterThanOrEqual => EvalValue::Bool(compare(&l, &r, op)), + BinaryOperator::Contains => match (l.as_str(), r.as_str()) { + (Some(hay), Some(needle)) => EvalValue::Bool(hay.contains(needle)), + _ => EvalValue::Bool(false), + }, + BinaryOperator::StartsWith => match (l.as_str(), r.as_str()) { + (Some(hay), Some(needle)) => EvalValue::Bool(hay.starts_with(needle)), + _ => EvalValue::Bool(false), + }, + BinaryOperator::EndsWith => match (l.as_str(), r.as_str()) { + (Some(hay), Some(needle)) => EvalValue::Bool(hay.ends_with(needle)), + _ => EvalValue::Bool(false), + }, + BinaryOperator::In => match r { + EvalValue::List(items) => { + EvalValue::Bool(items.iter().any(|item| values_equal(&l, item))) + } + _ => EvalValue::Bool(false), + }, + // `IS NULL` / `IS NOT NULL`: the parser puts NULL on the right. + BinaryOperator::Is => EvalValue::Bool(matches!(l, EvalValue::Null)), + BinaryOperator::IsNot => EvalValue::Bool(!matches!(l, EvalValue::Null)), + BinaryOperator::Add => arith(&l, &r, op), + BinaryOperator::Subtract => arith(&l, &r, op), + BinaryOperator::Multiply => arith(&l, &r, op), + BinaryOperator::Divide => arith(&l, &r, op), + BinaryOperator::Modulo => arith(&l, &r, op), + BinaryOperator::Power => arith(&l, &r, op), + // No regex engine is linked into this crate; `=~` is reported as + // unsupported by the caller rather than quietly matching nothing. + BinaryOperator::Matches => EvalValue::Null, + BinaryOperator::And | BinaryOperator::Or | BinaryOperator::Xor => unreachable!(), + } +} + +fn values_equal(l: &EvalValue, r: &EvalValue) -> bool { + match (l, r) { + (EvalValue::Null, _) | (_, EvalValue::Null) => false, + (EvalValue::Str(a), EvalValue::Str(b)) => a == b, + (EvalValue::Bool(a), EvalValue::Bool(b)) => a == b, + (EvalValue::List(a), EvalValue::List(b)) => { + a.len() == b.len() && a.iter().zip(b).all(|(x, y)| values_equal(x, y)) + } + // Numbers compare across Int/Float rather than by representation, so + // `WHERE n.age = 30` matches a stored 30.0. + _ => match (l.as_f64(), r.as_f64()) { + (Some(a), Some(b)) => a == b, + _ => false, + }, + } +} + +fn compare(l: &EvalValue, r: &EvalValue, op: BinaryOperator) -> bool { + let ordering = match (l, r) { + (EvalValue::Str(a), EvalValue::Str(b)) => a.as_str().partial_cmp(b.as_str()), + _ => match (l.as_f64(), r.as_f64()) { + // `partial_cmp` on a NaN operand yields None, which falls through + // to `false` below — never a panic. (ADR-340 invariant 1.) + (Some(a), Some(b)) => a.partial_cmp(&b), + _ => None, + }, + }; + let Some(ordering) = ordering else { + return false; + }; + match op { + BinaryOperator::LessThan => ordering.is_lt(), + BinaryOperator::LessThanOrEqual => ordering.is_le(), + BinaryOperator::GreaterThan => ordering.is_gt(), + BinaryOperator::GreaterThanOrEqual => ordering.is_ge(), + _ => false, + } +} + +fn arith(l: &EvalValue, r: &EvalValue, op: BinaryOperator) -> EvalValue { + // String concatenation is the one non-numeric `+`. + if let (BinaryOperator::Add, Some(a), Some(b)) = (op, l.as_str(), r.as_str()) { + return EvalValue::Str(format!("{a}{b}")); + } + let (Some(a), Some(b)) = (l.as_f64(), r.as_f64()) else { + return EvalValue::Null; + }; + let both_int = matches!(l, EvalValue::Int(_)) && matches!(r, EvalValue::Int(_)); + let result = match op { + BinaryOperator::Add => a + b, + BinaryOperator::Subtract => a - b, + BinaryOperator::Multiply => a * b, + BinaryOperator::Divide => { + if b == 0.0 { + return EvalValue::Null; + } + a / b + } + BinaryOperator::Modulo => { + if b == 0.0 { + return EvalValue::Null; + } + a % b + } + BinaryOperator::Power => a.powf(b), + _ => return EvalValue::Null, + }; + if both_int && op != BinaryOperator::Divide && result.fract() == 0.0 { + EvalValue::Int(result as i64) + } else { + EvalValue::Float(result) + } +} + +/// Does a node satisfy the inline property map of its pattern — +/// the `{id: 'n1'}` in `MATCH (n {id: 'n1'})`? +fn matches_inline_props(node: &Node, props: &Option, bindings: &Bindings) -> bool { + let Some(props) = props else { + return true; + }; + props.iter().all(|(key, expected)| { + let actual = lookup_property(&Bound::Node(node.clone()), key); + values_equal(&actual, &eval(expected, bindings)) + }) +} + +fn edge_matches_inline_props( + edge: &Edge, + props: &Option, + bindings: &Bindings, +) -> bool { + let Some(props) = props else { + return true; + }; + props.iter().all(|(key, expected)| { + let actual = lookup_property(&Bound::Edge(edge.clone()), key); + values_equal(&actual, &eval(expected, bindings)) + }) +} + +fn node_matches_labels(node: &Node, labels: &[String]) -> bool { + labels.iter().all(|want| node.has_label(want)) +} + +/// Candidate nodes for a node pattern, narrowed by the label index when it can be. +fn candidates_for(gdb: &GraphDB, pattern: &NodePattern) -> Vec { + match pattern.labels.first() { + // Multiple labels are conjunctive: seed from the most selective index + // we have (the first label) and filter the rest. + Some(label) => gdb + .get_nodes_by_label(label) + .into_iter() + .filter(|n| node_matches_labels(n, &pattern.labels)) + .collect(), + // No label to index on — this is the `MATCH (n)` full scan that #879 + // reported as returning nothing. + None => gdb.all_nodes(), + } +} + +/// The rows a `MATCH` produced, plus anything in it this executor could not honour. +#[derive(Debug, Default)] +pub struct ExecOutcome { + pub nodes: Vec, + pub edges: Vec, + /// Human-readable descriptions of clauses that were parsed but not executed. + /// The caller surfaces these as an error rather than returning a partial + /// result that looks complete. + pub unsupported: Vec, +} + +impl ExecOutcome { + fn push_node(&mut self, seen: &mut HashSet, node: Node) { + if seen.insert(node.id.clone()) { + self.nodes.push(node); + } + } +} + +/// Execute one `MATCH` clause against the graph. +pub fn execute_match(gdb: &GraphDB, clause: &MatchClause) -> ExecOutcome { + let mut outcome = ExecOutcome::default(); + let mut seen_nodes: HashSet = HashSet::new(); + let mut seen_edges: HashSet = HashSet::new(); + let predicate = clause.where_clause.as_ref().map(|w| &w.condition); + + if let Some(condition) = predicate { + if uses_regex(condition) { + outcome + .unsupported + .push("WHERE ... =~ (regex matching) is not supported".to_string()); + } + } + + for pattern in &clause.patterns { + exec_pattern( + gdb, + pattern, + predicate, + &mut outcome, + &mut seen_nodes, + &mut seen_edges, + ); + } + outcome +} + +fn uses_regex(expr: &Expression) -> bool { + match expr { + Expression::BinaryOp { left, op, right } => { + *op == BinaryOperator::Matches || uses_regex(left) || uses_regex(right) + } + Expression::UnaryOp { operand, .. } => uses_regex(operand), + _ => false, + } +} + +fn exec_pattern( + gdb: &GraphDB, + pattern: &Pattern, + predicate: Option<&Expression>, + outcome: &mut ExecOutcome, + seen_nodes: &mut HashSet, + seen_edges: &mut HashSet, +) { + match pattern { + Pattern::Node(np) => { + for node in candidates_for(gdb, np) { + let mut bindings = Bindings::new(); + if let Some(var) = &np.variable { + bindings.insert(var.clone(), Bound::Node(node.clone())); + } + if !matches_inline_props(&node, &np.properties, &bindings) { + continue; + } + if let Some(condition) = predicate { + if !eval(condition, &bindings).truthy() { + continue; + } + } + outcome.push_node(seen_nodes, node); + } + } + Pattern::Relationship(rp) => { + // A typed relationship uses the edge-type index; an untyped one + // has to scan, same reasoning as the label-less node pattern. + let edges = match &rp.rel_type { + Some(t) => gdb.get_edges_by_type(t), + None => gdb.all_edges(), + }; + // Only a direct `(a)-[r]->(b)` target is resolved; a chained + // pattern nests another Pattern here and needs a real join. + let target: Option<&NodePattern> = match rp.to.as_ref() { + Pattern::Node(np) => Some(np), + _ => { + outcome.unsupported.push( + "chained relationship patterns ((a)-[]->(b)<-[]-(c)) are not supported" + .to_string(), + ); + None + } + }; + if rp.range.is_some() { + outcome + .unsupported + .push("variable-length relationships ([*1..n]) are not supported".to_string()); + } + + for edge in edges { + let Some(from_node) = gdb.get_node(&edge.from) else { + continue; + }; + let Some(to_node) = gdb.get_node(&edge.to) else { + continue; + }; + // Undirected patterns accept the edge in either orientation; + // Incoming flips which endpoint the `from` pattern must match. + let orientations: &[(&Node, &Node)] = match rp.direction { + ruvector_graph::cypher::ast::Direction::Outgoing => &[(&from_node, &to_node)], + ruvector_graph::cypher::ast::Direction::Incoming => &[(&to_node, &from_node)], + ruvector_graph::cypher::ast::Direction::Undirected => { + &[(&from_node, &to_node), (&to_node, &from_node)] + } + }; + + for (src, dst) in orientations { + if !node_matches_labels(src, &rp.from.labels) { + continue; + } + if let Some(tp) = target { + if !node_matches_labels(dst, &tp.labels) { + continue; + } + } + + let mut bindings = Bindings::new(); + if let Some(var) = &rp.variable { + bindings.insert(var.clone(), Bound::Edge(edge.clone())); + } + if let Some(var) = &rp.from.variable { + bindings.insert(var.clone(), Bound::Node((*src).clone())); + } + if let Some(tp) = target { + if let Some(var) = &tp.variable { + bindings.insert(var.clone(), Bound::Node((*dst).clone())); + } + } + + if !matches_inline_props(src, &rp.from.properties, &bindings) { + continue; + } + if let Some(tp) = target { + if !matches_inline_props(dst, &tp.properties, &bindings) { + continue; + } + } + if !edge_matches_inline_props(&edge, &rp.properties, &bindings) { + continue; + } + if let Some(condition) = predicate { + if !eval(condition, &bindings).truthy() { + continue; + } + } + + if seen_edges.insert(edge.id.clone()) { + outcome.edges.push(edge.clone()); + } + outcome.push_node(seen_nodes, (*src).clone()); + outcome.push_node(seen_nodes, (*dst).clone()); + break; + } + } + } + Pattern::Path(path) => exec_pattern( + gdb, + &path.pattern, + predicate, + outcome, + seen_nodes, + seen_edges, + ), + Pattern::Hyperedge(_) => outcome.unsupported.push( + "hyperedge patterns in MATCH are not supported; use searchHyperedges()".to_string(), + ), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use ruvector_graph::cypher::parse_cypher; + use ruvector_graph::cypher::Statement; + use ruvector_graph::edge::EdgeBuilder; + use ruvector_graph::node::NodeBuilder; + + /// Two people and one `knows` edge between them. + fn fixture() -> GraphDB { + let gdb = GraphDB::new(); + for (id, name, age) in [("n1", "alice", 30i64), ("n2", "bob", 41)] { + gdb.create_node( + NodeBuilder::new() + .id(id) + .label("Person") + .property("name", PropertyValue::String(name.to_string())) + .property("age", PropertyValue::Integer(age)) + .build(), + ) + .expect("create node"); + } + gdb.create_node(NodeBuilder::new().id("c1").label("Company").build()) + .expect("create company"); + gdb.create_edge( + EdgeBuilder::new("n1".to_string(), "n2".to_string(), "knows") + .id("e1") + .property("since", PropertyValue::Integer(2020)) + .build(), + ) + .expect("create edge"); + gdb + } + + fn run(gdb: &GraphDB, cypher: &str) -> ExecOutcome { + let parsed = parse_cypher(cypher).expect("parse"); + let clause = parsed + .statements + .iter() + .find_map(|s| match s { + Statement::Match(m) => Some(m), + _ => None, + }) + .expect("a MATCH statement"); + execute_match(gdb, clause) + } + + /// The headline defect in #879: the label-less pattern returned nothing. + #[test] + fn label_less_match_returns_every_node() { + let gdb = fixture(); + let out = run(&gdb, "MATCH (n) RETURN n"); + assert_eq!(out.nodes.len(), 3, "MATCH (n) must see all 3 nodes"); + assert!(out.unsupported.is_empty()); + } + + /// The point-lookup shape #879 calls "the standard query shape". + #[test] + fn where_filters_on_identity() { + let gdb = fixture(); + let out = run(&gdb, "MATCH (n) WHERE n.id = 'n1' RETURN n"); + assert_eq!(out.nodes.len(), 1); + assert_eq!(out.nodes[0].id, "n1"); + } + + #[test] + fn where_filters_on_stored_property() { + let gdb = fixture(); + let out = run(&gdb, "MATCH (n) WHERE n.name = 'bob' RETURN n"); + assert_eq!(out.nodes.len(), 1); + assert_eq!(out.nodes[0].id, "n2"); + } + + #[test] + fn where_numeric_comparison_and_conjunction() { + let gdb = fixture(); + let out = run(&gdb, "MATCH (n:Person) WHERE n.age > 35 RETURN n"); + assert_eq!(out.nodes.len(), 1); + assert_eq!(out.nodes[0].id, "n2"); + + let out = run( + &gdb, + "MATCH (n:Person) WHERE n.age > 20 AND n.age < 35 RETURN n", + ); + assert_eq!(out.nodes.len(), 1); + assert_eq!(out.nodes[0].id, "n1"); + } + + /// A label filter still works — the one shape that worked before must not regress. + #[test] + fn label_scan_still_works() { + let gdb = fixture(); + let out = run(&gdb, "MATCH (n:Person) RETURN n"); + assert_eq!(out.nodes.len(), 2); + let out = run(&gdb, "MATCH (n:Company) RETURN n"); + assert_eq!(out.nodes.len(), 1); + assert_eq!(out.nodes[0].id, "c1"); + } + + /// `result_edges` was declared and never written to, for any query shape. + #[test] + fn relationship_pattern_returns_edges() { + let gdb = fixture(); + let out = run(&gdb, "MATCH (a)-[r:knows]->(b) RETURN a, r, b"); + assert_eq!(out.edges.len(), 1, "the knows edge must be returned"); + assert_eq!(out.edges[0].id, "e1"); + assert_eq!(out.edges[0].from, "n1"); + assert_eq!(out.edges[0].to, "n2"); + // Both endpoints come back with it. + let mut ids: Vec<_> = out.nodes.iter().map(|n| n.id.as_str()).collect(); + ids.sort(); + assert_eq!(ids, vec!["n1", "n2"]); + } + + #[test] + fn untyped_relationship_pattern_scans_all_edges() { + let gdb = fixture(); + let out = run(&gdb, "MATCH (a)-[r]->(b) RETURN r"); + assert_eq!(out.edges.len(), 1); + } + + #[test] + fn relationship_direction_is_honoured() { + let gdb = fixture(); + // n2 has no outgoing edge, so anchoring the source on a Company label + // must yield nothing rather than matching either endpoint. + let out = run(&gdb, "MATCH (a:Company)-[r:knows]->(b) RETURN r"); + assert!(out.edges.is_empty()); + } + + #[test] + fn inline_property_pattern_filters() { + let gdb = fixture(); + let out = run(&gdb, "MATCH (n {name: 'alice'}) RETURN n"); + assert_eq!(out.nodes.len(), 1); + assert_eq!(out.nodes[0].id, "n1"); + } + + #[test] + fn inequality_operator() { + let gdb = fixture(); + let out = run(&gdb, "MATCH (n:Person) WHERE n.age <> 30 RETURN n"); + assert_eq!(out.nodes.len(), 1); + assert_eq!(out.nodes[0].id, "n2"); + } + + /// `NOT` is deliberately not covered here: the parser binds it tighter than + /// comparison, so `NOT n.age = 30` arrives as `(NOT n.age) = 30`. That is a + /// precedence defect in `ruvector-graph`'s parser, tracked separately — this + /// executor evaluates faithfully whatever tree it is handed. + + /// A missing property compares false rather than panicking or matching. + #[test] + fn absent_property_never_matches() { + let gdb = fixture(); + let out = run(&gdb, "MATCH (n) WHERE n.nonexistent = 'x' RETURN n"); + assert!(out.nodes.is_empty()); + } + + /// NaN must not panic the comparison path (ADR-340 invariant 1). + #[test] + fn nan_comparison_is_false_not_a_panic() { + let gdb = GraphDB::new(); + gdb.create_node( + NodeBuilder::new() + .id("nan") + .property("score", PropertyValue::Float(f64::NAN)) + .build(), + ) + .expect("create node"); + let out = run(&gdb, "MATCH (n) WHERE n.score > 0 RETURN n"); + assert!(out.nodes.is_empty()); + let out = run(&gdb, "MATCH (n) WHERE n.score = 0 RETURN n"); + assert!(out.nodes.is_empty()); + } + + /// Unsupported constructs must announce themselves. Returning an empty set + /// is what let #879 hide for three releases. + #[test] + fn variable_length_relationship_is_reported_unsupported() { + let gdb = fixture(); + let out = run(&gdb, "MATCH (a)-[r*1..2]->(b) RETURN r"); + assert!( + out.unsupported + .iter() + .any(|u| u.contains("variable-length")), + "expected a variable-length notice, got {:?}", + out.unsupported + ); + } + + #[test] + fn chained_relationship_is_reported_unsupported() { + let gdb = fixture(); + let out = run(&gdb, "MATCH (a)-[r:knows]->(b)<-[s:knows]-(c) RETURN r"); + assert!( + out.unsupported.iter().any(|u| u.contains("chained")), + "expected a chained-pattern notice, got {:?}", + out.unsupported + ); + } +} diff --git a/crates/ruvector-graph-node/src/lib.rs b/crates/ruvector-graph-node/src/lib.rs index 8dad310eaf..5460a661df 100644 --- a/crates/ruvector-graph-node/src/lib.rs +++ b/crates/ruvector-graph-node/src/lib.rs @@ -21,8 +21,9 @@ use ruvector_graph::storage::GraphStorage; use ruvector_graph::types::PropertyValue; use ruvector_graph::GraphDB; use std::collections::HashMap; -use std::sync::{Arc, RwLock}; +use std::sync::{Arc, Mutex, RwLock}; +mod cypher_exec; mod streaming; mod transactions; mod types; @@ -60,6 +61,9 @@ pub struct GraphDatabase { transaction_manager: Arc>, /// Property graph database with Cypher support graph_db: Arc>, + /// Whether the durable store has been replayed into the in-memory indexes. + /// Set on first use rather than in the constructor — see [`hydrate_once`]. + hydrated: Arc>, /// Persistent storage backend (optional) storage: Option>>, /// Path to storage file (if persisted) @@ -117,6 +121,225 @@ fn register_node( Ok(()) } +/// Replay persisted records into the in-memory property and hypergraph indexes, +/// exactly once per database handle. +/// +/// This used to run synchronously inside the NAPI constructor, which blocked the +/// Node event loop for the whole replay — about 15 seconds on the 154k-node graph +/// in ruvnet/ruvector#826. Hydration is now deferred to the first operation that +/// actually needs the in-memory indexes; every async entry point calls this from +/// inside `spawn_blocking`, so the cost lands on the thread pool instead of the +/// main thread. Construction is O(1) again. +/// +/// The `hydrated` flag is held across the replay, so concurrent first-callers +/// serialise here rather than each replaying the store into the same indexes. +fn hydrate_once( + hydrated: &Arc>, + storage: Option<&Arc>>, + hypergraph: &Arc>, + graph_db: &Arc>, +) -> Result<()> { + let mut done = hydrated.lock().expect("hydration Mutex poisoned"); + if *done { + return Ok(()); + } + hydrate_from_storage(storage, hypergraph, graph_db)?; + *done = true; + Ok(()) +} + +fn hydrate_from_storage( + storage: Option<&Arc>>, + hypergraph: &Arc>, + graph_db: &Arc>, +) -> Result<()> { + let Some(storage_arc) = storage else { + return Ok(()); + }; + let storage = storage_arc.read().expect("Storage RwLock poisoned"); + let mut hg = hypergraph.write().expect("RwLock poisoned"); + let gdb = graph_db.write().expect("RwLock poisoned"); + + for id in storage + .all_node_ids() + .map_err(|e| Error::from_reason(format!("hydrate nodes: {e}")))? + { + if let Some(node) = storage + .get_node(&id) + .map_err(|e| Error::from_reason(format!("hydrate node {id}: {e}")))? + { + let embedding = prop_to_f32_vec(node.properties.get("__embedding")); + hg.add_entity(node.id.clone(), embedding); + gdb.create_node(node) + .map_err(|e| Error::from_reason(format!("hydrate node insert: {e}")))?; + } + } + + for id in storage + .all_edge_ids() + .map_err(|e| Error::from_reason(format!("hydrate edges: {e}")))? + { + if let Some(edge) = storage + .get_edge(&id) + .map_err(|e| Error::from_reason(format!("hydrate edge {id}: {e}")))? + { + let confidence = prop_to_f32_vec(edge.properties.get("__confidence")) + .first() + .copied() + .unwrap_or(1.0); + let embedding = prop_to_f32_vec(edge.properties.get("__embedding")); + let mut core_edge = CoreHyperedge::new( + vec![edge.from.clone(), edge.to.clone()], + edge.edge_type.clone(), + embedding, + confidence, + ); + core_edge.id = edge.id.clone(); + // A non-cascaded deletion deliberately leaves the durable edge, + // but neither in-memory index accepts an edge with a missing node. + if hg.add_hyperedge(core_edge).is_err() { + continue; + } + gdb.create_edge(edge) + .map_err(|e| Error::from_reason(format!("hydrate edge insert: {e}")))?; + } + } + + for id in storage + .all_hyperedge_ids() + .map_err(|e| Error::from_reason(format!("hydrate hyperedges: {e}")))? + { + if let Some(hyperedge) = storage + .get_hyperedge(&id) + .map_err(|e| Error::from_reason(format!("hydrate hyperedge {id}: {e}")))? + { + let embedding = prop_to_f32_vec(hyperedge.properties.get("__embedding")); + let mut core_edge = CoreHyperedge::new( + hyperedge.nodes, + hyperedge + .description + .unwrap_or_else(|| hyperedge.edge_type.clone()), + embedding, + hyperedge.confidence, + ); + core_edge.id = hyperedge.id; + // A non-cascaded deletion can deliberately leave this dangling. + let _ = hg.add_hyperedge(core_edge); + } + } + + Ok(()) +} + +/// Parse and execute a Cypher statement against the in-memory graph. +/// +/// Shared by `query()` and `querySync()` so the two cannot drift — before +/// ruvnet/ruvector#879 they had entirely separate bodies, and `querySync()` +/// never even parsed its argument. +fn run_query( + cypher: &str, + graph_db: &Arc>, + hypergraph: &Arc>, +) -> Result { + let parsed = parse_cypher(cypher) + .map_err(|e| Error::from_reason(format!("Cypher parse error: {}", e)))?; + + let gdb = graph_db.read().expect("RwLock poisoned"); + let hg = hypergraph.read().expect("RwLock poisoned"); + + let mut result_nodes: Vec = Vec::new(); + let mut result_edges: Vec = Vec::new(); + let mut unsupported: Vec = Vec::new(); + + for statement in &parsed.statements { + match statement { + Statement::Match(match_clause) => { + let outcome = cypher_exec::execute_match(&gdb, match_clause); + unsupported.extend(outcome.unsupported); + for node in outcome.nodes { + result_nodes.push(JsNodeResult { + id: node.id.clone(), + labels: node.labels.iter().map(|l| l.name.clone()).collect(), + properties: node + .properties + .iter() + .filter(|(k, _)| !is_internal_property(k)) + .map(|(k, v)| (k.clone(), property_to_string(v))) + .collect(), + }); + } + for edge in outcome.edges { + result_edges.push(JsEdgeResult { + id: edge.id.clone(), + from: edge.from.clone(), + to: edge.to.clone(), + edge_type: edge.edge_type.clone(), + properties: edge + .properties + .iter() + .filter(|(k, _)| !is_internal_property(k)) + .map(|(k, v)| (k.clone(), property_to_string(v))) + .collect(), + }); + } + } + // Writes through `query()` were previously accepted and silently + // discarded. Refuse them instead — the typed builders are the + // supported path and a dropped write is worse than a loud error. + Statement::Create(_) => unsupported.push( + "CREATE via query() is not supported; use createNode()/createEdge()".to_string(), + ), + Statement::Return(_) => {} + _ => {} + } + } + + if !unsupported.is_empty() { + unsupported.sort(); + unsupported.dedup(); + return Err(Error::from_reason(format!( + "Unsupported Cypher: {}", + unsupported.join("; ") + ))); + } + + let stats = hg.stats(); + Ok(JsQueryResult { + nodes: result_nodes, + edges: result_edges, + stats: Some(JsGraphStats { + total_nodes: stats.total_entities as u32, + total_edges: stats.total_hyperedges as u32, + avg_degree: stats.avg_entity_degree as f64, + }), + }) +} + +/// Properties this binding stores for its own use rather than the caller's. +/// +/// Embeddings and edge confidences are persisted as ordinary properties under a +/// `__` prefix. Returning them would put a 384-element float dump in every +/// result row's property map — invisible until #879 made `query()` return rows +/// at all. +fn is_internal_property(key: &str) -> bool { + key.starts_with("__") +} + +/// Render a stored property for the string-valued JS result maps. +/// +/// `format!("{:?}", v)` was used here, which surfaced `String("alice")` to +/// JavaScript instead of `alice`. Scalars now render as their plain value. +fn property_to_string(value: &PropertyValue) -> String { + match value { + PropertyValue::String(s) => s.clone(), + PropertyValue::Integer(i) => i.to_string(), + PropertyValue::Float(f) => f.to_string(), + PropertyValue::Boolean(b) => b.to_string(), + PropertyValue::Null => String::new(), + other => format!("{:?}", other), + } +} + #[napi] impl GraphDatabase { /// Create a new graph database @@ -148,10 +371,10 @@ impl GraphDatabase { causal_memory: Arc::new(RwLock::new(CoreCausalMemory::new(core_metric))), transaction_manager: Arc::new(RwLock::new(transactions::TransactionManager::new())), graph_db: Arc::new(RwLock::new(GraphDB::new())), + hydrated: Arc::new(Mutex::new(false)), storage, storage_path, }; - db.hydrate_from_storage()?; Ok(db) } @@ -173,10 +396,10 @@ impl GraphDatabase { causal_memory: Arc::new(RwLock::new(CoreCausalMemory::new(metric))), transaction_manager: Arc::new(RwLock::new(transactions::TransactionManager::new())), graph_db: Arc::new(RwLock::new(GraphDB::new())), + hydrated: Arc::new(Mutex::new(false)), storage: Some(Arc::new(RwLock::new(storage))), storage_path: Some(path), }; - db.hydrate_from_storage()?; Ok(db) } @@ -199,86 +422,6 @@ impl GraphDatabase { self.storage_path.clone() } - /// Replay persisted records into the in-memory property and hypergraph indexes. - fn hydrate_from_storage(&self) -> Result<()> { - let Some(storage_arc) = self.storage.as_ref() else { - return Ok(()); - }; - let storage = storage_arc.read().expect("Storage RwLock poisoned"); - let mut hg = self.hypergraph.write().expect("RwLock poisoned"); - let gdb = self.graph_db.write().expect("RwLock poisoned"); - - for id in storage - .all_node_ids() - .map_err(|e| Error::from_reason(format!("hydrate nodes: {e}")))? - { - if let Some(node) = storage - .get_node(&id) - .map_err(|e| Error::from_reason(format!("hydrate node {id}: {e}")))? - { - let embedding = prop_to_f32_vec(node.properties.get("__embedding")); - hg.add_entity(node.id.clone(), embedding); - gdb.create_node(node) - .map_err(|e| Error::from_reason(format!("hydrate node insert: {e}")))?; - } - } - - for id in storage - .all_edge_ids() - .map_err(|e| Error::from_reason(format!("hydrate edges: {e}")))? - { - if let Some(edge) = storage - .get_edge(&id) - .map_err(|e| Error::from_reason(format!("hydrate edge {id}: {e}")))? - { - let confidence = prop_to_f32_vec(edge.properties.get("__confidence")) - .first() - .copied() - .unwrap_or(1.0); - let embedding = prop_to_f32_vec(edge.properties.get("__embedding")); - let mut core_edge = CoreHyperedge::new( - vec![edge.from.clone(), edge.to.clone()], - edge.edge_type.clone(), - embedding, - confidence, - ); - core_edge.id = edge.id.clone(); - // A non-cascaded deletion deliberately leaves the durable edge, - // but neither in-memory index accepts an edge with a missing node. - if hg.add_hyperedge(core_edge).is_err() { - continue; - } - gdb.create_edge(edge) - .map_err(|e| Error::from_reason(format!("hydrate edge insert: {e}")))?; - } - } - - for id in storage - .all_hyperedge_ids() - .map_err(|e| Error::from_reason(format!("hydrate hyperedges: {e}")))? - { - if let Some(hyperedge) = storage - .get_hyperedge(&id) - .map_err(|e| Error::from_reason(format!("hydrate hyperedge {id}: {e}")))? - { - let embedding = prop_to_f32_vec(hyperedge.properties.get("__embedding")); - let mut core_edge = CoreHyperedge::new( - hyperedge.nodes, - hyperedge - .description - .unwrap_or_else(|| hyperedge.edge_type.clone()), - embedding, - hyperedge.confidence, - ); - core_edge.id = hyperedge.id; - // A non-cascaded deletion can deliberately leave this dangling. - let _ = hg.add_hyperedge(core_edge); - } - } - - Ok(()) - } - /// Create a node in the graph /// /// # Example @@ -299,7 +442,9 @@ impl GraphDatabase { let properties = node.properties.clone(); let labels = node.labels.clone(); + let hydrated = self.hydrated.clone(); tokio::task::spawn_blocking(move || { + hydrate_once(&hydrated, storage.as_ref(), &hypergraph, &graph_db)?; let mut hg = hypergraph.write().expect("RwLock poisoned"); let mut gdb = graph_db.write().expect("RwLock poisoned"); @@ -343,7 +488,9 @@ impl GraphDatabase { let embedding = edge.embedding.to_vec(); let confidence = edge.confidence.unwrap_or(1.0) as f32; + let hydrated = self.hydrated.clone(); tokio::task::spawn_blocking(move || { + hydrate_once(&hydrated, storage.as_ref(), &hypergraph, &graph_db)?; let core_edge = CoreHyperedge::new(nodes, description.clone(), embedding.clone(), confidence); let edge_id = core_edge.id.clone(); @@ -402,7 +549,10 @@ impl GraphDatabase { let embedding = hyperedge.embedding.to_vec(); let confidence = hyperedge.confidence.unwrap_or(1.0) as f32; + let graph_db = self.graph_db.clone(); + let hydrated = self.hydrated.clone(); tokio::task::spawn_blocking(move || { + hydrate_once(&hydrated, storage.as_ref(), &hypergraph, &graph_db)?; let core_edge = CoreHyperedge::new( nodes.clone(), description.clone(), @@ -449,74 +599,12 @@ impl GraphDatabase { pub async fn query(&self, cypher: String) -> Result { let graph_db = self.graph_db.clone(); let hypergraph = self.hypergraph.clone(); + let storage = self.storage.clone(); + let hydrated = self.hydrated.clone(); tokio::task::spawn_blocking(move || { - // Parse the Cypher query - let parsed = parse_cypher(&cypher) - .map_err(|e| Error::from_reason(format!("Cypher parse error: {}", e)))?; - - let gdb = graph_db.read().expect("RwLock poisoned"); - let hg = hypergraph.read().expect("RwLock poisoned"); - - let mut result_nodes: Vec = Vec::new(); - let mut result_edges: Vec = Vec::new(); - - // Execute each statement - for statement in &parsed.statements { - match statement { - Statement::Match(match_clause) => { - // Extract label from match patterns for query - for pattern in &match_clause.patterns { - if let ruvector_graph::cypher::ast::Pattern::Node(node_pattern) = - pattern - { - for label in &node_pattern.labels { - let nodes = gdb.get_nodes_by_label(label); - for node in nodes { - result_nodes.push(JsNodeResult { - id: node.id.clone(), - labels: node - .labels - .iter() - .map(|l| l.name.clone()) - .collect(), - properties: node - .properties - .iter() - .map(|(k, v)| (k.clone(), format!("{:?}", v))) - .collect(), - }); - } - } - // If no labels specified, return all nodes (simplified) - if node_pattern.labels.is_empty() && node_pattern.variable.is_some() - { - // This would need iteration over all nodes - for now just stats - } - } - } - } - Statement::Create(create_clause) => { - // Handle CREATE - but we need mutable access, so skip in query - } - Statement::Return(_) => { - // RETURN is handled implicitly - } - _ => {} - } - } - - let stats = hg.stats(); - - Ok::(JsQueryResult { - nodes: result_nodes, - edges: result_edges, - stats: Some(JsGraphStats { - total_nodes: stats.total_entities as u32, - total_edges: stats.total_hyperedges as u32, - avg_degree: stats.avg_entity_degree as f64, - }), - }) + hydrate_once(&hydrated, storage.as_ref(), &hypergraph, &graph_db)?; + run_query(&cypher, &graph_db, &hypergraph) }) .await .map_err(|e| Error::from_reason(format!("Task failed: {}", e)))? @@ -524,25 +612,23 @@ impl GraphDatabase { /// Query the graph synchronously /// + /// Identical semantics to [`GraphDatabase::query`], but it runs on the + /// calling thread. On a persisted database the first call also pays for + /// hydration, so prefer the async `query()` on large graphs. + /// /// # Example /// ```javascript /// const results = db.querySync('MATCH (n) RETURN n LIMIT 10'); /// ``` #[napi] pub fn query_sync(&self, cypher: String) -> Result { - let hg = self.hypergraph.read().expect("RwLock poisoned"); - let stats = hg.stats(); - - // Simplified query result for now - Ok(JsQueryResult { - nodes: vec![], - edges: vec![], - stats: Some(JsGraphStats { - total_nodes: stats.total_entities as u32, - total_edges: stats.total_hyperedges as u32, - avg_degree: stats.avg_entity_degree as f64, - }), - }) + hydrate_once( + &self.hydrated, + self.storage.as_ref(), + &self.hypergraph, + &self.graph_db, + )?; + run_query(&cypher, &self.graph_db, &self.hypergraph) } /// Search for similar hyperedges @@ -563,7 +649,11 @@ impl GraphDatabase { let embedding = query.embedding.to_vec(); let k = query.k as usize; + let graph_db = self.graph_db.clone(); + let storage = self.storage.clone(); + let hydrated = self.hydrated.clone(); tokio::task::spawn_blocking(move || { + hydrate_once(&hydrated, storage.as_ref(), &hypergraph, &graph_db)?; let hg = hypergraph.read().expect("RwLock poisoned"); let results = hg.search_hyperedges(&embedding, k); @@ -592,7 +682,11 @@ impl GraphDatabase { let hypergraph = self.hypergraph.clone(); let hops = k as usize; + let graph_db = self.graph_db.clone(); + let storage = self.storage.clone(); + let hydrated = self.hydrated.clone(); tokio::task::spawn_blocking(move || { + hydrate_once(&hydrated, storage.as_ref(), &hypergraph, &graph_db)?; let hg = hypergraph.read().expect("RwLock poisoned"); let neighbors = hg.k_hop_neighbors(start_node, hops); Ok::, Error>(neighbors.into_iter().collect()) @@ -676,7 +770,9 @@ impl GraphDatabase { let nodes = batch.nodes; let edges = batch.edges; + let hydrated = self.hydrated.clone(); tokio::task::spawn_blocking(move || { + hydrate_once(&hydrated, storage.as_ref(), &hypergraph, &graph_db)?; let mut hg = hypergraph.write().expect("RwLock poisoned"); let mut gdb = graph_db.write().expect("RwLock poisoned"); let mut node_ids = Vec::new(); @@ -765,7 +861,9 @@ impl GraphDatabase { let storage = self.storage.clone(); let cascade = opts.and_then(|o| o.cascade).unwrap_or(false); + let hydrated = self.hydrated.clone(); tokio::task::spawn_blocking(move || { + hydrate_once(&hydrated, storage.as_ref(), &hypergraph, &graph_db)?; let graph_edge_ids = if cascade { let gdb = graph_db.read().expect("RwLock poisoned"); let mut ids: Vec = gdb @@ -867,7 +965,9 @@ impl GraphDatabase { let storage = self.storage.clone(); let hypergraph = self.hypergraph.clone(); + let hydrated = self.hydrated.clone(); tokio::task::spawn_blocking(move || { + hydrate_once(&hydrated, storage.as_ref(), &hypergraph, &graph_db)?; let deleted_from_hypergraph = hypergraph .write() .expect("RwLock poisoned") @@ -909,7 +1009,9 @@ impl GraphDatabase { let graph_db = self.graph_db.clone(); let storage = self.storage.clone(); + let hydrated = self.hydrated.clone(); tokio::task::spawn_blocking(move || { + hydrate_once(&hydrated, storage.as_ref(), &hypergraph, &graph_db)?; let deleted_from_hypergraph = hypergraph .write() .expect("RwLock poisoned") @@ -965,7 +1067,11 @@ impl GraphDatabase { pub async fn stats(&self) -> Result { let hypergraph = self.hypergraph.clone(); + let graph_db = self.graph_db.clone(); + let storage = self.storage.clone(); + let hydrated = self.hydrated.clone(); tokio::task::spawn_blocking(move || { + hydrate_once(&hydrated, storage.as_ref(), &hypergraph, &graph_db)?; let hg = hypergraph.read().expect("RwLock poisoned"); let stats = hg.stats(); @@ -1047,6 +1153,86 @@ mod tests { (db, edge_id, hyperedge_id) } + /// Hydration is deferred to first use (ruvnet/ruvector#826), so the thing + /// that can now go wrong is replaying the store more than once and double + /// counting every record. Drive several entry points against one freshly + /// opened handle and assert the counts never move. + #[tokio::test] + async fn deferred_hydration_replays_exactly_once() { + let path = temp_storage_path("hydrate-once"); + let (db, _, _) = create_persistent_fixture(&path).await; + drop(db); + + let reopened = GraphDatabase::open(path.clone()).expect("reopen persisted database"); + // First touch triggers the replay; every later one must be a no-op. + assert_eq!(reopened.stats().await.expect("stats").total_nodes, 2); + assert_eq!(reopened.stats().await.expect("stats").total_nodes, 2); + let rows = reopened + .query("MATCH (n) RETURN n".to_string()) + .await + .expect("query"); + assert_eq!(rows.nodes.len(), 2, "one row per persisted node, not two"); + assert_eq!( + reopened + .k_hop_neighbors("a".to_string(), 1) + .await + .expect("khop") + .len(), + 2 + ); + assert_eq!(reopened.stats().await.expect("stats").total_nodes, 2); + drop(reopened); + std::fs::remove_file(path).expect("remove test database"); + } + + /// The label-less `MATCH (n)` and `WHERE` shapes from #879, end to end + /// through the NAPI surface rather than just the executor. + #[tokio::test] + async fn query_returns_rows_for_label_less_match_and_where() { + let db = GraphDatabase::new(Some(JsGraphOptions { + distance_metric: Some(JsDistanceMetric::Cosine), + dimensions: Some(2), + storage_path: None, + })) + .expect("create database"); + for id in ["n1", "n2"] { + db.create_node(JsNode { + id: id.to_string(), + embedding: Float32Array::new(vec![1.0, 0.0]), + labels: Some(vec!["Person".to_string()]), + properties: None, + }) + .await + .expect("create node"); + } + + let all = db + .query("MATCH (n) RETURN n".to_string()) + .await + .expect("label-less match"); + assert_eq!(all.nodes.len(), 2); + + let one = db + .query("MATCH (n) WHERE n.id = 'n1' RETURN n".to_string()) + .await + .expect("point lookup"); + assert_eq!(one.nodes.len(), 1); + assert_eq!(one.nodes[0].id, "n1"); + + // querySync used to ignore its argument entirely and always return []. + let sync = db + .query_sync("MATCH (n) WHERE n.id = 'n2' RETURN n".to_string()) + .expect("sync point lookup"); + assert_eq!(sync.nodes.len(), 1); + assert_eq!(sync.nodes[0].id, "n2"); + + // A write through query() is refused rather than silently dropped. + assert!(db + .query("CREATE (n:Person {name: 'x'})".to_string()) + .await + .is_err()); + } + #[tokio::test] async fn persisted_graph_hydrates_after_reopen() { let path = temp_storage_path("reopen"); diff --git a/crates/ruvector-graph-node/src/streaming.rs b/crates/ruvector-graph-node/src/streaming.rs index 1510aeee7f..b75cd3f984 100644 --- a/crates/ruvector-graph-node/src/streaming.rs +++ b/crates/ruvector-graph-node/src/streaming.rs @@ -22,17 +22,13 @@ impl QueryResultStream { #[napi] impl QueryResultStream { - /// Get the next result from the stream + /// Get the next result from the stream. /// - /// # Example - /// ```javascript - /// const stream = await db.queryStream('MATCH (n) RETURN n'); - /// while (true) { - /// const result = await stream.next(); - /// if (!result) break; - /// console.log(result); - /// } - /// ``` + /// **Not implemented.** This always returns `null`, and no method on + /// `GraphDatabase` returns a `QueryResultStream` in the first place — there + /// is no `db.queryStream()`. The previous doc comment here showed exactly + /// that call, which does not exist. Use `db.query()` until streaming is + /// built; this type is exported only to keep the shape reserved. #[napi] pub fn next(&mut self) -> Result> { // This would poll the stream in a real implementation @@ -56,15 +52,11 @@ impl HyperedgeStream { #[napi] impl HyperedgeStream { - /// Get the next hyperedge result + /// Get the next hyperedge result. /// - /// # Example - /// ```javascript - /// const stream = await db.searchHyperedgesStream(query); - /// for await (const result of stream) { - /// console.log(result); - /// } - /// ``` + /// Note: no method on `GraphDatabase` returns a `HyperedgeStream` — there is + /// no `db.searchHyperedgesStream()`, which the previous doc example here + /// claimed. Use `db.searchHyperedges()`, which returns the full array. #[napi] pub fn next(&mut self) -> Result> { if self.index < self.results.len() { diff --git a/crates/ruvector-graph/src/graph.rs b/crates/ruvector-graph/src/graph.rs index 60f33ea6c4..8bb9883647 100644 --- a/crates/ruvector-graph/src/graph.rs +++ b/crates/ruvector-graph/src/graph.rs @@ -274,6 +274,22 @@ impl GraphDB { .collect() } + /// Get every node in the graph. + /// + /// This is a full scan of the node map — it backs the label-less Cypher + /// pattern `MATCH (n)`, which has no index to consult by definition. + pub fn all_nodes(&self) -> Vec { + self.nodes.iter().map(|e| e.value().clone()).collect() + } + + /// Get every edge in the graph. + /// + /// Full scan, for the same reason as [`GraphDB::all_nodes`]: a relationship + /// pattern with no type filter (`-[r]->`) cannot use the edge-type index. + pub fn all_edges(&self) -> Vec { + self.edges.iter().map(|e| e.value().clone()).collect() + } + /// Get nodes by property pub fn get_nodes_by_property(&self, key: &str, value: &PropertyValue) -> Vec { self.property_index diff --git a/npm/packages/graph-node/README.md b/npm/packages/graph-node/README.md index 3a47c71fa2..3b316b7d10 100644 --- a/npm/packages/graph-node/README.md +++ b/npm/packages/graph-node/README.md @@ -6,7 +6,7 @@ Native Node.js bindings for RuVector Graph Database with hypergraph support, Cyp - **Native Performance**: Direct NAPI-RS bindings - no WASM overhead - **Hypergraph Support**: Multi-node relationships with vector embeddings -- **Cypher Queries**: Neo4j-compatible query language +- **Cypher Queries**: a practical subset of Cypher — see [Cypher support](#cypher-support) - **Persistence**: ACID-compliant storage with redb backend - **Vector Similarity Search**: Fast k-NN search on embeddings - **Graph Traversal**: k-hop neighbor discovery @@ -81,6 +81,49 @@ const stats = await db.stats(); console.log(\`Nodes: \${stats.totalNodes}, Edges: \${stats.totalEdges}\`); ``` +## Cypher support + +`query()` and `querySync()` run a single-pattern matcher, not a full planner. +Anything it cannot execute returns an **error naming what was refused** — it +never returns an empty result set to mean "unsupported". + +Supported: + +```javascript +await db.query('MATCH (n) RETURN n'); // full scan +await db.query('MATCH (n:Person) RETURN n'); // label index +await db.query("MATCH (n) WHERE n.id = 'alice' RETURN n"); // point lookup +await db.query('MATCH (n:Person) WHERE n.age > 30 RETURN n'); +await db.query('MATCH (n) WHERE n.a > 1 AND n.b < 5 RETURN n'); +await db.query("MATCH (n {name: 'alice'}) RETURN n"); // inline props +await db.query('MATCH (a)-[r:knows]->(b) RETURN a, r, b'); // typed edges +await db.query('MATCH (a)-[r]->(b) RETURN r'); // any edge +``` + +`WHERE` handles `=`, `<>`, `<`, `<=`, `>`, `>=`, `AND`, `OR`, arithmetic, and +property access. `n.id` resolves to the node's identity when no stored property +shadows it. A missing property compares false rather than matching or throwing. + +Not supported — these **raise an error**, they do not silently return `[]`: + +| Construct | Use instead | +|---|---| +| `CREATE` / `SET` / `DELETE` via `query()` | `createNode()`, `createEdge()`, `deleteNode()` | +| variable-length paths, `[*1..3]` | `kHopNeighbors()` | +| chained patterns, `(a)-[]->(b)<-[]-(c)` | separate queries | +| hyperedge patterns in `MATCH` | `searchHyperedges()` | + +Not supported at the parser level, so these fail to parse: `CONTAINS`, +`STARTS WITH`, `ENDS WITH`, `IN`, `IS NULL`, `=~`. Aggregations (`count()`, +`collect()`), `ORDER BY`, `SKIP` and `LIMIT` are parsed but not applied. + +Known defect: `NOT` binds tighter than comparison, so `NOT n.age = 30` parses +as `(NOT n.age) = 30` and matches nothing. Use `<>`. Tracked in +[#939](https://github.com/ruvnet/RuVector/issues/939). + +Internal properties (`__embedding`, `__confidence`) are not returned in result +rows. + ## Benchmarks | Operation | Throughput | Latency | diff --git a/npm/packages/graph-node/index.d.ts b/npm/packages/graph-node/index.d.ts index a32543a65b..28b4de59b5 100644 --- a/npm/packages/graph-node/index.d.ts +++ b/npm/packages/graph-node/index.d.ts @@ -133,6 +133,23 @@ export const enum JsTemporalGranularity { Monthly = 'Monthly', Yearly = 'Yearly' } +/** Options for deleteNode */ +export interface JsDeleteNodeOptions { + /** If true, all incident hyperedges are removed along with the node */ + cascade?: boolean +} +/** Result of deleting a node (with optional cascade) */ +export interface JsDeleteNodeResult { + /** Whether the node existed and was deleted */ + deletedNode: boolean + /** Number of incident edges/hyperedges removed (cascade only) */ + deletedEdges: number +} +/** Result of deleting a single edge or hyperedge */ +export interface JsDeleteResult { + /** Whether the record existed and was deleted */ + deleted: boolean +} /** Temporal hyperedge */ export interface JsTemporalHyperedge { /** Base hyperedge */ @@ -151,32 +168,24 @@ export declare function hello(): string /** Streaming query result iterator */ export declare class QueryResultStream { /** - * Get the next result from the stream + * Get the next result from the stream. * - * # Example - * ```javascript - * const stream = await db.queryStream('MATCH (n) RETURN n'); - * while (true) { - * const result = await stream.next(); - * if (!result) break; - * console.log(result); - * } - * ``` + * **Not implemented.** This always returns `null`, and no method on + * `GraphDatabase` returns a `QueryResultStream` in the first place — there + * is no `db.queryStream()`. The previous doc comment here showed exactly + * that call, which does not exist. Use `db.query()` until streaming is + * built; this type is exported only to keep the shape reserved. */ next(): JsQueryResult | null } /** Streaming hyperedge result iterator */ export declare class HyperedgeStream { /** - * Get the next hyperedge result + * Get the next hyperedge result. * - * # Example - * ```javascript - * const stream = await db.searchHyperedgesStream(query); - * for await (const result of stream) { - * console.log(result); - * } - * ``` + * Note: no method on `GraphDatabase` returns a `HyperedgeStream` — there is + * no `db.searchHyperedgesStream()`, which the previous doc example here + * claimed. Use `db.searchHyperedges()`, which returns the full array. */ next(): JsHyperedgeResult | null /** Collect all remaining results */ @@ -280,6 +289,10 @@ export declare class GraphDatabase { /** * Query the graph synchronously * + * Identical semantics to [`GraphDatabase::query`], but it runs on the + * calling thread. On a persisted database the first call also pays for + * hydration, so prefer the async `query()` on large graphs. + * * # Example * ```javascript * const results = db.querySync('MATCH (n) RETURN n LIMIT 10'); @@ -346,6 +359,36 @@ export declare class GraphDatabase { * ``` */ batchInsert(batch: JsBatchInsert): Promise + /** + * Delete a node and optionally cascade to its incident hyperedges + * + * # Example + * ```javascript + * const result = await db.deleteNode('node1', { cascade: true }); + * console.log(`Deleted: ${result.deletedNode}, edges removed: ${result.deletedEdges}`); + * ``` + */ + deleteNode(id: string, opts?: JsDeleteNodeOptions | undefined | null): Promise + /** + * Delete an edge by ID + * + * # Example + * ```javascript + * const result = await db.deleteEdge('edge-id'); + * console.log(`Deleted: ${result.deleted}`); + * ``` + */ + deleteEdge(id: string): Promise + /** + * Delete a hyperedge by ID + * + * # Example + * ```javascript + * const result = await db.deleteHyperedge('hyperedge-id'); + * console.log(`Deleted: ${result.deleted}`); + * ``` + */ + deleteHyperedge(id: string): Promise /** * Subscribe to graph changes (returns a change stream) * diff --git a/npm/packages/graph-node/test.js b/npm/packages/graph-node/test.js index d6d2e7ed5d..380cc3cbc6 100644 --- a/npm/packages/graph-node/test.js +++ b/npm/packages/graph-node/test.js @@ -1,4 +1,5 @@ const { GraphDatabase, version, hello } = require('./index.js'); +const assert = require('node:assert'); const fs = require('fs'); const path = require('path'); const os = require('os'); @@ -69,11 +70,46 @@ console.log('3. Creating nodes:'); console.log(' Created hyperedge:', hyperedgeId); console.log(' ✓ Hyperedge created\n'); - // Test 6: Query + // Test 6: Query (regression cover for #879 — these shapes returned [] in 2.0.4) console.log('6. Querying graph:'); - const results = await db.query('MATCH (n) RETURN n'); - console.log(' Query results:', JSON.stringify(results, null, 2)); - console.log(' ✓ Query executed\n'); + + // 6a. Label-less MATCH must see every node, not zero. + const all = await db.query('MATCH (n) RETURN n'); + console.log(' MATCH (n) ->', all.nodes.length, 'nodes'); + assert.strictEqual(all.nodes.length, 3, 'MATCH (n) must return all 3 nodes'); + + // 6b. Label-scoped MATCH still narrows. + const people = await db.query('MATCH (n:Person) RETURN n'); + assert.strictEqual(people.nodes.length, 2, 'MATCH (n:Person) must return 2 nodes'); + + // 6c. WHERE actually filters — the point-lookup shape from #879. + const one = await db.query("MATCH (n) WHERE n.id = 'alice' RETURN n"); + assert.strictEqual(one.nodes.length, 1, 'point lookup must return exactly 1 node'); + assert.strictEqual(one.nodes[0].id, 'alice'); + assert.ok(one.nodes[0].labels.includes('Person'), 'labels must round-trip'); + assert.strictEqual(one.nodes[0].properties.name, 'Alice', 'properties render as plain values'); + assert.ok( + !('__embedding' in one.nodes[0].properties), + 'internal __-prefixed properties must not leak into results' + ); + + // 6d. Relationship patterns populate `edges`, which was always []. + const rels = await db.query('MATCH (a)-[r]->(b) RETURN a, r, b'); + assert.ok(rels.edges.length >= 1, 'a relationship pattern must return edges'); + assert.ok(rels.edges[0].from && rels.edges[0].to, 'edge endpoints must be populated'); + + // 6e. querySync used to ignore its argument entirely. + const sync = db.querySync("MATCH (n) WHERE n.id = 'bob' RETURN n"); + assert.strictEqual(sync.nodes.length, 1, 'querySync must execute its argument'); + assert.strictEqual(sync.nodes[0].id, 'bob'); + + // 6f. Unsupported constructs error instead of returning an empty result. + await assert.rejects( + () => db.query("CREATE (n:Person {name: 'x'})"), + /Unsupported Cypher/, + 'writes through query() must be refused, not silently dropped' + ); + console.log(' ✓ Query returns real rows for MATCH/WHERE/relationships\n'); // Test 7: Search hyperedges console.log('7. Searching hyperedges:'); @@ -149,11 +185,32 @@ console.log('3. Creating nodes:'); const persistentStats = await persistentDb.stats(); console.log(' Persistent DB stats:', persistentStats); + assert.strictEqual(persistentStats.totalNodes, 1, 'the write must be visible on its own handle'); - // Test opening existing database + // Test opening existing database. This is the #879 headline: a second + // handle used to report zero for everything the first handle had written. console.log(' Opening existing database with GraphDatabase.open()...'); const reopenedDb = GraphDatabase.open(dbPath); - console.log(' Reopened isPersistent():', reopenedDb.isPersistent()); + assert.strictEqual(reopenedDb.isPersistent(), true); + const reopenedStats = await reopenedDb.stats(); + console.log(' Reopened DB stats:', reopenedStats); + assert.strictEqual( + reopenedStats.totalNodes, + persistentStats.totalNodes, + 'a reopened handle must see the persisted nodes' + ); + const reopenedRows = await reopenedDb.query('MATCH (n) RETURN n'); + assert.strictEqual(reopenedRows.nodes.length, 1, 'and must be able to query them back'); + assert.strictEqual(reopenedRows.nodes[0].id, 'persistent_node_1'); + + // A second handle in the *same* process must agree too. + const secondHandle = new GraphDatabase({ + distanceMetric: 'Cosine', + dimensions: 3, + storagePath: dbPath + }); + const secondStats = await secondHandle.stats(); + assert.strictEqual(secondStats.totalNodes, 1, 'a same-process second handle must see the data'); // Cleanup try {