jrgemignani commented on code in PR #2376:
URL: https://github.com/apache/age/pull/2376#discussion_r3088391657


##########
src/backend/utils/adt/age_global_graph.c:
##########
@@ -1277,9 +1390,41 @@ Oid get_edge_entry_label_table_oid(edge_entry *ee)
     return ee->edge_label_table_oid;
 }
 
+/*
+ * Fetch edge properties on demand from the heap via stored TID.
+ * See get_vertex_entry_properties for memory and safety notes.
+ */
 Datum get_edge_entry_properties(edge_entry *ee)
 {
-    return ee->edge_properties;
+    Relation rel;
+    HeapTupleData tuple;
+    Buffer buffer;
+    Datum result = (Datum) 0;
+
+    rel = table_open(ee->edge_label_table_oid, AccessShareLock);
+    tuple.t_self = ee->tid;
+
+    if (heap_fetch(rel, GetActiveSnapshot(), &tuple, &buffer, true))
+    {
+        TupleDesc tupdesc = RelationGetDescr(rel);
+        bool isnull;
+        Datum props;
+
+        /* properties is column 4 (1-indexed) */
+        props = heap_getattr(&tuple, 4, tupdesc, &isnull);
+        if (!isnull)
+            result = datumCopy(props, false, -1);
+
+        ReleaseBuffer(buffer);
+    }
+
+    table_close(rel, AccessShareLock);
+

Review Comment:
   Per Opus, not really an issue.



##########
src/backend/utils/adt/age_global_graph.c:
##########
@@ -1261,9 +1328,55 @@ Oid get_vertex_entry_label_table_oid(vertex_entry *ve)
     return ve->vertex_label_table_oid;
 }
 
+/*
+ * Fetch vertex properties on demand from the heap via stored TID.
+ *
+ * Returns a datumCopy of the properties in the current memory context.
+ * The caller does not need to free the result explicitly — it will be
+ * freed when the memory context is reset (typically the SRF multi-call
+ * context for VLE, which is cleaned up when the SRF completes).
+ *
+ * If the tuple is no longer visible (e.g., concurrent mutation between
+ * cache build and fetch), the version counter should have invalidated
+ * the cache. If we get here with a stale TID, it indicates a bug in
+ * the invalidation logic.
+ */
 Datum get_vertex_entry_properties(vertex_entry *ve)
 {
-    return ve->vertex_properties;
+    Relation rel;
+    HeapTupleData tuple;
+    Buffer buffer;
+    Datum result = (Datum) 0;
+
+    rel = table_open(ve->vertex_label_table_oid, AccessShareLock);
+    tuple.t_self = ve->tid;
+
+    if (heap_fetch(rel, GetActiveSnapshot(), &tuple, &buffer, true))
+    {
+        TupleDesc tupdesc = RelationGetDescr(rel);
+        bool isnull;
+        Datum props;
+
+        /* properties is column 2 (1-indexed) */
+        props = heap_getattr(&tuple, 2, tupdesc, &isnull);
+        if (!isnull)
+            result = datumCopy(props, false, -1);
+
+        ReleaseBuffer(buffer);
+    }
+
+    table_close(rel, AccessShareLock);
+

Review Comment:
   Per Opus, not really an issue.



-- 
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]

Reply via email to