Re: [I] Clarify SET semantics when one entity appears in multiple result rows [age]

["gregfelice (via GitHub)" <gi...@apache.org>]

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: dev-unsubscribe@age.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org