gregfelice commented on issue #2487:
URL: https://github.com/apache/age/issues/2487#issuecomment-5369740198
Reproduced on current master (`4d9b2506`, AGE 1.8.0, PostgreSQL 18.4) —
identical to what you reported:
```
--- SET over the duplicated entity
cnt | mv
-----+----
1 | 1
1 | 2
--- stored value afterwards
cnt
-----
1
```
To answer the question you actually asked — intended AGE behavior, or a
correctness issue: **I read this as a correctness issue, not intended
semantics.** It should not be closed as working-as-designed.
Under openCypher, `SET` executes once per incoming row and each execution
reads the entity's *current* stored state. The vertex reaches `SET` twice, so
the expected outcome is `cnt` = 1 then 2, with 2 stored. AGE instead evaluates
`n.cnt + 1` against the property snapshot each row carried out of `MATCH`, so
both rows compute `0 + 1` and the second write stores the value the first one
already wrote. The update is lost rather than applied twice.
This is the same defect family as #2493 (fixed in #2495): a write not
visible to a later reader in the same query. #2495 addressed visibility across
*clauses*; this is visibility across *rows within a single clause*, which that
fix did not touch and was not intended to.
The blast radius is wider than an off-by-one counter. Any read-modify-write
over a pattern that duplicates an entity silently loses writes — `SET n.total =
n.total + m.amount` across a one-to-many join is the realistic version, and it
fails quietly with no error, which is the worst way for it to fail.
Two things I think this needs before anyone writes a patch:
1. **Confirmation against a real Neo4j instance** that `cnt` ends at 2. I am
confident in the openCypher reading, but a behavior change should not be
implemented off someone's recollection of the spec. If anyone has an instance
handy, please post the actual output.
2. **A scope decision**, since the same snapshot-staleness question applies
to `REMOVE`, and to `DELETE` of an entity already deleted on an earlier row.
Fixing `SET` alone would leave the family half-addressed.
On your dev@ note about the two PRs: #2517 has review comments from
@MuhammadTahaNaveed already. #2486 is still untouched — it is the larger of the
two and spans parser propagation, executor lookup and SET/REMOVE/DELETE
integration, so it needs a reviewer with a real block of time rather than a
drive-by pass. Worth stating explicitly that neither PR fixes the issue above,
so it does not get assumed closed when they land.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]