Skip to content

Commit 0e30566

Browse files
Fix stale TID errors in VLE path materialization (#2551)
Four problems with the TID-based VLE cache and its version counters, all from 798917c ("VLE cache + performance improvements"). 1. A path bound by MATCH could not be projected once the same statement deleted its endpoints (#2549): MATCH p = (n0)<-[:R*..2]-(n1) DETACH DELETE n0, n1 RETURN p ERROR: get_vertex_entry_properties: stale TID - ... That commit replaced the properties Datum in vertex_entry and edge_entry with a TID fetched lazily at projection. cypher_delete() advances es_snapshot->curcid past every delete, so a path's own endpoints fail the visibility test by the time they are read; before, properties were captured at cache build and a later delete could not affect them. Such a tuple is still physically present, the deleting transaction having not committed, so it is read anyway and the path reports the properties it was matched with. The relaxation is narrow: only a tuple deleted by our own transaction qualifies, and only while the row still carries the cached entity, so a recycled line pointer cannot be substituted. Any other unreachable TID still raises the error, keeping a real invalidation bug visible. Properties are detoasted under the buffer pin, and the buffer is now released on the failing path too, since heap_fetch is called with keep_buf, which leaves it pinned when only visibility fails. 2. VACUUM FULL and CLUSTER rewrite the heap, moving every cached TID, and announce themselves through no trigger and no version counter. A cached context then resolved stale TIDs against the new file, giving the stale TID error or "could not read block", which never reaches AGE's guard. Both are now intercepted in ag_ProcessUtility_hook as TRUNCATE already was, and the database-wide forms invalidate every tracked graph. Plain VACUUM and ANALYZE do not move tuples and are ignored. 3. NULL properties were reported as a stale TID. Label tables are created with properties NOT NULL, so a NULL means the table was altered out from under AGE; that is now said plainly rather than blamed on the cache. 4. Version counter slots were never released, making the table a tally of every graph ever mutated rather than of those that exist. A server cycling graphs filled it, then warned on every mutation and fell back to snapshot invalidation. drop_graph() now releases its slot and a freed slot is reused; a new occupant seeds its version above every value the table has issued, so a context cached for the previous occupant cannot compare equal. The cap moves 128 -> 256, about 4 KB of shared memory at 16 bytes per entry; lookups scan only the entries in use, so unused slots cost nothing. Both accessors now share one helper, and hardcoded column numbers give way to the Anum_ag_label_* constants. cypher_vle gains 14 cases: the reported query, a fan-out that fails if the result depends on which row is projected first, partial and edge-only deletes, the edge-list projection, an edge property constraint reaching the accessor during traversal, zero-length bounds, self-loops, labelled vertices, a multi-hop chain, a delete from an earlier statement that must not be resurrected, savepoint and transaction rollback, and out-of-line TOAST asserted set-identical to a live read. age_global_graph covers CLUSTER and all three VACUUM FULL spellings (named, parenthesised, database-wide), plus plain VACUUM, ANALYZE and FULL false which must not invalidate; NULL properties on a vertex and an edge; 260 create/drop cycles that must stay silent; a rolled-back drop; and a graph recreated under a dropped name. Verified on PostgreSQL 18.4 and 18.6: clean build, no warnings; installcheck 43/43 on 18.4 before and after, run twice; and 43/43 on 18.6 with --enable-cassert, reporting no assertion failure or resource leak. Fixes #2549 modified: regress/expected/age_global_graph.out modified: regress/expected/cypher_vle.out modified: regress/sql/age_global_graph.sql modified: regress/sql/cypher_vle.sql modified: src/backend/catalog/ag_catalog.c modified: src/backend/commands/graph_commands.c modified: src/backend/utils/adt/age_global_graph.c modified: src/include/utils/age_global_graph.h Co-authored-by: GitHub Copilot (Claude Opus 5) <noreply@github.com>
1 parent 4d9b250 commit 0e30566

8 files changed

Lines changed: 1877 additions & 64 deletions

File tree

‎regress/expected/age_global_graph.out‎

Lines changed: 384 additions & 0 deletions
Large diffs are not rendered by default.

‎regress/expected/cypher_vle.out‎

Lines changed: 603 additions & 0 deletions
Large diffs are not rendered by default.

‎regress/sql/age_global_graph.sql‎

Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,237 @@ $$) AS (name agtype);
354354
-- Cleanup
355355
SELECT * FROM drop_graph('vle_trigger_test', true);
356356

357+
-----------------------------------------------------------------------------------------------------------------------------
358+
--
359+
-- Heap rewrites must invalidate cached graph contexts
360+
--
361+
-- A rewrite moves every TID a cached context holds and announces itself through
362+
-- no trigger and no version counter. Left untracked, the context keeps
363+
-- resolving stale TIDs against the new file: it reads the wrong tuple, or a
364+
-- block past the end of a now shorter relation.
365+
--
366+
-- Every rewrite below is preceded by a read that caches the context and
367+
-- followed by a read in the SAME session; a fresh session would rebuild and
368+
-- prove nothing. The low ids are deleted first so the survivors sit at high
369+
-- offsets and a rewrite has to move them. Ground truth throughout is the
370+
-- 59 -> 60 path.
371+
--
372+
-----------------------------------------------------------------------------------------------------------------------------
373+
374+
SELECT create_graph('rewrite_inval');
375+
376+
SELECT * FROM cypher('rewrite_inval', $$
377+
UNWIND range(1, 60) AS i CREATE ({id: i})
378+
$$) AS (v agtype);
379+
380+
SELECT * FROM cypher('rewrite_inval', $$
381+
MATCH (a), (b) WHERE a.id = 59 AND b.id = 60 CREATE (a)-[:R]->(b)
382+
$$) AS (v agtype);
383+
384+
SELECT * FROM cypher('rewrite_inval', $$
385+
MATCH (n) WHERE n.id < 51 DETACH DELETE n
386+
$$) AS (v agtype);
387+
388+
SELECT * FROM cypher('rewrite_inval', $$
389+
MATCH p = (n0)-[:R*..2]->(n1) RETURN p
390+
$$) AS (p agtype);
391+
392+
-- CLUSTER rewrites in index order, moving the survivors to the front
393+
CLUSTER rewrite_inval."_ag_label_vertex" USING "_ag_label_vertex_pkey";
394+
395+
SELECT * FROM cypher('rewrite_inval', $$
396+
MATCH p = (n0)-[:R*..2]->(n1) RETURN p
397+
$$) AS (p agtype);
398+
399+
-- make dead space again so VACUUM FULL has something to compact away
400+
SELECT * FROM cypher('rewrite_inval', $$
401+
UNWIND range(100, 119) AS i CREATE ({id: i})
402+
$$) AS (v agtype);
403+
404+
SELECT * FROM cypher('rewrite_inval', $$
405+
MATCH (n) WHERE n.id >= 100 AND n.id < 110 DETACH DELETE n
406+
$$) AS (v agtype);
407+
408+
SELECT * FROM cypher('rewrite_inval', $$
409+
MATCH p = (n0)-[:R*..2]->(n1) RETURN p
410+
$$) AS (p agtype);
411+
412+
VACUUM FULL rewrite_inval."_ag_label_vertex";
413+
VACUUM FULL rewrite_inval."R";
414+
415+
SELECT * FROM cypher('rewrite_inval', $$
416+
MATCH p = (n0)-[:R*..2]->(n1) RETURN p
417+
$$) AS (p agtype);
418+
419+
-- plain VACUUM and ANALYZE leave tuples in place, so they are not rewrites and
420+
-- the cached context stays usable
421+
VACUUM rewrite_inval."_ag_label_vertex";
422+
ANALYZE rewrite_inval."_ag_label_vertex";
423+
424+
SELECT * FROM cypher('rewrite_inval', $$
425+
MATCH p = (n0)-[:R*..2]->(n1) RETURN p
426+
$$) AS (p agtype);
427+
428+
-- the parenthesised spelling is a rewrite too
429+
VACUUM (FULL) rewrite_inval."_ag_label_vertex";
430+
431+
SELECT * FROM cypher('rewrite_inval', $$
432+
MATCH p = (n0)-[:R*..2]->(n1) RETURN p
433+
$$) AS (p agtype);
434+
435+
-- and FULL turned off explicitly is not
436+
VACUUM (FULL false) rewrite_inval."_ag_label_vertex";
437+
438+
SELECT * FROM cypher('rewrite_inval', $$
439+
MATCH p = (n0)-[:R*..2]->(n1) RETURN p
440+
$$) AS (p agtype);
441+
442+
-- naming no relation rewrites the whole database, which has to invalidate
443+
-- every tracked graph rather than a named one
444+
VACUUM FULL;
445+
446+
SELECT * FROM cypher('rewrite_inval', $$
447+
MATCH p = (n0)-[:R*..2]->(n1) RETURN p
448+
$$) AS (p agtype);
449+
450+
SELECT drop_graph('rewrite_inval', true);
451+
452+
-----------------------------------------------------------------------------------------------------------------------------
453+
--
454+
-- NULL properties are a schema problem, not a stale cache
455+
--
456+
-- Label tables are created with properties NOT NULL, so a NULL there means the
457+
-- table was altered out from under AGE. Reporting that as a stale TID sends the
458+
-- reader after the wrong thing.
459+
--
460+
-----------------------------------------------------------------------------------------------------------------------------
461+
462+
SELECT create_graph('null_props');
463+
464+
SELECT * FROM cypher('null_props', $$
465+
CREATE ({id: 1})-[:R {w: 1}]->({id: 2})
466+
$$) AS (v agtype);
467+
468+
ALTER TABLE null_props."_ag_label_vertex"
469+
ALTER COLUMN properties DROP NOT NULL;
470+
UPDATE null_props."_ag_label_vertex" SET properties = NULL;
471+
472+
SELECT * FROM cypher('null_props', $$
473+
MATCH p = (n0)-[:R*..2]->(n1) RETURN p
474+
$$) AS (p agtype);
475+
476+
UPDATE null_props."_ag_label_vertex"
477+
SET properties = ag_catalog.agtype_build_map();
478+
479+
-- the edge constraint is inherited from _ag_label_edge, so it is dropped there
480+
ALTER TABLE null_props."_ag_label_edge"
481+
ALTER COLUMN properties DROP NOT NULL;
482+
UPDATE null_props."R" SET properties = NULL;
483+
484+
SELECT * FROM cypher('null_props', $$
485+
MATCH p = (n0)-[:R*..2]->(n1) RETURN p
486+
$$) AS (p agtype);
487+
488+
-- restored: projection works again
489+
UPDATE null_props."R" SET properties = ag_catalog.agtype_build_map();
490+
491+
SELECT * FROM cypher('null_props', $$
492+
MATCH p = (n0)-[:R*..2]->(n1) RETURN p
493+
$$) AS (p agtype);
494+
495+
SELECT drop_graph('null_props', true);
496+
497+
-----------------------------------------------------------------------------------------------------------------------------
498+
--
499+
-- A dropped graph releases its version counter slot
500+
--
501+
-- The counter table is a fixed size. While slots were never released it was a
502+
-- tally of every graph ever mutated, so a server that cycles graphs filled it,
503+
-- warned on every later mutation, and fell back to snapshot invalidation --
504+
-- correct, but far more eager. Cycling well past the cap must now be silent.
505+
--
506+
-- NOTICEs are muted so the loop does not bury the output; a WARNING from a full
507+
-- table would still come through and fail this test, which is the assertion.
508+
--
509+
-----------------------------------------------------------------------------------------------------------------------------
510+
511+
SET client_min_messages = warning;
512+
513+
DO $slots$
514+
DECLARE
515+
i int;
516+
BEGIN
517+
FOR i IN 1..260 LOOP
518+
PERFORM ag_catalog.create_graph('slot_churn_' || i);
519+
EXECUTE format('SELECT * FROM ag_catalog.cypher(%L, $c$ CREATE ({id: 1}) $c$) AS (v ag_catalog.agtype)',
520+
'slot_churn_' || i);
521+
PERFORM ag_catalog.drop_graph('slot_churn_' || i, true);
522+
END LOOP;
523+
END
524+
$slots$;
525+
526+
RESET client_min_messages;
527+
528+
SELECT count(*) AS churn_graphs_left
529+
FROM ag_catalog.ag_graph WHERE name::text LIKE 'slot_churn_%';
530+
531+
-- a drop that is rolled back must leave the graph tracked and usable
532+
SELECT create_graph('drop_rollback');
533+
534+
SELECT * FROM cypher('drop_rollback', $$
535+
CREATE ({id: 1})-[:R]->({id: 2})
536+
$$) AS (v agtype);
537+
538+
SELECT * FROM cypher('drop_rollback', $$
539+
MATCH p = (n0)-[:R*..2]->(n1) RETURN p
540+
$$) AS (p agtype);
541+
542+
BEGIN;
543+
SELECT drop_graph('drop_rollback', true);
544+
ROLLBACK;
545+
546+
SELECT * FROM cypher('drop_rollback', $$
547+
MATCH p = (n0)-[:R*..2]->(n1) RETURN p
548+
$$) AS (p agtype);
549+
550+
-- and a later mutation still invalidates correctly
551+
SELECT * FROM cypher('drop_rollback', $$ CREATE ({id: 3}) $$) AS (v agtype);
552+
553+
SELECT * FROM cypher('drop_rollback', $$
554+
MATCH p = (n0)-[:R*..2]->(n1) RETURN p
555+
$$) AS (p agtype);
556+
557+
SELECT drop_graph('drop_rollback', true);
558+
559+
-- a graph recreated under the same name must not inherit the old one's
560+
-- context. The freed slot may be handed straight back, so the new occupant's
561+
-- version has to be distinguishable from whatever was cached for the previous
562+
-- one.
563+
SELECT create_graph('slot_reuse');
564+
565+
SELECT * FROM cypher('slot_reuse', $$
566+
CREATE ({id: 111})-[:R]->({id: 222})
567+
$$) AS (v agtype);
568+
569+
SELECT * FROM cypher('slot_reuse', $$
570+
MATCH p = (a)-[:R*..2]->(b) RETURN p
571+
$$) AS (p agtype);
572+
573+
SELECT drop_graph('slot_reuse', true);
574+
575+
SELECT create_graph('slot_reuse');
576+
577+
SELECT * FROM cypher('slot_reuse', $$
578+
CREATE ({id: 333})-[:R]->({id: 444})
579+
$$) AS (v agtype);
580+
581+
-- 333 -> 444, never the dropped graph's 111 -> 222
582+
SELECT * FROM cypher('slot_reuse', $$
583+
MATCH p = (a)-[:R*..2]->(b) RETURN p
584+
$$) AS (p agtype);
585+
586+
SELECT drop_graph('slot_reuse', true);
587+
357588
-----------------------------------------------------------------------------------------------------------------------------
358589
--
359590
-- End of tests

0 commit comments

Comments
 (0)