Copilot commented on code in PR #2376:
URL: https://github.com/apache/age/pull/2376#discussion_r3060722479
##########
src/backend/utils/adt/age_global_graph.c:
##########
@@ -1450,3 +1595,272 @@ Datum age_graph_stats(PG_FUNCTION_ARGS)
PG_RETURN_POINTER(agtype_value_to_agtype(result.res));
}
+
+/*
+ * ============================================================================
+ * Graph Version Counter Implementation
+ *
+ * Provides per-graph monotonic version counters in shared memory for
+ * cross-backend VLE cache invalidation. Three modes are supported:
+ *
+ * DSM (PG 17+): Uses GetNamedDSMSegment — works without shared_preload_libs
+ * SHMEM (PG <17): Uses shmem_request/startup hooks — needs shared_preload_libs
+ * SNAPSHOT: Falls back to original snapshot-based invalidation
+ * ============================================================================
+ */
+
+#if PG_VERSION_NUM >= 170000
+/*
+ * DSM path: GetNamedDSMSegment init callback.
+ * Called once when the DSM segment is first created.
+ */
+static void age_dsm_init_callback(void *ptr)
+{
+ GraphVersionState *state = (GraphVersionState *) ptr;
+
+ LWLockInitialize(&state->lock,
+ LWLockNewTrancheId());
+ LWLockRegisterTranche(state->lock.tranche, "age_graph_version");
+ state->num_entries = 0;
+ memset(state->entries, 0, sizeof(state->entries));
+}
+
+/*
+ * Get the shared GraphVersionState via DSM registry.
+ * The segment is created on first access and persists until server shutdown.
+ */
+static GraphVersionState *get_version_state_dsm(void)
+{
+ bool found;
+
+ return (GraphVersionState *)
+ GetNamedDSMSegment("age_graph_versions",
+ sizeof(GraphVersionState),
+ age_dsm_init_callback,
+ &found);
+}
+#endif /* PG_VERSION_NUM >= 170000 */
+
+/*
+ * SHMEM path: request and startup hooks for PG < 17.
+ * These are registered in _PG_init when shared_preload_libraries is used.
+ */
+void age_graph_version_shmem_request(void)
+{
+ RequestAddinShmemSpace(MAXALIGN(sizeof(GraphVersionState)));
+ RequestNamedLWLockTranche("age_graph_version", 1);
+}
+
+void age_graph_version_shmem_startup(void)
+{
+ bool found;
+
+ LWLockAcquire(AddinShmemInitLock, LW_EXCLUSIVE);
+
+ shmem_version_state =
+ (GraphVersionState *) ShmemInitStruct("AGE Graph Version State",
+ sizeof(GraphVersionState),
+ &found);
+ if (!found)
+ {
+ LWLockInitialize(&shmem_version_state->lock,
+ LWLockNewTrancheId());
+ LWLockRegisterTranche(shmem_version_state->lock.tranche,
+ "age_graph_version");
+ shmem_version_state->num_entries = 0;
+ memset(shmem_version_state->entries, 0,
+ sizeof(shmem_version_state->entries));
+ }
+
+ LWLockRelease(AddinShmemInitLock);
+}
+
+/*
+ * Detect which version mode to use. Called once per backend on first access.
+ * Emits a DEBUG1 log message indicating the chosen mode.
+ */
+static void detect_version_mode(void)
+{
+#if PG_VERSION_NUM >= 170000
+ version_mode = VERSION_MODE_DSM;
+ elog(DEBUG1, "AGE: VLE cache using DSM version counter");
+#else
+ if (shmem_version_state != NULL)
+ {
+ version_mode = VERSION_MODE_SHMEM;
+ elog(DEBUG1, "AGE: VLE cache using SHMEM version counter");
+ }
+ else
+ {
+ version_mode = VERSION_MODE_SNAPSHOT;
+ elog(DEBUG1, "AGE: VLE cache using snapshot-based invalidation "
+ "(add AGE to shared_preload_libraries for better caching)");
+ }
+#endif
+}
+
+/*
+ * Get a pointer to the GraphVersionState, regardless of mode.
+ * Returns NULL only in SNAPSHOT mode (no shared memory available).
+ */
+static GraphVersionState *get_version_state(void)
+{
+ if (version_mode == VERSION_MODE_UNKNOWN)
+ detect_version_mode();
+
+#if PG_VERSION_NUM >= 170000
+ if (version_mode == VERSION_MODE_DSM)
+ return get_version_state_dsm();
+#endif
+
+ if (version_mode == VERSION_MODE_SHMEM)
+ return shmem_version_state;
+
+ return NULL;
+}
+
+/*
+ * Get the current version counter for a graph.
+ * Returns 0 if the graph has never been tracked or if shared memory
+ * is not available. Lock-free read via pg_atomic_read_u64.
+ */
+uint64 get_graph_version(Oid graph_oid)
+{
+ GraphVersionState *state = get_version_state();
+ int i;
+
+ if (state == NULL)
+ return 0;
+
+ /* lock-free scan of the array */
+ for (i = 0; i < state->num_entries; i++)
+ {
+ if (state->entries[i].graph_oid == graph_oid)
+ return pg_atomic_read_u64(&state->entries[i].version);
+ }
+
+ return 0;
+}
+
+/*
+ * Increment the version counter for a graph.
+ * Called after any graph mutation (Cypher or SQL trigger).
+ * Lock-free for existing entries; acquires LWLock only to allocate new slots.
+ */
+void increment_graph_version(Oid graph_oid)
+{
+ GraphVersionState *state = get_version_state();
+ int i;
+
+ if (state == NULL)
+ return;
+
+ /* try to find existing entry (lock-free) */
+ for (i = 0; i < state->num_entries; i++)
+ {
+ if (state->entries[i].graph_oid == graph_oid)
+ {
+ pg_atomic_fetch_add_u64(&state->entries[i].version, 1);
+ return;
+ }
+ }
+
+ /* new graph — need lock to allocate slot */
+ LWLockAcquire(&state->lock, LW_EXCLUSIVE);
+
+ /* re-check after acquiring lock (another backend may have added it) */
+ for (i = 0; i < state->num_entries; i++)
+ {
+ if (state->entries[i].graph_oid == graph_oid)
+ {
+ LWLockRelease(&state->lock);
+ pg_atomic_fetch_add_u64(&state->entries[i].version, 1);
+ return;
+ }
+ }
+
+ /* add new entry */
+ if (state->num_entries < AGE_MAX_GRAPHS)
+ {
+ int idx = state->num_entries;
+
+ state->entries[idx].graph_oid = graph_oid;
+ pg_atomic_init_u64(&state->entries[idx].version, 1);
+ state->num_entries++;
+ }
+ else
+ {
+ elog(WARNING, "AGE: graph version counter table full (%d graphs)",
+ AGE_MAX_GRAPHS);
+ }
+
+ LWLockRelease(&state->lock);
+}
+
+/*
+ * Helper function to look up the graph OID for a given label table OID.
+ * Scans ag_label catalog for a matching relation column.
+ * Returns InvalidOid if the table is not a graph label table.
+ */
+Oid get_graph_oid_for_table(Oid table_oid)
+{
+ ScanKeyData scan_key;
+ Relation ag_label_rel;
+ TableScanDesc scan;
+ HeapTuple tuple;
+ Oid graph_oid = InvalidOid;
+
+ ag_label_rel = table_open(ag_label_relation_id(), AccessShareLock);
+
+ ScanKeyInit(&scan_key, Anum_ag_label_relation,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(table_oid));
+
+ scan = table_beginscan_catalog(ag_label_rel, 1, &scan_key);
+
+ if ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
+ {
+ bool isnull;
+ Datum d = heap_getattr(tuple, Anum_ag_label_graph,
+ RelationGetDescr(ag_label_rel), &isnull);
+ if (!isnull)
+ graph_oid = DatumGetObjectId(d);
+ }
+
+ table_endscan(scan);
+ table_close(ag_label_rel, AccessShareLock);
+
+ return graph_oid;
+}
+
+/*
+ * SQL-callable trigger function for VLE cache invalidation.
+ * Installed on graph label tables (AFTER INSERT/UPDATE/DELETE FOR EACH
STATEMENT).
+ * Looks up which graph the triggering table belongs to and increments
+ * that graph's version counter.
+ */
+PG_FUNCTION_INFO_V1(age_invalidate_graph_cache);
+
+Datum age_invalidate_graph_cache(PG_FUNCTION_ARGS)
+{
+ TriggerData *trigdata;
+ Oid table_oid;
+ Oid graph_oid;
+
+ /* verify called as trigger */
+ if (!CALLED_AS_TRIGGER(fcinfo))
+ ereport(ERROR,
+ (errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
+ errmsg("age_invalidate_graph_cache: not called as trigger")));
+
+ trigdata = (TriggerData *) fcinfo->context;
+ table_oid = RelationGetRelid(trigdata->tg_relation);
+
+ /* look up which graph this label table belongs to */
+ graph_oid = get_graph_oid_for_table(table_oid);
+
+ if (OidIsValid(graph_oid))
+ increment_graph_version(graph_oid);
+
+ return PointerGetDatum(NULL);
Review Comment:
In the trigger function `age_invalidate_graph_cache`, prefer
`PG_RETURN_NULL()` for the return value (standard trigger-function convention)
instead of `return PointerGetDatum(NULL);` for clarity and consistency with
other PostgreSQL trigger implementations.
```suggestion
PG_RETURN_NULL();
```
##########
src/backend/utils/adt/age_global_graph.c:
##########
@@ -21,13 +21,23 @@
#include "access/heapam.h"
#include "catalog/namespace.h"
+#include "commands/trigger.h"
#include "common/hashfn.h"
#include "commands/label_commands.h"
+#include "port/atomics.h"
+#include "storage/lwlock.h"
#include "utils/datum.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
#include "utils/snapmgr.h"
+#if PG_VERSION_NUM >= 170000
+#include "storage/dsm_registry.h"
+#else
+#include "storage/ipc.h"
+#include "storage/shmem.h"
+#endif
Review Comment:
`age_graph_version_shmem_request()` / `age_graph_version_shmem_startup()`
are compiled unconditionally, but the file only includes `storage/ipc.h` /
`storage/shmem.h` in the `PG_VERSION_NUM < 170000` branch. On PG 17+ this will
likely fail to compile due to missing declarations for
`RequestAddinShmemSpace`, `ShmemInitStruct`, and `AddinShmemInitLock`. Wrap the
SHMEM-only functions in `#if PG_VERSION_NUM < 170000` (or include the required
headers unconditionally) so PG 17 builds don’t break.
##########
regress/expected/age_global_graph.out:
##########
@@ -413,6 +413,156 @@ NOTICE: graph "ag_graph_3" has been dropped
(1 row)
+-----------------------------------------------------------------------------------------------------------------------------
+--
+-- VLE cache invalidation tests
+--
+-- These tests verify that the graph version counter properly invalidates
+-- the VLE hash table cache when the graph is mutated, and that thin
+-- entry lazy property fetch returns correct data.
+--
+-- Setup: create a graph with a chain a->b->c->d
+SELECT * FROM create_graph('vle_cache_test');
+NOTICE: graph "vle_cache_test" has been created
+ create_graph
+--------------
+
+(1 row)
+
+SELECT * FROM cypher('vle_cache_test', $$
+ CREATE (a:Node {name: 'a'})-[:Edge]->(b:Node {name: 'b'})-[:Edge]->(c:Node
{name: 'c'})-[:Edge]->(d:Node {name: 'd'})
+$$) AS (v agtype);
+ v
+---
+(0 rows)
+
+-- VLE query: find all paths from a's neighbors (should find b, b->c, b->c->d)
+SELECT * FROM cypher('vle_cache_test', $$
+ MATCH (a:Node {name: 'a'})-[:Edge*1..3]->(n:Node)
+ RETURN n.name
+ ORDER BY n.name
+$$) AS (name agtype);
+ name
+------
+ "b"
+ "c"
+ "d"
+(3 rows)
+
+-- Now add a new node e connected to d. This should invalidate the cache.
+SELECT * FROM cypher('vle_cache_test', $$
+ MATCH (d:Node {name: 'd'})
+ CREATE (d)-[:Edge]->(:Node {name: 'e'})
+$$) AS (v agtype);
+ v
+---
+(0 rows)
+
+-- VLE query again: should now also find e via a->b->c->d->e (4 hops won't
reach,
+-- but d->e is 1 hop from d, and a->b->c->d->e would be 4 hops from a).
+-- Increase range to *1..4 to include e
+SELECT * FROM cypher('vle_cache_test', $$
+ MATCH (a:Node {name: 'a'})-[:Edge*1..4]->(n:Node)
+ RETURN n.name
+ ORDER BY n.name
+$$) AS (name agtype);
+ name
+------
+ "b"
+ "c"
+ "d"
+ "e"
+(4 rows)
+
+-- Test cache invalidation on DELETE: remove node c and its edges
+SELECT * FROM cypher('vle_cache_test', $$
+ MATCH (c:Node {name: 'c'})
+ DETACH DELETE c
+$$) AS (v agtype);
+ v
+---
+(0 rows)
+
+-- VLE query: should only find b now (c is gone, so b->c path is broken)
+SELECT * FROM cypher('vle_cache_test', $$
+ MATCH (a:Node {name: 'a'})-[:Edge*1..4]->(n:Node)
+ RETURN n.name
+ ORDER BY n.name
+$$) AS (name agtype);
+ name
+------
+ "b"
+(1 row)
+
+-- Test cache invalidation on SET: change b's name property
+SELECT * FROM cypher('vle_cache_test', $$
+ MATCH (b:Node {name: 'b'})
+ SET b.name = 'b_modified'
+ RETURN b.name
+$$) AS (name agtype);
+ name
+--------------
+ "b_modified"
+(1 row)
+
+-- VLE query: verify the updated property is returned via lazy fetch
+SELECT * FROM cypher('vle_cache_test', $$
+ MATCH (a:Node {name: 'a'})-[:Edge*1..4]->(n:Node)
+ RETURN n.name
+ ORDER BY n.name
+$$) AS (name agtype);
+ name
+--------------
+ "b_modified"
+(1 row)
+
+-- Test VLE with edge properties (exercises thin entry edge property fetch)
+SELECT * FROM drop_graph('vle_cache_test', true);
+NOTICE: drop cascades to 4 other objects
+DETAIL: drop cascades to table vle_cache_test._ag_label_vertex
+drop cascades to table vle_cache_test._ag_label_edge
+drop cascades to table vle_cache_test."Node"
+drop cascades to table vle_cache_test."Edge"
+NOTICE: graph "vle_cache_test" has been dropped
+ drop_graph
+------------
+
+(1 row)
+
+SELECT * FROM create_graph('vle_cache_test2');
+NOTICE: graph "vle_cache_test2" has been created
+ create_graph
+--------------
+
+(1 row)
+
+SELECT * FROM cypher('vle_cache_test2', $$
+ CREATE (a:N {name: 'a'})-[:E {weight: 1}]->(b:N {name: 'b'})-[:E {weight:
2}]->(c:N {name: 'c'})
+$$) AS (v agtype);
+ v
+---
+(0 rows)
+
+-- VLE returning edge list to verify edge properties are fetched correctly
+SELECT * FROM cypher('vle_cache_test2', $$
+ MATCH (a:N {name: 'a'})-[e:E *1..2]->(n:N)
+ RETURN n.name, length(e)
+ ORDER BY n.name
+$$) AS (name agtype, pathlen agtype);
+ERROR: length() argument must resolve to a scalar
Review Comment:
The expected output for the new “edge properties” regression test is an
error from `length(e)` on a variable-length relationship list. If the intent is
to verify thin-entry edge property fetching, this should be changed to an
assertion on successful output (and the SQL adjusted accordingly), otherwise
the test is only checking that an unrelated type error is raised.
```suggestion
RETURN n.name, [edge IN e | edge.weight]
ORDER BY n.name
$$) AS (name agtype, weights agtype);
name | weights
------+---------
"b" | [1]
"c" | [1, 2]
(2 rows)
```
##########
regress/sql/age_global_graph.sql:
##########
@@ -146,6 +146,90 @@ RESET client_min_messages;
SELECT * FROM drop_graph('ag_graph_1', true);
SELECT * FROM drop_graph('ag_graph_2', true);
SELECT * FROM drop_graph('ag_graph_3', true);
+
+-----------------------------------------------------------------------------------------------------------------------------
+--
+-- VLE cache invalidation tests
+--
+-- These tests verify that the graph version counter properly invalidates
+-- the VLE hash table cache when the graph is mutated, and that thin
+-- entry lazy property fetch returns correct data.
+--
+
+-- Setup: create a graph with a chain a->b->c->d
+SELECT * FROM create_graph('vle_cache_test');
+
+SELECT * FROM cypher('vle_cache_test', $$
+ CREATE (a:Node {name: 'a'})-[:Edge]->(b:Node {name: 'b'})-[:Edge]->(c:Node
{name: 'c'})-[:Edge]->(d:Node {name: 'd'})
+$$) AS (v agtype);
+
+-- VLE query: find all paths from a's neighbors (should find b, b->c, b->c->d)
+SELECT * FROM cypher('vle_cache_test', $$
+ MATCH (a:Node {name: 'a'})-[:Edge*1..3]->(n:Node)
+ RETURN n.name
+ ORDER BY n.name
+$$) AS (name agtype);
+
+-- Now add a new node e connected to d. This should invalidate the cache.
+SELECT * FROM cypher('vle_cache_test', $$
+ MATCH (d:Node {name: 'd'})
+ CREATE (d)-[:Edge]->(:Node {name: 'e'})
+$$) AS (v agtype);
+
+-- VLE query again: should now also find e via a->b->c->d->e (4 hops won't
reach,
+-- but d->e is 1 hop from d, and a->b->c->d->e would be 4 hops from a).
+-- Increase range to *1..4 to include e
+SELECT * FROM cypher('vle_cache_test', $$
+ MATCH (a:Node {name: 'a'})-[:Edge*1..4]->(n:Node)
+ RETURN n.name
+ ORDER BY n.name
+$$) AS (name agtype);
+
+-- Test cache invalidation on DELETE: remove node c and its edges
+SELECT * FROM cypher('vle_cache_test', $$
+ MATCH (c:Node {name: 'c'})
+ DETACH DELETE c
+$$) AS (v agtype);
+
+-- VLE query: should only find b now (c is gone, so b->c path is broken)
+SELECT * FROM cypher('vle_cache_test', $$
+ MATCH (a:Node {name: 'a'})-[:Edge*1..4]->(n:Node)
+ RETURN n.name
+ ORDER BY n.name
+$$) AS (name agtype);
+
+-- Test cache invalidation on SET: change b's name property
+SELECT * FROM cypher('vle_cache_test', $$
+ MATCH (b:Node {name: 'b'})
+ SET b.name = 'b_modified'
+ RETURN b.name
+$$) AS (name agtype);
+
+-- VLE query: verify the updated property is returned via lazy fetch
+SELECT * FROM cypher('vle_cache_test', $$
+ MATCH (a:Node {name: 'a'})-[:Edge*1..4]->(n:Node)
+ RETURN n.name
+ ORDER BY n.name
+$$) AS (name agtype);
+
+-- Test VLE with edge properties (exercises thin entry edge property fetch)
+SELECT * FROM drop_graph('vle_cache_test', true);
+SELECT * FROM create_graph('vle_cache_test2');
+
+SELECT * FROM cypher('vle_cache_test2', $$
+ CREATE (a:N {name: 'a'})-[:E {weight: 1}]->(b:N {name: 'b'})-[:E {weight:
2}]->(c:N {name: 'c'})
+$$) AS (v agtype);
+
+-- VLE returning edge list to verify edge properties are fetched correctly
+SELECT * FROM cypher('vle_cache_test2', $$
+ MATCH (a:N {name: 'a'})-[e:E *1..2]->(n:N)
+ RETURN n.name, length(e)
+ ORDER BY n.name
+$$) AS (name agtype, pathlen agtype);
Review Comment:
The new “edge properties” regression test currently executes `RETURN n.name,
length(e)` where `e` is a variable-length relationship binding (`[e:E *1..2]`).
The expected output shows this errors (`length() argument must resolve to a
scalar`), so the test doesn’t actually validate thin-entry edge property
fetching. Update the query to return something that forces edge property
materialization and succeeds (e.g., bind a path `p=` and inspect
`relationships(p)[0]` / `properties(e[0])`), then assert the expected values.
```suggestion
-- VLE returning edge properties to verify thin-entry edge property fetch
works correctly
SELECT * FROM cypher('vle_cache_test2', $$
MATCH p=(a:N {name: 'a'})-[e:E *1..2]->(n:N)
RETURN n.name,
properties(relationships(p)[0]),
CASE WHEN length(p) > 1 THEN properties(relationships(p)[1]) ELSE
NULL END
ORDER BY n.name
$$) AS (name agtype, edge1_props agtype, edge2_props agtype);
```
##########
src/backend/commands/label_commands.c:
##########
@@ -432,6 +434,64 @@ static void create_table_for_label(char *graph_name, char
*label_name,
create_index_on_column(schema_name, rel_name, "start_id", false);
create_index_on_column(schema_name, rel_name, "end_id", false);
}
+
+ /*
+ * Install a cache invalidation trigger on the new label table, if the
+ * trigger function exists. The function is registered in the extension
+ * SQL (age_main.sql). It may not exist if running against an older
+ * version of the extension SQL that hasn't been upgraded yet.
+ *
+ * When installed, the trigger fires AFTER INSERT/UPDATE/DELETE/TRUNCATE
+ * (FOR EACH STATEMENT) and increments the graph's version counter so
+ * VLE caches are properly invalidated when the table is modified via SQL.
+ */
+ {
+ Oid func_oid;
+
+ /* check if the trigger function is registered in the catalog */
+ func_oid = LookupFuncName(
+ list_make2(makeString("ag_catalog"),
+ makeString("age_invalidate_graph_cache")),
+ 0, NULL, true);
+
+ if (OidIsValid(func_oid))
+ {
+ CreateTrigStmt *trigger_stmt = makeNode(CreateTrigStmt);
+ PlannedStmt *trigger_wrapper;
+
+ trigger_stmt->replace = false;
+ trigger_stmt->isconstraint = false;
+ trigger_stmt->trigname = "_age_cache_invalidate";
+ trigger_stmt->relation = makeRangeVar(graph_name, rel_name, -1);
Review Comment:
Trigger creation uses `makeRangeVar(graph_name, rel_name, ...)`, but the
table was created with `schema_name` (`makeRangeVar(schema_name, rel_name,
...)`). If `graph_name` differs from the actual namespace name (case-folding,
quoting, etc.), the generated `CREATE TRIGGER` may target a non-existent
relation. Use the same `schema_name` used for table creation when constructing
the `RangeVar` for the trigger.
```suggestion
trigger_stmt->relation = makeRangeVar(schema_name, rel_name, -1);
```
##########
src/backend/utils/adt/age_global_graph.c:
##########
@@ -1450,3 +1595,272 @@ Datum age_graph_stats(PG_FUNCTION_ARGS)
PG_RETURN_POINTER(agtype_value_to_agtype(result.res));
}
+
+/*
+ * ============================================================================
+ * Graph Version Counter Implementation
+ *
+ * Provides per-graph monotonic version counters in shared memory for
+ * cross-backend VLE cache invalidation. Three modes are supported:
+ *
+ * DSM (PG 17+): Uses GetNamedDSMSegment — works without shared_preload_libs
+ * SHMEM (PG <17): Uses shmem_request/startup hooks — needs shared_preload_libs
+ * SNAPSHOT: Falls back to original snapshot-based invalidation
+ * ============================================================================
+ */
+
+#if PG_VERSION_NUM >= 170000
+/*
+ * DSM path: GetNamedDSMSegment init callback.
+ * Called once when the DSM segment is first created.
+ */
+static void age_dsm_init_callback(void *ptr)
+{
+ GraphVersionState *state = (GraphVersionState *) ptr;
+
+ LWLockInitialize(&state->lock,
+ LWLockNewTrancheId());
+ LWLockRegisterTranche(state->lock.tranche, "age_graph_version");
+ state->num_entries = 0;
+ memset(state->entries, 0, sizeof(state->entries));
+}
+
+/*
+ * Get the shared GraphVersionState via DSM registry.
+ * The segment is created on first access and persists until server shutdown.
+ */
+static GraphVersionState *get_version_state_dsm(void)
+{
+ bool found;
+
+ return (GraphVersionState *)
+ GetNamedDSMSegment("age_graph_versions",
+ sizeof(GraphVersionState),
+ age_dsm_init_callback,
+ &found);
+}
+#endif /* PG_VERSION_NUM >= 170000 */
+
+/*
+ * SHMEM path: request and startup hooks for PG < 17.
+ * These are registered in _PG_init when shared_preload_libraries is used.
+ */
+void age_graph_version_shmem_request(void)
+{
+ RequestAddinShmemSpace(MAXALIGN(sizeof(GraphVersionState)));
+ RequestNamedLWLockTranche("age_graph_version", 1);
+}
+
+void age_graph_version_shmem_startup(void)
+{
+ bool found;
+
+ LWLockAcquire(AddinShmemInitLock, LW_EXCLUSIVE);
+
+ shmem_version_state =
+ (GraphVersionState *) ShmemInitStruct("AGE Graph Version State",
+ sizeof(GraphVersionState),
+ &found);
+ if (!found)
+ {
+ LWLockInitialize(&shmem_version_state->lock,
+ LWLockNewTrancheId());
+ LWLockRegisterTranche(shmem_version_state->lock.tranche,
+ "age_graph_version");
+ shmem_version_state->num_entries = 0;
+ memset(shmem_version_state->entries, 0,
+ sizeof(shmem_version_state->entries));
+ }
+
+ LWLockRelease(AddinShmemInitLock);
+}
+
+/*
+ * Detect which version mode to use. Called once per backend on first access.
+ * Emits a DEBUG1 log message indicating the chosen mode.
+ */
+static void detect_version_mode(void)
+{
+#if PG_VERSION_NUM >= 170000
+ version_mode = VERSION_MODE_DSM;
+ elog(DEBUG1, "AGE: VLE cache using DSM version counter");
+#else
+ if (shmem_version_state != NULL)
+ {
+ version_mode = VERSION_MODE_SHMEM;
+ elog(DEBUG1, "AGE: VLE cache using SHMEM version counter");
+ }
+ else
+ {
+ version_mode = VERSION_MODE_SNAPSHOT;
+ elog(DEBUG1, "AGE: VLE cache using snapshot-based invalidation "
+ "(add AGE to shared_preload_libraries for better caching)");
+ }
+#endif
+}
+
+/*
+ * Get a pointer to the GraphVersionState, regardless of mode.
+ * Returns NULL only in SNAPSHOT mode (no shared memory available).
+ */
+static GraphVersionState *get_version_state(void)
+{
+ if (version_mode == VERSION_MODE_UNKNOWN)
+ detect_version_mode();
+
+#if PG_VERSION_NUM >= 170000
+ if (version_mode == VERSION_MODE_DSM)
+ return get_version_state_dsm();
+#endif
+
+ if (version_mode == VERSION_MODE_SHMEM)
+ return shmem_version_state;
+
+ return NULL;
+}
+
+/*
+ * Get the current version counter for a graph.
+ * Returns 0 if the graph has never been tracked or if shared memory
+ * is not available. Lock-free read via pg_atomic_read_u64.
+ */
+uint64 get_graph_version(Oid graph_oid)
+{
+ GraphVersionState *state = get_version_state();
+ int i;
+
+ if (state == NULL)
+ return 0;
+
+ /* lock-free scan of the array */
+ for (i = 0; i < state->num_entries; i++)
+ {
+ if (state->entries[i].graph_oid == graph_oid)
+ return pg_atomic_read_u64(&state->entries[i].version);
+ }
Review Comment:
`GraphVersionState.num_entries` is written under `LWLock`, but read
lock-free in `get_graph_version()`/`increment_graph_version()`. On weak
memory-ordering architectures this can allow readers to observe `num_entries`
incremented before the corresponding `entries[idx]` fields are fully
initialized, leading to missed/incorrect version reads and unreliable cache
invalidation. Consider making `num_entries` an atomic with proper
release/acquire semantics (or scanning the full fixed-size array, or taking the
LWLock for reads) to guarantee initialization visibility.
##########
src/backend/utils/adt/age_global_graph.c:
##########
@@ -1450,3 +1595,272 @@ Datum age_graph_stats(PG_FUNCTION_ARGS)
PG_RETURN_POINTER(agtype_value_to_agtype(result.res));
}
+
+/*
+ * ============================================================================
+ * Graph Version Counter Implementation
+ *
+ * Provides per-graph monotonic version counters in shared memory for
+ * cross-backend VLE cache invalidation. Three modes are supported:
+ *
+ * DSM (PG 17+): Uses GetNamedDSMSegment — works without shared_preload_libs
+ * SHMEM (PG <17): Uses shmem_request/startup hooks — needs shared_preload_libs
+ * SNAPSHOT: Falls back to original snapshot-based invalidation
+ * ============================================================================
+ */
+
+#if PG_VERSION_NUM >= 170000
+/*
+ * DSM path: GetNamedDSMSegment init callback.
+ * Called once when the DSM segment is first created.
+ */
+static void age_dsm_init_callback(void *ptr)
+{
+ GraphVersionState *state = (GraphVersionState *) ptr;
+
+ LWLockInitialize(&state->lock,
+ LWLockNewTrancheId());
+ LWLockRegisterTranche(state->lock.tranche, "age_graph_version");
+ state->num_entries = 0;
+ memset(state->entries, 0, sizeof(state->entries));
+}
+
+/*
+ * Get the shared GraphVersionState via DSM registry.
+ * The segment is created on first access and persists until server shutdown.
+ */
+static GraphVersionState *get_version_state_dsm(void)
+{
+ bool found;
+
+ return (GraphVersionState *)
+ GetNamedDSMSegment("age_graph_versions",
+ sizeof(GraphVersionState),
+ age_dsm_init_callback,
+ &found);
+}
+#endif /* PG_VERSION_NUM >= 170000 */
+
+/*
+ * SHMEM path: request and startup hooks for PG < 17.
+ * These are registered in _PG_init when shared_preload_libraries is used.
+ */
+void age_graph_version_shmem_request(void)
+{
+ RequestAddinShmemSpace(MAXALIGN(sizeof(GraphVersionState)));
+ RequestNamedLWLockTranche("age_graph_version", 1);
Review Comment:
`age_graph_version_shmem_request()` calls
`RequestNamedLWLockTranche("age_graph_version", 1)`, but the SHMEM
implementation uses an embedded `LWLock` initialized via `LWLockNewTrancheId()`
and never calls `GetNamedLWLockTranche()` to use the requested lock(s). Either
remove the `RequestNamedLWLockTranche` request or switch to the named-tranche
allocation pattern so the requested LWLock resources aren’t unused.
```suggestion
```
##########
src/backend/utils/adt/age_vle.c:
##########
@@ -364,6 +364,17 @@ static bool is_an_edge_match(VLE_local_context *vlelctx,
edge_entry *ee)
return false;
}
+ /*
+ * Fast path: if the label matched (or wasn't constrained) and there
+ * are no property constraints, the edge is a match. This avoids
+ * accessing edge properties entirely for label-only VLE patterns
+ * like [:KNOWS*1..3] which are the common case.
+ */
+ if (num_edge_property_constraints == 0)
+ {
+ return true;
+ }
+
/* get our edge's properties */
edge_property = DATUM_GET_AGTYPE_P(get_edge_entry_properties(ee));
Review Comment:
With thin entries, `get_edge_entry_properties()` now does a heap fetch. In
`is_an_edge_match`, the constrained-properties path still calls
`get_edge_entry_properties(ee)` multiple times (first to build `edge_property`,
then again for the hash/image fast check), which can trigger multiple heap
fetches for the same edge. Cache the fetched properties `Datum`/`agtype*`
locally and reuse it to avoid duplicate heap access when property constraints
are present.
```suggestion
/* get our edge's properties once and reuse them below */
{
Datum edge_property_datum;
edge_property_datum = get_edge_entry_properties(ee);
edge_property = DATUM_GET_AGTYPE_P(edge_property_datum);
}
```
--
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]
