From 432336dc2462a21a9509f1f721da1d1a89b76cfe Mon Sep 17 00:00:00 2001 From: nareshkolimi Date: Mon, 19 Oct 2026 17:43:01 +0530 Subject: [PATCH] A SET applies its writes in the order they were written (#1896) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SetClause` held three vectors by kind (property, label, whole-entity), so the order between kinds was lost at parse time, and `SetPropertyOperator` ran every `n.k = v` before every `n = {…}`. `SET n.k = 7, n = {…}` came out right and `SET n = {…}, n.k = 7` silently lost `k`, which is the bulk-import shape `SET n = row.props, n.source_id = row.id`. - ast: `SetClause { ops: Vec }`, one ordered sequence; `items()`, `label_items()` and `entity_items()` read it by kind. - operator: `SetPropertyOperator` holds `Vec` and applies it in order. Property values are still all evaluated before any write, so `SET a.x = a.y, a.y = a.x` still swaps. - planner: `attach_set` builds operators per clause instead of flattening every clause into one list, so `SET n = {…} SET n.k = 7` keeps `k`. `Clause::Set` and the bare SET after MERGE go through it too; the MERGE path used to keep only `n.prop = v` items and drop `n = {…}` and labels. - tests/set_order.rs pins both orders, two clauses, the import shape, `+=`, the swap, and SET after MERGE. --- src/query/ast.rs | 54 +++++++++++--- src/query/executor/mod.rs | 11 +-- src/query/executor/operator.rs | 86 +++++++++++++++------- src/query/executor/planner.rs | 85 +++++++++------------- src/query/parser.rs | 23 +++--- src/query/parser_cov_tests.rs | 12 ++-- src/query/validate.rs | 14 ++-- tests/set_order.rs | 127 +++++++++++++++++++++++++++++++++ 8 files changed, 297 insertions(+), 115 deletions(-) create mode 100644 tests/set_order.rs diff --git a/src/query/ast.rs b/src/query/ast.rs index 69322403..3fecb27f 100644 --- a/src/query/ast.rs +++ b/src/query/ast.rs @@ -1014,16 +1014,52 @@ pub struct DeleteClause { /// SET clause #[derive(Debug, Clone, PartialEq)] pub struct SetClause { - /// Items to set - pub items: Vec, - /// Labels to add: `SET n:Admin`, `SET n:A:B`. + /// The items, in the order they were written. /// - /// Kept beside `items` rather than folded into it because `SetItem` is a - /// struct read by field in several places, and an enum would touch all of - /// them for no gain. `RemoveItem` is an enum because it always was. - pub label_items: Vec, - /// Whole-entity assignment: `SET n = {…}` and `SET n += {…}`. - pub entity_items: Vec, + /// One sequence rather than a vector per kind, because the order between + /// kinds is part of the meaning: `SET n = {a: 1}, n.k = 7` keeps `k` and + /// `SET n.k = 7, n = {a: 1}` does not. Three vectors kept each kind's own + /// order and lost the order between them, so the executor had to pick one + /// -- and whichever it picked, the other way round was silently wrong + /// (#1896). + pub ops: Vec, +} + +impl SetClause { + /// `n.prop = value` items, in order. + pub fn items(&self) -> impl Iterator { + self.ops.iter().filter_map(|op| match op { + SetOp::Property(i) => Some(i), + _ => None, + }) + } + + /// `n:Label` items, in order. + pub fn label_items(&self) -> impl Iterator { + self.ops.iter().filter_map(|op| match op { + SetOp::Labels(i) => Some(i), + _ => None, + }) + } + + /// `n = …` and `n += …` items, in order. + pub fn entity_items(&self) -> impl Iterator { + self.ops.iter().filter_map(|op| match op { + SetOp::Entity(i) => Some(i), + _ => None, + }) + } +} + +/// One item of a `SET` clause. +#[derive(Debug, Clone, PartialEq)] +pub enum SetOp { + /// `n.prop = value` + Property(SetItem), + /// `n:Admin`, `n:A:B` + Labels(SetLabelItem), + /// `n = {…}` (replace) or `n += {…}` (merge) + Entity(SetEntityItem), } /// `SET n = ` (replace every property) or `SET n += ` (merge). diff --git a/src/query/executor/mod.rs b/src/query/executor/mod.rs index e59a28b9..f57a4ea3 100644 --- a/src/query/executor/mod.rs +++ b/src/query/executor/mod.rs @@ -1084,11 +1084,12 @@ fn substitute_set( sc: &mut crate::query::ast::SetClause, p: &HashMap, ) -> ExecutionResult<()> { - for item in &mut sc.items { - substitute_expr(&mut item.value, p)?; - } - for item in &mut sc.entity_items { - substitute_expr(&mut item.value, p)?; + for op in &mut sc.ops { + match op { + crate::query::ast::SetOp::Property(item) => substitute_expr(&mut item.value, p)?, + crate::query::ast::SetOp::Entity(item) => substitute_expr(&mut item.value, p)?, + crate::query::ast::SetOp::Labels(_) => {} + } } Ok(()) } diff --git a/src/query/executor/operator.rs b/src/query/executor/operator.rs index 5185b9eb..51a084ac 100644 --- a/src/query/executor/operator.rs +++ b/src/query/executor/operator.rs @@ -20611,25 +20611,50 @@ fn apply_entity_assignment( Ok(()) } +/// One write of a `SET` clause, as `SetPropertyOperator` applies it. +#[derive(Debug, Clone)] +pub enum SetWrite { + /// `variable.property = value` + Property(String, String, Expression), + /// `variable = value` or, when the flag is set, `variable += value`. + Entity(String, bool, Expression), +} + pub struct SetPropertyOperator { input: OperatorBox, - items: Vec<(String, String, Expression)>, // (variable, property, value_expr) - /// Whole-entity assignments: `(variable, merge, value)`. `merge` is `+=`. - entity_items: Vec<(String, bool, Expression)>, + /// The writes, in the order the query wrote them. + /// + /// Applied in that order because the order between a property and a + /// whole-entity write decides the result: `SET n = {a: 1}, n.k = 7` keeps + /// `k`, and `SET n.k = 7, n = {a: 1}` replaces it. Applying every + /// property write first and every entity write after made the second form + /// right and the first silently lose `k` (#1896). + writes: Vec, } impl SetPropertyOperator { pub fn new(input: OperatorBox, items: Vec<(String, String, Expression)>) -> Self { - Self { input, items, entity_items: Vec::new() } + let writes = items.into_iter().map(|(v, p, e)| SetWrite::Property(v, p, e)).collect(); + Self { input, writes } } - /// With whole-entity assignments (`SET n = {…}`, `SET n += {…}`). + /// The property writes, then the whole-entity ones (`SET n = {…}`, + /// `SET n += {…}`). For writes in the order a query gave them, use + /// [`SetPropertyOperator::from_writes`]. pub fn with_entity_items( input: OperatorBox, items: Vec<(String, String, Expression)>, entity_items: Vec<(String, bool, Expression)>, ) -> Self { - Self { input, items, entity_items } + let mut writes: Vec = + items.into_iter().map(|(v, p, e)| SetWrite::Property(v, p, e)).collect(); + writes.extend(entity_items.into_iter().map(|(v, m, e)| SetWrite::Entity(v, m, e))); + Self { input, writes } + } + + /// Writes applied in exactly this order. + pub fn from_writes(input: OperatorBox, writes: Vec) -> Self { + Self { input, writes } } /// The properties a right-hand side contributes. @@ -20697,8 +20722,11 @@ impl PhysicalOperator for SetPropertyOperator { fn next_mut(&mut self, store: &mut GraphStore, tenant_id: &str) -> ExecutionResult> { if let Some(record) = self.input.next_mut(store, tenant_id)? { - // Evaluate all SET expressions first (immutable borrow of store) - let evaluated: Vec<_> = self.items.iter().map(|(var, prop, expr)| { + // Evaluate every property write's value first, before anything is + // written, so `SET a.x = a.y, a.y = a.x` swaps. A whole-entity + // write is evaluated where it stands, as it always was. + let evaluated: Vec> = self.writes.iter().map(|w| { + let SetWrite::Property(_, _, expr) = w else { return None }; let val = match eval_expression(expr, &record, store) { Ok(v) => match v { Value::Property(pv) => pv, @@ -20748,7 +20776,7 @@ impl PhysicalOperator for SetPropertyOperator { }, Err(_) => PropertyValue::Null, }; - (var.clone(), prop.clone(), val) + Some(val) }).collect(); // A property is a scalar or a list of scalars. `SET a.maplist = @@ -20758,7 +20786,8 @@ impl PhysicalOperator for SetPropertyOperator { // logic, so the shared `storable_property` never saw it; the check // is applied to the result instead, which covers whichever // converter produced it. - for (_, prop, val) in &evaluated { + for (w, val) in self.writes.iter().zip(&evaluated) { + let (SetWrite::Property(_, prop, _), Some(val)) = (w, val) else { continue }; if !property_is_storable(val) { return Err(ExecutionError::TypeError(format!( "InvalidPropertyType: `{prop}` cannot hold a map inside a list. \ @@ -20767,8 +20796,19 @@ impl PhysicalOperator for SetPropertyOperator { } } - // Apply mutations via store methods (syncs columnar + row + index) - for (var, prop, val) in &evaluated { + // Apply mutations via store methods (syncs columnar + row + index), + // in the order the query wrote them. + for (w, val) in self.writes.iter().zip(&evaluated) { + let (var, prop, val) = match (w, val) { + (SetWrite::Property(var, prop, _), Some(val)) => (var, prop, val), + (SetWrite::Entity(var, merge, expr), _) => { + let Some(target) = record.get(var).cloned() else { continue }; + let value = eval_expression(expr, &record, store)?; + apply_entity_assignment(&target, &value, *merge, store, tenant_id)?; + continue; + } + _ => continue, + }; if let Some(node_val) = record.get(var) { // `SET n.prop = null` **removes** the property. Storing a @@ -20800,14 +20840,6 @@ impl PhysicalOperator for SetPropertyOperator { } } - // Whole-entity assignment. Evaluated after the per-property items - // so that `SET n.a = 1, n = {b: 2}` behaves as written rather than - // as ordered by implementation detail. - for (var, merge, expr) in &self.entity_items { - let Some(target) = record.get(var).cloned() else { continue }; - let value = eval_expression(expr, &record, store)?; - apply_entity_assignment(&target, &value, *merge, store, tenant_id)?; - } Ok(Some(record)) } else { @@ -20824,10 +20856,16 @@ impl PhysicalOperator for SetPropertyOperator { } fn describe(&self) -> OperatorDescription { - let mut sets: Vec = self.items.iter().map(|(v, p, e)| format!("{}.{} = {}", v, p, format_expression(e))).collect(); - sets.extend(self.entity_items.iter().map(|(v, merge, e)| { - format!("{} {} {}", v, if *merge { "+=" } else { "=" }, format_expression(e)) - })); + let sets: Vec = self + .writes + .iter() + .map(|w| match w { + SetWrite::Property(v, p, e) => format!("{}.{} = {}", v, p, format_expression(e)), + SetWrite::Entity(v, merge, e) => { + format!("{} {} {}", v, if *merge { "+=" } else { "=" }, format_expression(e)) + } + }) + .collect(); OperatorDescription { name: "SetProperty".to_string(), details: sets.join(", "), diff --git a/src/query/executor/planner.rs b/src/query/executor/planner.rs index 2ea5e2dc..3a706b55 100644 --- a/src/query/executor/planner.rs +++ b/src/query/executor/planner.rs @@ -1796,16 +1796,12 @@ impl QueryPlanner { // A bare `SET` after MERGE applies on both branches, unlike ON CREATE / // ON MATCH. It parsed but was dropped here, so `MERGE (m) SET m.x = 1` // silently left the property unset. + // + // Through `attach_set` like every other SET: this path used to + // keep only the `n.prop = v` items, so `MERGE (m) SET m = {…}` + // and `SET m:Label` were dropped without a word. if !query.set_clauses.is_empty() { - let items: Vec<(String, String, Expression)> = query - .set_clauses - .iter() - .flat_map(|sc| sc.items.iter()) - .map(|item| { - (item.variable.clone(), item.property.clone(), item.value.clone()) - }) - .collect(); - operator = Box::new(SetPropertyOperator::new(operator, items)); + operator = Self::attach_set(operator, &query.set_clauses); } let mut output_columns = Vec::new(); @@ -8609,29 +8605,7 @@ impl QueryPlanner { bound.insert(u.variable.clone()); } Clause::Set(sc) => { - let items: Vec<(String, String, Expression)> = sc - .items - .iter() - .map(|i| (i.variable.clone(), i.property.clone(), i.value.clone())) - .collect(); - let entity_items: Vec<(String, bool, Expression)> = sc - .entity_items - .iter() - .map(|i| (i.variable.clone(), i.merge, i.value.clone())) - .collect(); - if !items.is_empty() || !entity_items.is_empty() { - operator = Box::new(SetPropertyOperator::with_entity_items( - operator, items, entity_items, - )); - } - let adds: Vec<(String, Label)> = sc - .label_items - .iter() - .flat_map(|i| i.labels.iter().map(|l| (i.variable.clone(), l.clone()))) - .collect(); - if !adds.is_empty() { - operator = Box::new(LabelMutationOperator::new(operator, adds, Vec::new())); - } + operator = Self::attach_set(operator, std::slice::from_ref(sc)); } Clause::Remove(rc) => { let mut props = Vec::new(); @@ -9145,31 +9119,38 @@ impl QueryPlanner { operator } - /// `SET` over `operator`: property and whole-entity items, then labels. + /// `SET` over `operator`, one clause after another. + /// + /// Each clause gets its own operators, so a later clause's writes land + /// after an earlier clause's. Flattening every clause into one list lost + /// that, and `SET n = {…} SET n.k = 7` dropped `k` (#1896). Within a + /// clause the property and whole-entity writes keep the order they were + /// written in; labels follow, which no property write can disturb. fn attach_set(mut operator: OperatorBox, set_clauses: &[SetClause]) -> OperatorBox { - let mut items = Vec::new(); - let mut label_adds = Vec::new(); - let mut entity_items = Vec::new(); + use crate::query::executor::operator::SetWrite; for set_clause in set_clauses { - for item in &set_clause.items { - items.push((item.variable.clone(), item.property.clone(), item.value.clone())); - } - for item in &set_clause.label_items { - for label in &item.labels { - label_adds.push((item.variable.clone(), label.clone())); + let mut writes = Vec::new(); + let mut label_adds = Vec::new(); + for op in &set_clause.ops { + match op { + SetOp::Property(i) => writes.push(SetWrite::Property( + i.variable.clone(), + i.property.clone(), + i.value.clone(), + )), + SetOp::Entity(i) => { + writes.push(SetWrite::Entity(i.variable.clone(), i.merge, i.value.clone())) + } + SetOp::Labels(i) => label_adds + .extend(i.labels.iter().map(|l| (i.variable.clone(), l.clone()))), } } - for item in &set_clause.entity_items { - entity_items.push((item.variable.clone(), item.merge, item.value.clone())); + if !writes.is_empty() { + operator = Box::new(SetPropertyOperator::from_writes(operator, writes)); + } + if !label_adds.is_empty() { + operator = Box::new(LabelMutationOperator::new(operator, label_adds, Vec::new())); } - } - if !items.is_empty() || !entity_items.is_empty() { - operator = Box::new(SetPropertyOperator::with_entity_items( - operator, items, entity_items, - )); - } - if !label_adds.is_empty() { - operator = Box::new(LabelMutationOperator::new(operator, label_adds, Vec::new())); } operator } diff --git a/src/query/parser.rs b/src/query/parser.rs index a4f87b5c..d92ccf6f 100644 --- a/src/query/parser.rs +++ b/src/query/parser.rs @@ -1650,17 +1650,16 @@ fn parse_set_label_item(pair: pest::iterators::Pair) -> ParseResult) -> ParseResult { - let mut items = Vec::new(); - let mut label_items: Vec = Vec::new(); - let mut entity_items: Vec = Vec::new(); + use crate::query::ast::SetOp; + let mut ops = Vec::new(); for inner in pair.into_inner() { if inner.as_rule() == Rule::set_label_item { - label_items.push(parse_set_label_item(inner)?); + ops.push(SetOp::Labels(parse_set_label_item(inner)?)); continue; } if inner.as_rule() == Rule::set_entity_item { - entity_items.push(parse_set_entity_item(inner)?); + ops.push(SetOp::Entity(parse_set_entity_item(inner)?)); continue; } if inner.as_rule() == Rule::set_item { @@ -1686,15 +1685,15 @@ fn parse_set_clause(pair: pest::iterators::Pair) -> ParseResult } } - items.push(SetItem { + ops.push(SetOp::Property(SetItem { variable, property, value: value.ok_or_else(|| ParseError::SemanticError("SET item missing value".to_string()))?, - }); + })); } } - Ok(SetClause { items, label_items, entity_items }) + Ok(SetClause { ops }) } fn parse_remove_clause(pair: pest::iterators::Pair) -> ParseResult { @@ -3590,8 +3589,8 @@ mod tests { assert!(result.is_ok(), "Failed to parse SET: {:?}", result.err()); let ast = result.unwrap(); assert_eq!(ast.set_clauses.len(), 1); - assert_eq!(ast.set_clauses[0].items[0].variable, "n"); - assert_eq!(ast.set_clauses[0].items[0].property, "name"); + assert_eq!(ast.set_clauses[0].items().next().unwrap().variable, "n"); + assert_eq!(ast.set_clauses[0].items().next().unwrap().property, "name"); } #[test] @@ -4692,7 +4691,7 @@ mod tests { assert!(result.is_ok(), "Failed to parse SET clause: {:?}", result.err()); let ast = result.unwrap(); assert!(!ast.set_clauses.is_empty()); - let item = &ast.set_clauses[0].items[0]; + let item = ast.set_clauses[0].items().next().unwrap(); assert_eq!(item.variable, "n"); assert_eq!(item.property, "age"); } @@ -4723,7 +4722,7 @@ mod tests { assert!(result.is_ok(), "Failed to parse multiple SET items: {:?}", result.err()); let ast = result.unwrap(); assert!(!ast.set_clauses.is_empty()); - assert!(ast.set_clauses[0].items.len() >= 2); + assert!(ast.set_clauses[0].items().count() >= 2); } #[test] diff --git a/src/query/parser_cov_tests.rs b/src/query/parser_cov_tests.rs index 31702470..01db08b1 100644 --- a/src/query/parser_cov_tests.rs +++ b/src/query/parser_cov_tests.rs @@ -522,12 +522,12 @@ fn with_clause_carries_its_own_filter_and_row_counts() { fn set_items_of_every_form() { let q = ok("MATCH (n) SET n.a = 1, n = {b: 2}, n += {c: 3}, n:L1:L2"); let s = &q.set_clauses[0]; - assert_eq!(s.items.len(), 1); - assert_eq!(s.entity_items.len(), 2); - assert!(!s.entity_items[0].merge); - assert!(s.entity_items[1].merge); - assert_eq!(s.label_items.len(), 1); - assert_eq!(s.label_items[0].labels.len(), 2); + assert_eq!(s.items().count(), 1); + assert_eq!(s.entity_items().count(), 2); + assert!(!s.entity_items().next().unwrap().merge); + assert!(s.entity_items().nth(1).unwrap().merge); + assert_eq!(s.label_items().count(), 1); + assert_eq!(s.label_items().next().unwrap().labels.len(), 2); } #[test] diff --git a/src/query/validate.rs b/src/query/validate.rs index 2bdc0be8..faf350c6 100644 --- a/src/query/validate.rs +++ b/src/query/validate.rs @@ -1837,10 +1837,10 @@ fn validate_variables_are_bound(query: &Query) -> Result<(), ValidationError> { } fn note_set(sc: &SetClause, bound: &mut HashSet) { - for item in &sc.items { + for item in sc.items() { binders(&item.value, bound); } - for item in &sc.entity_items { + for item in sc.entity_items() { binders(&item.value, bound); } } @@ -2035,15 +2035,15 @@ fn validate_variables_are_bound(query: &Query) -> Result<(), ValidationError> { note_expr(e, &mut used); } let mut note_set_use = |sc: &SetClause, used: &mut Vec| { - for item in &sc.items { + for item in sc.items() { used.push(item.variable.clone()); collect_all_vars(&item.value, used); } - for item in &sc.entity_items { + for item in sc.entity_items() { used.push(item.variable.clone()); collect_all_vars(&item.value, used); } - for item in &sc.label_items { + for item in sc.label_items() { used.push(item.variable.clone()); } }; @@ -3094,8 +3094,8 @@ pub fn validate(query: &Query) -> Result<(), ValidationError> { } } fn set_values_of(sc: &crate::query::ast::SetClause) -> Vec<&Expression> { - let mut out: Vec<&Expression> = sc.items.iter().map(|i| &i.value).collect(); - out.extend(sc.entity_items.iter().map(|i| &i.value)); + let mut out: Vec<&Expression> = sc.items().map(|i| &i.value).collect(); + out.extend(sc.entity_items().map(|i| &i.value)); out } let mut set_values: Vec<&Expression> = Vec::new(); diff --git a/tests/set_order.rs b/tests/set_order.rs new file mode 100644 index 00000000..4ff12b1b --- /dev/null +++ b/tests/set_order.rs @@ -0,0 +1,127 @@ +//! A SET applies its writes in the order they were written (#1896). +//! +//! `SET n = {…}` replaces every property and `SET n.k = v` sets one, so which +//! runs first decides the result. The executor used to run every `n.k = v` +//! before every `n = {…}`, whatever the query said: `SET n.k = 7, n = {…}` came +//! out right and `SET n = {…}, n.k = 7` silently lost `k`. The second is the +//! bulk-import shape `SET n = row.props, n.source_id = row.id`, and losing the +//! id loses every later MATCH on it. +//! +//! Every test reads back a property the statement should have *removed* as +//! well as the ones it should have kept: checking only the kept ones would +//! pass against an implementation that never replaced anything. + +use samyama::graph::{GraphStore, PropertyValue}; +use samyama::query::executor::{MutQueryExecutor, QueryExecutor, Value}; +use samyama::query::parser::parse_query; + +fn write(s: &mut GraphStore, q: &str) { + let p = parse_query(q).unwrap_or_else(|e| panic!("`{q}`: {e}")); + MutQueryExecutor::new(s, "default".into()) + .execute(&p) + .unwrap_or_else(|e| panic!("`{q}`: {e}")); +} + +/// The named columns of the single row `q` returns, `None` where absent. +fn row(s: &GraphStore, q: &str, columns: &[&str]) -> Vec> { + let p = parse_query(q).unwrap_or_else(|e| panic!("`{q}`: {e}")); + let out = QueryExecutor::new(s).execute(&p).unwrap_or_else(|e| panic!("`{q}`: {e}")); + assert_eq!(out.records.len(), 1, "`{q}` should return one row"); + let r = &out.records[0]; + columns + .iter() + .map(|c| match r.get(c) { + Some(Value::Property(PropertyValue::String(x))) => Some(x.clone()), + Some(Value::Property(PropertyValue::Integer(i))) => Some(i.to_string()), + Some(Value::Property(PropertyValue::Null)) | Some(Value::Null) | None => None, + other => panic!("unexpected value in {c}: {other:?}"), + }) + .collect() +} + +fn some(v: &str) -> Option { + Some(v.to_string()) +} + +const READ: &str = "MATCH (n:T) RETURN n.name AS name, n.k AS k, n.x AS x"; + +#[test] +fn a_property_after_a_replace_is_kept() { + let mut s = GraphStore::new(); + write(&mut s, "CREATE (n:T) SET n = {name: 'A'}, n.k = 7"); + assert_eq!(row(&s, READ, &["name", "k", "x"]), vec![some("A"), some("7"), None]); +} + +#[test] +fn a_property_before_a_replace_is_replaced() { + // The order the old executor got right. It must stay right. + let mut s = GraphStore::new(); + write(&mut s, "CREATE (n:T) SET n.k = 7, n = {name: 'A'}"); + assert_eq!(row(&s, READ, &["name", "k", "x"]), vec![some("A"), None, None]); +} + +#[test] +fn a_later_set_clause_runs_after_an_earlier_one() { + // Two SET clauses: unambiguous in any reading, and still reordered, + // because every clause was flattened into one list. + let mut s = GraphStore::new(); + write(&mut s, "CREATE (n:T) SET n = {name: 'A'} SET n.k = 7"); + assert_eq!(row(&s, READ, &["name", "k", "x"]), vec![some("A"), some("7"), None]); + + let mut s = GraphStore::new(); + write(&mut s, "CREATE (n:T) SET n.k = 7 SET n = {name: 'A'}"); + assert_eq!(row(&s, READ, &["name", "k", "x"]), vec![some("A"), None, None]); +} + +#[test] +fn the_bulk_import_shape_keeps_its_source_id() { + let mut s = GraphStore::new(); + write( + &mut s, + "UNWIND [{props: {name: 'A'}, fid: 7}] AS row \ + CREATE (n:T) SET n = row.props, n.k = row.fid", + ); + assert_eq!(row(&s, READ, &["name", "k", "x"]), vec![some("A"), some("7"), None]); + // The point of keeping it: a later MATCH on it finds the node. + assert_eq!(row(&s, "MATCH (n:T {k: 7}) RETURN n.name AS name", &["name"]), vec![some("A")]); +} + +#[test] +fn a_replace_on_a_matched_node_removes_what_came_before_it_only() { + let mut s = GraphStore::new(); + write(&mut s, "CREATE (:T {x: 1, name: 'old'})"); + write(&mut s, "MATCH (n:T) SET n.x = 2, n = {name: 'A'}, n.k = 7"); + assert_eq!(row(&s, READ, &["name", "k", "x"]), vec![some("A"), some("7"), None]); +} + +#[test] +fn merge_is_unaffected_by_order() { + let mut s = GraphStore::new(); + write(&mut s, "CREATE (:T {x: 1})"); + write(&mut s, "MATCH (n:T) SET n += {name: 'A'}, n.k = 7"); + assert_eq!(row(&s, READ, &["name", "k", "x"]), vec![some("A"), some("7"), some("1")]); +} + +#[test] +fn property_values_are_still_read_before_any_write() { + // `SET a.x = a.y, a.y = a.x` swaps. Applying writes in order must not + // start evaluating each value after the writes before it. + let mut s = GraphStore::new(); + write(&mut s, "CREATE (:T {k: 1, x: 2})"); + write(&mut s, "MATCH (n:T) SET n.k = n.x, n.x = n.k"); + assert_eq!(row(&s, READ, &["name", "k", "x"]), vec![None, some("2"), some("1")]); +} + +#[test] +fn a_set_after_merge_applies_whole_entity_and_label_items() { + // This path kept only `n.prop = v` items, so the replace and the label + // were dropped without an error. + let mut s = GraphStore::new(); + write(&mut s, "MERGE (n:T {x: 1}) SET n = {name: 'A'}, n.k = 7, n:Extra"); + assert_eq!(row(&s, READ, &["name", "k", "x"]), vec![some("A"), some("7"), None]); + assert_eq!( + row(&s, "MATCH (n:Extra) RETURN n.name AS name", &["name"]), + vec![some("A")] + ); +} +