Copilot commented on code in PR #2551:
URL: https://github.com/apache/age/pull/2551#discussion_r3848487794
##########
src/backend/utils/adt/age_global_graph.c:
##########
@@ -1372,52 +1385,133 @@ Oid get_vertex_entry_label_table_oid(vertex_entry *ve)
}
/*
- * Fetch vertex properties on demand from the heap via stored TID.
+ * Outcome of fetch_entry_properties(). A missing row and a row whose
properties
+ * are NULL are different failures: the first means the cache is stale, the
+ * second means the label table no longer satisfies the NOT NULL that AGE
+ * creates it with. Reporting them apart keeps a schema problem from being
+ * described as a cache problem.
+ */
+typedef enum entry_fetch_status
+{
+ ENTRY_FETCH_OK,
+ ENTRY_FETCH_GONE,
+ ENTRY_FETCH_NULL_PROPS
+} entry_fetch_status;
+
+/*
+ * Read one cached entry's properties out of the heap.
*
- * 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).
+ * The stored TID normally resolves under the active snapshot. It does not once
+ * this statement has deleted the tuple: cypher_delete() advances
+ * es_snapshot->curcid past every delete, so a path bound by an earlier MATCH
+ * can no longer see its own endpoints by the time it is projected (issue
+ * #2549). That tuple is still physically present -- our transaction has not
+ * committed, so nothing may prune it -- and the properties it carried when the
+ * path was matched are what the path should report, so it is read anyway.
+ *
+ * The relaxation is deliberately narrow: only a tuple deleted by our own
+ * transaction qualifies, and only while the row still holds the entity that
was
+ * cached, so a line pointer recycled by vacuum cannot be mistaken for the
+ * original. Anything else leaves *found false and the caller reports a stale
+ * entry, which is what keeps a genuine cache-invalidation bug visible.
Review Comment:
The block comment still refers to "*found" ("Anything else leaves *found
false"), but fetch_entry_properties() now reports outcome via the
entry_fetch_status* parameter. This is misleading documentation for the new
helper and can confuse future maintenance/debugging of stale-TID behavior.
--
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]
