jrgemignani commented on code in PR #2561:
URL: https://github.com/apache/age/pull/2561#discussion_r3929850802
##########
src/backend/utils/adt/age_global_graph.c:
##########
@@ -724,6 +786,279 @@ static bool insert_vertex_edge(GRAPH_global_context
*ggctx,
return false;
}
+/*
+ * RLS-aware loader for a single vertex label table.
+ *
+ * Loads (id, ctid) through SPI so the planner applies row-level security
+ * policies (and table/column ACL) for the current role, then inserts each
+ * visible row into the graph's global vertex hashtable. The physical tuple
+ * location (ctid) is stored for the same lazy property fetch the direct-scan
+ * path uses (get_vertex_entry_properties); because only RLS-visible rows are
+ * loaded, that later heap_fetch only ever reads authorized tuples. Used in
+ * place of the direct heap scan when age.enforce_rls_in_traversal is on and
RLS
+ * is active on the label table. Rows are fetched in batches so the entire
label
+ * table is never materialized at once.
+ */
+static void load_vertex_label_rls(GRAPH_global_context *ggctx,
+ Oid vertex_label_table_oid,
+ char *vertex_label_name)
+{
+ MemoryContext ggctx_cxt = CurrentMemoryContext;
+ StringInfoData query;
+ SPIPlanPtr plan;
+ Portal portal;
+
+ if (SPI_connect() != SPI_OK_CONNECT)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INTERNAL_ERROR),
+ errmsg("load_vertex_label_rls: SPI_connect failed")));
+ }
+
+ /*
+ * Build the query text AFTER SPI_connect(): SPI switches to its own
+ * procedure memory context here, so the StringInfo buffer and the
+ * quote_qualified_identifier() result are allocated in that context and
+ * freed by SPI_finish(), instead of leaking into the caller's long-lived
+ * (TopMemoryContext) cache-build context that is re-entered on every RLS
+ * cache rebuild.
+ */
+ initStringInfo(&query);
+ appendStringInfo(&query, "SELECT id, ctid FROM ONLY %s",
+ quote_qualified_identifier(ggctx->graph_name,
+ vertex_label_name));
+
+ plan = SPI_prepare(query.data, 0, NULL);
+ if (plan == NULL)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INTERNAL_ERROR),
+ errmsg("load_vertex_label_rls: SPI_prepare failed: %s",
+ SPI_result_code_string(SPI_result))));
+ }
+
+ /* read-only cursor: reuses the active snapshot; RLS applied by planner */
+ portal = SPI_cursor_open(NULL, plan, NULL, NULL, true);
Review Comment:
SPI_cursor_open() cannot return NULL. From the PostgreSQL docs
(doc/src/sgml/spi.sgml, Return Value for SPI_cursor_open):
Pointer to portal containing the cursor. Note there is no error return
convention; any error will be reported via elog.
The implementation agrees. SPI_cursor_open() is a thin wrapper over
SPI_cursor_open_internal(), which has exactly one return portal; and no return
NULL; anywhere — every failure mode is ereport(ERROR)/elog(ERROR) (not
connected, bad plan magic, multi-query plan, etc.), and the portal comes from
CreatePortal()/CreateNewPortal(), which palloc (never returns NULL) or
elog(ERROR) on a duplicate name.
So the claimed "NULL portal dereference" is unreachable, and the suggested
fix is dead code.
--
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]
