Writes are silently discarded when a later clause reads a label whose table is empty (DML CustomScan never executed)

[gregfelice]

Describe the bug

In a multi-part query, all writes can be silently discarded — not merely invisible to later clauses, but never executed at all. The statement succeeds, returns a plausible result, and leaves the database unchanged.

AGE's DML executes inside a CustomScan buried in the plan tree rather than as a top-level ModifyTable. The planner is therefore free to place that subtree on either side of a join, and several executor nodes are entitled to skip an input entirely when the other input turns out to be empty. When the skipped input is the one containing the Cypher Create node, the writes never happen.

The empty input is typically the scan of the very label the query is writing to — which is empty precisely because the node that would populate it is the side being skipped. Chicken-and-egg, and completely silent.

Split out of #2491, which hits this as well as the separate visibility defect tracked in #2493. This issue is only the discarded-writes half.

How are you accessing AGE (Command line, driver, etc.)?

Command line, psql.

What data setup do we need to do?

None beyond an empty graph.

LOAD 'age';
SET search_path = ag_catalog, public;
SELECT create_graph('g');

What is the necessary configuration info needed?

None. Default configuration, no extra extensions.

What is the command that caused the error?

SELECT * FROM cypher('g', $cypher$
  UNWIND [1,2] AS i CREATE (:N)-[:R {id:i}]->(:N)
  WITH count(*) AS ig
  MATCH ()-[r:R]->() RETURN count(r)
$cypher$) AS (v agtype);

 v
---
 0
(1 row)

No error is raised. Afterwards the graph is empty — zero nodes and zero edges:

SELECT * FROM cypher('g', $cypher$ MATCH (n) RETURN count(n) $cypher$) AS (c agtype);
 c
---
 0

EXPLAIN (ANALYZE) shows the writes were never executed:

 Aggregate (actual rows=1.00 loops=1)
   ->  Merge Join (actual rows=0.00 loops=1)
         ->  Merge Append (actual rows=0.00 loops=1)
               ->  Index Only Scan using _ag_label_vertex_pkey ... (actual rows=0.00 loops=1)
               ->  Index Only Scan using "N_pkey" on "N" ...      (actual rows=0.00 loops=1)
         ->  Materialize (never executed)
               ->  Nested Loop (never executed)
                     ->  Index Scan using "R_end_id_idx" on "R" r (never executed)
                     ->  ...
                           ->  Custom Scan (Cypher Create) (never executed)

The merge join exhausts its left input, concludes the join can produce nothing, and never pulls the right input — which is where the CREATE lives.

It is not specific to merge joins. The query as filed in #2491 reaches the same outcome through a hash join, where an empty build side lets PostgreSQL skip reading the outer relation entirely:

Hash Join (actual rows=0.00 loops=1)
  ->  Hash Join (never executed)
        ...
        ->  Custom Scan (Cypher Create) (never executed)   <- the nodes
              ...
              ->  Custom Scan (Cypher Create) (never executed)   <- the edges
  ->  Hash (actual rows=0.00 loops=1)
        ->  Seq Scan on "N" _age_default_alias_6 (actual rows=0.00 loops=1)

Two different join strategies, same failure — so this is not one unlucky plan shape.

What decides whether it fires

Plan shape, not input-row count. Measured on master @ 8014174:

Variant	Writes persisted?
Empty graph; later MATCH scans the label just written; 2 input rows	❌ discarded
Same, 1 input row (CREATE (:N)-[:R {id:1}]->(:N))	❌ discarded
Same, non-aggregating WITH i AS keep instead of WITH count(*)	❌ discarded
Same query, but the graph already contains one :R edge	✅ persisted
#2491 as filed (three stages, fresh graph)	❌ discarded
Later MATCH scans a different empty label than the one written	✅ persisted
#2490 as filed (single CREATE, later MATCH with WHERE r.id = item.id)	✅ persisted

The failing cases share one property: the scan of the label being written back has no visible rows at execution time. Pre-existing rows in that table are enough to make the same query behave.

Expected behavior

A write clause must execute for every input row it receives, regardless of the join strategy chosen for clauses that read afterwards. Query results may be debatable under AGE's current visibility rules (see #2493), but a CREATE that the user asked for must never be silently skipped.

Environment (please complete the following information):

• Version: master @ 8014174 (1.8.0)
• PostgreSQL: 18.4 (Debian), built from source
• OS: Debian, Linux 6.12

Also reproduces on 1.7.0 — the original report in #2491 was against the 1.7.0 Docker image.

Additional context

Severity is worth noting: #2493 produces wrong answers, which is bad but visible. This one silently drops committed-looking work with no error, so an application has no way to detect that its writes did not happen short of reading back.

Fixing it likely means preventing the planner from placing the DML subtree where the executor may skip it, rather than patching any individual join node — the two reproductions above already show two independent paths to the same outcome.

Related: #2491 (original report, also covers the visibility defect), #2493 (visibility defect), #2490.

[gregfelice]

I have a working fix for this, but it carries a semantics change that I would rather have agreement on before opening a PR — particularly with 1.8.0 mid-release.

Environment: master @ 8014174, PostgreSQL 18.4, built from source.

Proposed fix

PostgreSQL already solves exactly this problem for data-modifying CTEs. ExecPostprocessPlan() runs everything on EState.es_auxmodifytables to completion once the main plan is done, specifically so that writes do not depend on the main query pulling from them. It iterates that list as plain PlanState * and calls ExecProcNode, so AGE's DML CustomScan can join it directly:

/* in begin_cypher_create(), after Increment_Estate_CommandId() */
if (!(eflags & EXEC_FLAG_EXPLAIN_ONLY))
{
    estate->es_auxmodifytables = lappend(estate->es_auxmodifytables, node);
}

Re-running an already-exhausted node is a no-op — its subtree returns NULL immediately. EXPLAIN without ANALYZE never reaches ExecutorFinish, hence the guard.

Result:

Query	Writes before	Writes after
Minimal repro in this issue	0 nodes, 0 edges	4 nodes, 2 edges
#2491 as filed	0 nodes, 0 edges	8 nodes, 56 edges

Full regression suite: 42/42.

What it does not fix

The writes now happen, but they happen at ExecutorFinish, after the reading clause has already run. So the query results are unchanged: this issue's repro still returns 0, and #2491 still returns "FAIL". Only the silent data loss is fixed.

Making the results correct means getting the planner to order the write before the read — the DML CustomScan would have to be somewhere the executor cannot skip and cannot schedule after the reading scan. That is a much larger change and I have not attempted it.

So this is a fix for the title of the issue, not for the full openCypher semantics behind it.

The semantics change to agree on

Draining to completion also changes what LIMIT does over a write clause. Against 5 :src vertices:

MATCH (n:src) CREATE (:made {from: n.id}) RETURN n LIMIT 1

	:made vertices created
master today	1
with this change	5

Measured both ways on the same build, not inferred.

I think 5 is the correct answer — it is what Neo4j does (the write clause processes every incoming row; LIMIT bounds the returned rows, not the writes), and it matches PostgreSQL's rule that data-modifying CTEs always execute to completion regardless of what the primary query reads. By that reading, today's behaviour is the same defect as this issue wearing a different hat: a write silently skipped because the executor stopped pulling.

But it is a user-visible change, no test in the suite asserts either behaviour, and it would land during a release. Hence asking rather than filing.

Scope

The same hazard applies to SET, DELETE, and MERGE — each is a CustomScan the planner can place on a skippable side of a join. The fix is the same one-liner in each begin_* function. I held off pending direction on the above.

@jrgemignani @MuhammadTahaNaveed — three questions:

1. Is the LIMIT-over-write change acceptable, or should writes stay bounded by what the plan pulls?
2. Land this for 1.8.0, or hold for after the release?
3. All four DML clauses in one PR, or CREATE first?

Happy to open the PR immediately once there is a direction.

[gregfelice]

Correction to my previous comment, and a scope finding.

I said there that the same hazard applies to SET, DELETE and MERGE and that "the fix is the same one-liner in each begin_* function". That is wrong. I have now tried it, and registering the other three the same way takes the regression suite from 42/42 to 14 failures, in two distinct modes.

Results

Clause	Registered in es_auxmodifytables	Outcome
CREATE	safe	42/42
MERGE	duplicates data	cypher_merge, cypher_vle fail — entities created twice (2 rows where 1 expected, 6 where 3)
SET + DELETE	segfault	backend killed by signal 11, core dumped; 12 further tests fall over behind it

The crash reproduces on a statement carrying both a DELETE and a SET:

MATCH (n)-[e]->(m) DETACH DELETE n SET e.i = 1 RETURN e

LOG:  client backend (PID 3650008) was terminated by signal 11: Segmentation fault

That query is already in regress/sql/cypher_delete.sql, so the suite catches it immediately.

Why

My earlier claim that re-running an exhausted node is a no-op holds for CREATE only — I had verified it there and generalised it without checking, which was the error.

ExecPostprocessPlan() re-runs everything on the list unconditionally, so registration is safe only for a node that is genuinely idempotent on re-entry. exec_cypher_merge re-processes and creates duplicates; the SET/DELETE paths dereference state that is no longer valid by then.

What a real extension needs

A per-node "finished" flag: set on every path where exec returns NULL, checked on entry so the drain is a true no-op for an already-exhausted node. That means a struct field plus guards on every exit path — five in exec_cypher_merge alone — across four executors, each needing separate verification. It also opens a path that has never run before: a SET or DELETE node that was skipped entirely and is then drained at ExecutorFinish.

That is a much bigger change than the registration itself, and I would not want it rushed alongside a release.

Revised proposal

Ship CREATE only, as a bounded fix for the reported case, and treat the other three as a separate piece of work behind the flag. CREATE is where the reported symptom occurs (this issue and #2491), and it is the clause most exposed in practice, because it is the one that populates a previously empty label table — which is the condition that lets the executor skip the write in the first place.

For whoever picks up the follow-up, the LIMIT-over-write numbers I measured for each clause, 5 matched rows and LIMIT 1:

Clause	master today	with registration
CREATE	1 created	5 created
SET	1 updated	5 updated
DELETE	1 deleted	all 5 deleted
MERGE	5	5 (already eager)

DELETE going from "deletes one" to "deletes all matched" deserves an explicit decision of its own, separate from the CREATE question I asked above.

The CREATE-only branch is ready to push whenever there is a direction on that question.