From d2e5167dbda5c6be2bb750ac24a469be48a627b9 Mon Sep 17 00:00:00 2001 From: John Gemignani Date: Tue, 13 Oct 2026 00:58:41 +0000 Subject: [PATCH] Reuse RLS-filtered graphs only within the traversal that loaded them Since #2592, a query reuses an RLS-filtered graph across its traversals when a walker over the planned row-security quals judges that the policies cannot change during the statement. The walker copies PostgreSQL's contain_mutable_functions_walker, treats SQL value functions such as CURRENT_DATE and current_user as fixed, and reads the quals through SPI_plan_get_plan_sources(), which PostgreSQL leaves undocumented. The classification is not reliable. CURRENT_DATE, CURRENT_TIME, LOCALTIME and LOCALTIMESTAMP depend on the session TimeZone, which a statement can change with set_config(), and the inspected quals do not contain function default arguments, which the planner inserts later. A policy comparing CURRENT_DATE with an immutable date expression showed 5 links to the first of two traversals and, after the second switched the time zone, the same 5 links to the second, where two scans of the label return 5 and 0. The walker also uses node types that PostgreSQL 16 and earlier do not have, so it cannot be backported unchanged, and every new node type must be tracked by hand. Remove the walker and reuse a graph loaded through RLS only for the traversal that loaded it, as already done for policies the walker classified as varying. A traversal still loads the graph once instead of once per row, so the 400-edge workload from the #2561 review still evaluates its policy 400 times instead of 160,000. A query with several traversals of one RLS-protected graph loads it once per traversal. Graphs loaded without RLS are unaffected. In rls_vle 8.3, two traversals with an immutable policy, alone and with current_user, now evaluate it 40 times instead of 20; the other counts are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- regress/expected/rls_vle.out | 23 ++- regress/sql/rls_vle.sql | 19 +-- src/backend/utils/adt/age_global_graph.c | 192 ++--------------------- 3 files changed, 26 insertions(+), 208 deletions(-) diff --git a/regress/expected/rls_vle.out b/regress/expected/rls_vle.out index e567da5f5..2b114529b 100644 --- a/regress/expected/rls_vle.out +++ b/regress/expected/rls_vle.out @@ -444,11 +444,8 @@ RESET ROLE; -- 8.3: policy evaluations per query. Each traversal loads the 20 visible -- edges once and reuses its load when it is evaluated again for another row, -- as PostgreSQL rescans a materialized scan. Another traversal in the query --- shares that load only when the policy gives the same result throughout the --- statement: when it calls only IMMUTABLE functions, besides values such as --- current_user that cannot change unnoticed. Otherwise, as with the STABLE --- and volatile counters below, it loads the graph again, as a separate scan --- would evaluate the policy again. +-- loads the graph again, as a separate scan would evaluate the policy again, +-- whichever functions the policy calls. SELECT age_delete_global_graphs(NULL) IS NOT NULL AS graph_caches_cleared; graph_caches_cleared ---------------------- @@ -556,10 +553,9 @@ SELECT age_delete_global_graphs(NULL) IS NOT NULL AS graph_caches_cleared; t (1 row) --- policies that give the same result throughout the statement: the two --- traversals share one load. The counter is declared IMMUTABLE so that the --- policy counts as fixed, and its argument keeps it from being evaluated --- once at planning. +-- a policy that calls only IMMUTABLE functions, alone and with current_user: +-- each traversal still loads the graph. The counter's argument keeps it from +-- being evaluated once at planning. CREATE FUNCTION public.rls_vle_count_fixed(id graphid) RETURNS boolean LANGUAGE plpgsql IMMUTABLE AS $f$ @@ -588,7 +584,7 @@ SELECT last_value AS immutable_two_call_site_evaluations FROM public.rls_vle_policy_calls; immutable_two_call_site_evaluations ------------------------------------- - 20 + 40 (1 row) ALTER POLICY link_all ON rls_vle_many."LINK" @@ -611,12 +607,11 @@ SELECT last_value AS current_user_two_call_site_evaluations FROM public.rls_vle_policy_calls; current_user_two_call_site_evaluations ---------------------------------------- - 20 + 40 (1 row) --- the search path can change during the statement, and a STABLE function --- in a subquery of the policy counts as in the policy itself: each --- traversal loads the graph +-- a policy that reads the search path, and one that calls a STABLE function +-- in a subquery: each traversal loads the graph ALTER POLICY link_all ON rls_vle_many."LINK" USING (public.rls_vle_count_fixed(id) AND current_schema IS NOT NULL); ALTER SEQUENCE public.rls_vle_policy_calls RESTART; diff --git a/regress/sql/rls_vle.sql b/regress/sql/rls_vle.sql index 11e6de507..1882e8cda 100644 --- a/regress/sql/rls_vle.sql +++ b/regress/sql/rls_vle.sql @@ -313,11 +313,8 @@ RESET ROLE; -- 8.3: policy evaluations per query. Each traversal loads the 20 visible -- edges once and reuses its load when it is evaluated again for another row, -- as PostgreSQL rescans a materialized scan. Another traversal in the query --- shares that load only when the policy gives the same result throughout the --- statement: when it calls only IMMUTABLE functions, besides values such as --- current_user that cannot change unnoticed. Otherwise, as with the STABLE --- and volatile counters below, it loads the graph again, as a separate scan --- would evaluate the policy again. +-- loads the graph again, as a separate scan would evaluate the policy again, +-- whichever functions the policy calls. SELECT age_delete_global_graphs(NULL) IS NOT NULL AS graph_caches_cleared; CREATE SEQUENCE public.rls_vle_policy_calls; GRANT USAGE, SELECT ON SEQUENCE public.rls_vle_policy_calls TO rls_vle_user; @@ -376,10 +373,9 @@ SELECT count(*) AS contexts_after_stable_query FROM pg_backend_memory_contexts WHERE name = 'AGE edge_table'; SELECT age_delete_global_graphs(NULL) IS NOT NULL AS graph_caches_cleared; --- policies that give the same result throughout the statement: the two --- traversals share one load. The counter is declared IMMUTABLE so that the --- policy counts as fixed, and its argument keeps it from being evaluated --- once at planning. +-- a policy that calls only IMMUTABLE functions, alone and with current_user: +-- each traversal still loads the graph. The counter's argument keeps it from +-- being evaluated once at planning. CREATE FUNCTION public.rls_vle_count_fixed(id graphid) RETURNS boolean LANGUAGE plpgsql IMMUTABLE AS $f$ @@ -416,9 +412,8 @@ RESET ROLE; SELECT last_value AS current_user_two_call_site_evaluations FROM public.rls_vle_policy_calls; --- the search path can change during the statement, and a STABLE function --- in a subquery of the policy counts as in the policy itself: each --- traversal loads the graph +-- a policy that reads the search path, and one that calls a STABLE function +-- in a subquery: each traversal loads the graph ALTER POLICY link_all ON rls_vle_many."LINK" USING (public.rls_vle_count_fixed(id) AND current_schema IS NOT NULL); ALTER SEQUENCE public.rls_vle_policy_calls RESTART; diff --git a/src/backend/utils/adt/age_global_graph.c b/src/backend/utils/adt/age_global_graph.c index fae12d794..1e366f3a9 100644 --- a/src/backend/utils/adt/age_global_graph.c +++ b/src/backend/utils/adt/age_global_graph.c @@ -38,13 +38,6 @@ #include "executor/spi.h" #include "lib/stringinfo.h" #include "miscadmin.h" -#include "optimizer/optimizer.h" -#include "catalog/pg_proc.h" -#include "nodes/nodeFuncs.h" -#include "utils/json.h" -#include "utils/jsonb.h" -#include "utils/jsonpath.h" -#include "utils/plancache.h" #include "utils/rls.h" #include "utils/acl.h" #include "utils/inval.h" @@ -192,7 +185,6 @@ typedef struct GRAPH_global_context bool loaded_rls_enforced; /* age.enforce_rls_in_traversal value at load */ bool vertices_rls_filtered; /* true if any vertex label was RLS-filtered at load */ bool loaded_with_rls; /* true if any label was loaded through RLS (SPI) */ - bool rls_quals_vary; /* RLS quals can vary in a statement */ bool security_invalidated; /* set by ACL/RLS catalog inval callbacks */ bool retired; /* not offered to new acquisitions */ int pin_count; /* queries still using this context */ @@ -302,9 +294,6 @@ static void load_vertex_label_rls(GRAPH_global_context *ggctx, static void load_edge_label_rls(GRAPH_global_context *ggctx, Oid edge_label_table_oid, char *edge_label_name); -static bool rls_function_not_immutable(Oid func_id, void *context); -static bool rls_quals_vary_walker(Node *node, void *context); -static bool rls_plan_quals_vary(SPIPlanPtr plan); static void unlink_GRAPH_global_context(GRAPH_global_context *ggctx); static void release_GRAPH_context_pin(void *arg); static bool graph_context_reusable(GRAPH_context_pin *pin, @@ -863,157 +852,6 @@ static bool insert_vertex_edge(GRAPH_global_context *ggctx, return false; } -/* checker for check_functions_in_node: a function that is not IMMUTABLE */ -static bool rls_function_not_immutable(Oid func_id, void *context) -{ - return (func_volatile(func_id) != PROVOLATILE_IMMUTABLE); -} - -/* - * PostgreSQL's contain_mutable_functions_walker, except for the SQL value - * functions whose result cannot change during a statement without the reuse - * check noticing: the date and time ones are fixed for the transaction, - * CURRENT_ROLE, CURRENT_USER and USER are the current role, which the reuse - * check compares, and CURRENT_CATALOG is fixed for the session. SESSION_USER - * and CURRENT_SCHEMA can change within a statement. - */ -static bool rls_quals_vary_walker(Node *node, void *context) -{ - if (node == NULL) - { - return false; - } - - if (check_functions_in_node(node, rls_function_not_immutable, context)) - { - return true; - } - - if (IsA(node, JsonConstructorExpr)) - { - JsonConstructorExpr *ctor = (JsonConstructorExpr *) node; - bool is_jsonb = (ctor->returning->format->format_type == - JS_FORMAT_JSONB); - ListCell *lc; - - foreach (lc, ctor->args) - { - Oid typid = exprType(lfirst(lc)); - - if (is_jsonb ? !to_jsonb_is_immutable(typid) : - !to_json_is_immutable(typid)) - { - return true; - } - } - } - - if (IsA(node, JsonExpr)) - { - JsonExpr *jexpr = (JsonExpr *) node; - Const *cnst; - - if (!IsA(jexpr->path_spec, Const)) - { - return true; - } - - cnst = (Const *) jexpr->path_spec; - if (cnst->constisnull) - { - return false; - } - - if (jspIsMutable(DatumGetJsonPathP(cnst->constvalue), - jexpr->passing_names, jexpr->passing_values)) - { - return true; - } - } - - if (IsA(node, SQLValueFunction)) - { - switch (((SQLValueFunction *) node)->op) - { - case SVFOP_CURRENT_DATE: - case SVFOP_CURRENT_TIME: - case SVFOP_CURRENT_TIME_N: - case SVFOP_CURRENT_TIMESTAMP: - case SVFOP_CURRENT_TIMESTAMP_N: - case SVFOP_LOCALTIME: - case SVFOP_LOCALTIME_N: - case SVFOP_LOCALTIMESTAMP: - case SVFOP_LOCALTIMESTAMP_N: - case SVFOP_CURRENT_ROLE: - case SVFOP_CURRENT_USER: - case SVFOP_USER: - case SVFOP_CURRENT_CATALOG: - return false; - default: - return true; - } - } - - if (IsA(node, NextValueExpr)) - { - return true; - } - - if (IsA(node, Query)) - { - return query_tree_walker((Query *) node, rls_quals_vary_walker, - context, 0); - } - - return expression_tree_walker(node, rls_quals_vary_walker, context); -} - -/* - * Whether the row-security quals the planner applied to an RLS load can give - * another result later in the same statement, so that another scan of the - * label could return other rows. They can when they call a function that is - * not IMMUTABLE: a volatile one, or a STABLE one such as current_setting(), - * whose input the statement itself can change with set_config(). Policies - * from row_security_policy_hook_permissive and _restrictive are turned into - * the same securityQuals as catalog policies, so they are inspected too. A - * plan whose queries cannot be inspected is reported as varying; reuse is - * then limited to the call site that loaded the graph, which is valid for any - * policy. - */ -static bool rls_plan_quals_vary(SPIPlanPtr plan) -{ - List *sources; - ListCell *source_cell; - - sources = SPI_plan_get_plan_sources(plan); - if (sources == NIL) - { - return true; - } - - foreach (source_cell, sources) - { - CachedPlanSource *source = lfirst(source_cell); - ListCell *query_cell; - - if (source->query_list == NIL) - { - return true; - } - - foreach (query_cell, source->query_list) - { - /* walks the range table's securityQuals and any sublinks */ - if (rls_quals_vary_walker(lfirst(query_cell), NULL)) - { - return true; - } - } - } - - return false; -} - /* * Keep an owned, detoasted copy of an entry's properties in the context's * property memory, which is freed with the context. @@ -1098,11 +936,6 @@ static void load_vertex_label_rls(GRAPH_global_context *ggctx, SPI_result_code_string(SPI_result)))); } - if (rls_plan_quals_vary(plan)) - { - ggctx->rls_quals_vary = true; - } - /* read-only cursor: reuses the active snapshot; RLS applied by planner */ portal = SPI_cursor_open(NULL, plan, NULL, NULL, true); @@ -1214,11 +1047,6 @@ static void load_edge_label_rls(GRAPH_global_context *ggctx, SPI_result_code_string(SPI_result)))); } - if (rls_plan_quals_vary(plan)) - { - ggctx->rls_quals_vary = true; - } - /* read-only cursor: reuses the active snapshot; RLS applied by planner */ portal = SPI_cursor_open(NULL, plan, NULL, NULL, true); @@ -2517,12 +2345,12 @@ void pin_GRAPH_global_context(GRAPH_global_context *ggctx, MemoryContext owner, * the policies when it plans the statement and checks the privileges when * it starts it, so an authorization change made while the statement runs * does not end the reuse. - * - When the RLS quals give the same result throughout the statement, every - * scan of the labels in this statement returns the same rows, so any - * traversal may share the graph (see rls_plan_quals_vary). - * - Otherwise only the traversal call site that loaded the graph shares it, - * as when PostgreSQL scans a relation once and rescans the result. Another - * call site is a separate scan and evaluates the policies itself. + * - A graph loaded through RLS serves only the traversal call site that + * loaded it, as when PostgreSQL scans a relation once and rescans the + * result. Another call site is a separate scan and evaluates the policies + * itself, because a policy can depend on session state, such as a setting + * or the time zone, that the statement can change. A graph loaded without + * RLS may serve any traversal in the query. */ static bool graph_context_reusable(GRAPH_context_pin *pin, const void *call_site) @@ -2544,7 +2372,7 @@ static bool graph_context_reusable(GRAPH_context_pin *pin, return false; } - if (ggctx->rls_quals_vary && + if (ggctx->loaded_with_rls && (call_site == NULL || pin->call_site != call_site)) { return false; @@ -2559,9 +2387,9 @@ static bool graph_context_reusable(GRAPH_context_pin *pin, * the traversal or the materialization of its results still needs it. * call_site identifies the traversal within the query; it stays the same * when the traversal is evaluated again for another row. A context the query - * already pins is reused when graph_context_reusable() allows it, so the - * policies run once for each row a load scans rather than again for every - * traversal. + * already pins is reused when graph_context_reusable() allows it, so a + * traversal does not load the graph, and evaluate its policies, again each + * time it is evaluated for another row. */ GRAPH_global_context *acquire_GRAPH_global_context(char *graph_name, Oid graph_oid,