gregfelice opened a new pull request, #2495:
URL: https://github.com/apache/age/pull/2495
Fixes #2493. Also fixes the visibility half of #2491, and the underlying
defect behind #2490.
## Problem
In a multi-part query, a clause that reads sees only the rows written by the
**first input row** of a preceding `CREATE` or `SET`. Everything written by the
remaining input rows is invisible for the rest of the statement.
```sql
SELECT * FROM cypher('g', $cypher$
UNWIND [1, 2, 3] AS i
CREATE (:v {id: i})
WITH count(*) AS ignored
MATCH (n:v)
RETURN count(n)
$cypher$) AS (visible agtype);
```
Returns `1`. All three vertices are persisted.
The visible unit is the command id, which covers one input row — so the
symptom depends on how the write is driven, not how much it writes:
| Setup | Written | Visible to the later `MATCH` | Persisted |
|---|---|---|---|
| `UNWIND range(1,2) AS i CREATE (:N {id:i})` | 2 | **1** | 2 |
| `UNWIND range(1,8) AS i CREATE (:N {id:i})` | 8 | **1** | 8 |
| 5 pre-existing, then create 3 in-statement | 3 | **6** (5 + 1) | 8 |
| `CREATE (:N),(:N),(:N)` — one input row | 3 | 3 ✅ | 3 |
`SET` behaves the same way: `MATCH (n:x) SET n.marked = true WITH count(*)
AS ig MATCH (m:x) WHERE m.marked = true RETURN count(m)` sees 1 of 3, though
all 3 updates persist.
## Root cause
Entities are written with the **global** command id — `insert_entity_tuple`
for `CREATE`, the `cid` in `update_entity_tuple` for `SET` — and
`CommandCounterIncrement()` advances that id once per input row.
The executor's snapshot does not follow it. `CommandCounterIncrement()`
updates the current and secondary snapshots, not the pushed one `es_snapshot`
points at, and `Increment_Estate_CommandId` bumps `curcid` only once, when the
clause begins. So `curcid` sits one step past the command id used by the first
input row, and only that row's tuples satisfy `cmin < curcid`.
This is already documented in-tree at
`src/backend/executor/cypher_utils.c:248-260`, where `entity_exists()` works
around it locally with `Max(saved_curcid, GetCurrentCommandId(false))`. Nothing
applied the same correction to ordinary `MATCH` scans.
## Fix
When a `CREATE` or `SET` clause reaches the end of its input, raise
`es_snapshot->curcid` to the global command id:
```c
estate->es_snapshot->curcid = Max(estate->es_snapshot->curcid,
GetCurrentCommandId(false));
```
Doing this at end of input rather than after each row is what preserves the
existing protection against a clause seeing its own writes: by that point the
subtree is exhausted, so raising `curcid` cannot feed a written row back into
the pattern that wrote it. `Max()` because `Increment_Estate_CommandId` can
push `curcid` above the global command id, and lowering it would hide tuples
that are already visible.
`REMOVE` is covered by the `SET` path it shares. `DELETE` already
synchronizes `curcid` explicitly (`cypher_delete.c:356`, `:482`) and was
unaffected; `MERGE` behaved correctly in the same probes.
## Testing
Full suite green — **42/42**, PostgreSQL 18.4.
Behaviour changes, all verified:
| Case | Before | After |
|---|---|---|
| #2493 as filed | `rows=1, bound=1` | **`3, 3`** |
| #2490 driven by two input rows | 2 | **4** |
| Create k vertices, count visible (k=1,2,3,8) | 1, 1, 1, 1 | **1, 2, 3, 8**
|
| 5 pre-existing + 3 new | 6 | **8** |
| #2491 middle stage (8 vertices → edges) | 0 edges | **56 edges** |
| `SET` 3 vertices, later `MATCH` on the new property | 1 | **3** |
| `REMOVE` from 3, later `MATCH` counts still-marked | 2 | **0** |
| `MATCH (n:N) CREATE (:N)` — must not self-feed | 6 | **6** (unchanged) |
New regression coverage in `cypher_create` and `cypher_set` for visibility
of every written row, for writes from an earlier clause driving a later one,
and for a write clause still not seeing its own writes. Each new assertion was
confirmed to fail without this change — `visible` reports 1 instead of 3,
`edges` 0 instead of 6, `still_marked` 2 instead of 0.
I also ran the suite on unmodified `master` and with this change through an
identical harness: the outputs are byte-identical apart from the new
assertions, so nothing else moved.
## Out of scope
#2491 additionally hits a separate defect, filed as #2494: the planner can
place the DML `CustomScan` on a side of a join the executor never pulls, so the
writes are skipped entirely and nothing persists. That is untouched here, and
#2491 will still fail until it is fixed.
--
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]
