[nareshkolimi]

Fixes #1896.

The bug

SET n = {name: 'A'}, n.k = 7 stored {name: 'A'} and reported success. Any property assignment written after SET n = map on the same variable was lost, in the same SET clause or a later one. This is the bulk-import shape SET n = row.props, n.source_id = row.id: the id was dropped, and every later MATCH (n {source_id: …}) found nothing.

Root cause

As Kalyanpulikanti-01 traced in the issue, there were two separate losses of order:

1. Within a clause. SetClause kept three vectors by kind (items, label_items, entity_items), so the order between kinds was discarded at parse time. SetPropertyOperator then ran every n.k = v before every n = {…}. That made SET n.k = 7, n = {…} right and SET n = {…}, n.k = 7 wrong. Any rule based on kind gets exactly one of the two orders right.
2. Across clauses. attach_set flattened every SET clause of a query into one list, so SET n = {…} SET n.k = 7 was reordered too.

The fix

• AST (src/query/ast.rs): SetClause { ops: Vec<SetOp> }, with SetOp::{Property, Labels, Entity} in source order. This is one source of truth rather than an ordinal beside three vectors. items(), label_items() and entity_items() read it by kind for the places that only care about one kind (validation and parameter substitution).
• Operator (src/query/executor/operator.rs): SetPropertyOperator holds Vec<SetWrite> and applies it in order. Property values are still all evaluated before any write, as before, so SET a.x = a.y, a.y = a.x still swaps. A whole-entity value is evaluated where it stands, as it always was.
• Planner (src/query/executor/planner.rs): attach_set builds operators per clause. Clause::Set now goes through it instead of keeping its own copy.
• Found along the way: a bare SET after MERGE (MERGE (m) SET …) had its own path that kept only the n.prop = v items. MERGE (m) SET m = {…} and MERGE (m) SET m:Label were dropped without an error. That path now uses attach_set too.

Tests

tests/set_order.rs checks every row of the issue's table:

• the replace-then-property order, which was wrong and is now fixed;
• the property-then-replace order, which was right and still is;
• two SET clauses, in both orders;
• the UNWIND … SET n = row.props, n.k = row.fid import shape, including that a later MATCH (n {k: …}) finds the node;
• += alongside a property;
• the swap, which still reads values before writing;
• SET after MERGE with a replace and a label.

Every test also reads back a property that should have been removed, so an implementation that never replaced anything would fail. cargo check --workspace --tests --examples --benches is clean, and the lint ratchet is under its ceiling.

Not done here

MERGE … ON CREATE SET / ON MATCH SET have the same by-kind split (on_create_set beside on_create_entity_set), and the merge operators apply the property items first. So ON CREATE SET n = row.props, n.id = row.id probably loses id in the same way. Those operators keep their own copies of the set logic in several places, so I've left them to a follow-up rather than widen this PR.

[Kalyanpulikanti-01]

Approving. This is the fix the diagnosis pointed at, and it went further than the diagnosis did.

One source of truth, not an ordinal. SetClause { ops: Vec<SetOp> } with items()/label_items()/entity_items() as filtered iterators is the right shape: kind-only readers (validation, parameter substitution) keep reading by kind, and nothing has to know to sort. An order: usize beside three vectors would have left the invariant unenforced.

The new doc comment also directly replaces the old one that argued against this, which is the honest way to retire a wrong rationale rather than quietly deleting it.

The evaluation split is preserved correctly, and that's the subtle part. evaluated pre-computes only the SetWrite::Property right-hand sides (entity entries map to None), then the single apply loop zips writes with evaluated in source order — properties from the pre-read values, entity assignments evaluated where they stand. So SET a.x = a.y, a.y = a.x still swaps while ordering is now positional. Doing the obvious thing instead — evaluating each RHS at its write — would have fixed #1896 and broken the swap, and property_values_are_still_read_before_any_write is what pins it.

No production caller retains the old order. I checked: all three sites are gone — planner.rs:1844 (SetPropertyOperator::new), :8814 and :9358 (with_entity_items) — replaced by one from_writes. with_entity_items survives as public API for the cov tests and still appends entity items last, which is correct for a constructor that is handed two kind-separated lists and has no ordering to preserve. It is now the only way to get the old behaviour, and its doc comment points at from_writes; worth removing outright once the cov tests move over, since a future caller reaching for it reintroduces #1896 silently.

The third bug is the one worth calling out. MERGE (m) SET m = {…} and MERGE (m) SET m:Label being dropped without an error is a separate silent write loss from the one in the issue — not a reordering, a total discard — and it was found only because this path was being consolidated. That belongs in the release note as its own line, not folded into "SET ordering".

The tests can fail. Every row asserts x is None, so the replace demonstrably happened, alongside k surviving. An implementation that simply stopped replacing would satisfy "the later property is kept" and fail on x — which is exactly the shortcut a narrower test would have let through. the_bulk_import_shape_keeps_its_source_id then reads the node back by {k: 7}, which is the failure the issue was actually about rather than a proxy for it.

One note: SetOp::Labels is carried in ops in source order but labels are still applied by a separate LabelMutationOperator after the property/entity writes, so label position relative to the others is not honoured. That is correct and worth a sentence somewhere, because it reads like an oversight: SET n = {…} replaces properties only and never touches labels, so no ordering between a label item and an entity item is observable. If whole-entity assignment ever gained label semantics, that would stop being true.

I've filed the deferred half as #1918 — ON CREATE SET / ON MATCH SET keeping the same by-kind split. Your read is almost certainly right that ON CREATE SET n = row.props, n.id = row.id loses id, and leaving it inside this PR would have widened it across several operators that each keep their own copy of the set logic. Splitting it was the right call; it just needed to exist somewhere other than a PR description.

Thanks for crediting the trace — and for not stopping at the row in the issue table.

[Kalyanpulikanti-01]

Correction to my review above: the follow-up issue is #1919, not #1918 — another issue took that number while I was writing. Everything else stands.

#1919 confirms the diagnosis against the code: MergeClause keeps four by-kind vectors (ast.rs:1163-1169), the parser buckets them at parser.rs:1922,1932, and MatchMergeEdgeOperator applies on_create_set (operator.rs:17118) before on_create_entity_set (:17143), with MergeOperator carrying the same pair. So it is the same defect, in two operators rather than one.