diff options
| -rw-r--r-- | src/backend/utils/cache/plancache.c | 41 | ||||
| -rw-r--r-- | src/test/regress/expected/rowsecurity.out | 14 | ||||
| -rw-r--r-- | src/test/regress/sql/rowsecurity.sql | 9 |
3 files changed, 49 insertions, 15 deletions
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c index a93825d0087..8fd9f2b573f 100644 --- a/src/backend/utils/cache/plancache.c +++ b/src/backend/utils/cache/plancache.c @@ -16,7 +16,8 @@ * if it has one. When (and if) the next demand for a cached plan occurs, * parse analysis and rewrite is repeated to build a new valid query tree, * and then planning is performed as normal. We also force re-analysis and - * re-planning if the active search_path is different from the previous time. + * re-planning if the active search_path is different from the previous time + * or, if RLS is involved, if the user changes or the RLS environment changes. * * Note that if the sinval was a result of user DDL actions, parse analysis * could throw an error, for example if a column referenced by the query is @@ -208,8 +209,8 @@ CreateCachedPlan(Node *raw_parse_tree, plansource->total_custom_cost = 0; plansource->num_custom_plans = 0; plansource->hasRowSecurity = false; - plansource->row_security_env = row_security; plansource->planUserId = InvalidOid; + plansource->row_security_env = false; MemoryContextSwitchTo(oldcxt); @@ -275,6 +276,8 @@ CreateOneShotCachedPlan(Node *raw_parse_tree, plansource->generic_cost = -1; plansource->total_custom_cost = 0; plansource->num_custom_plans = 0; + plansource->planUserId = InvalidOid; + plansource->row_security_env = false; return plansource; } @@ -413,6 +416,8 @@ CompleteCachedPlan(CachedPlanSource *plansource, plansource->cursor_options = cursor_options; plansource->fixed_result = fixed_result; plansource->resultDesc = PlanCacheComputeResultDesc(querytree_list); + plansource->planUserId = GetUserId(); + plansource->row_security_env = row_security; MemoryContextSwitchTo(oldcxt); @@ -576,24 +581,14 @@ RevalidateCachedQuery(CachedPlanSource *plansource) } /* - * If this is a new cached plan, then set the user id it was planned by - * and under what row security settings; these are needed to determine - * plan invalidation when RLS is involved or foreign joins are pushed - * down. - */ - if (!OidIsValid(plansource->planUserId)) - { - plansource->planUserId = GetUserId(); - plansource->row_security_env = row_security; - } - - /* * If the query is currently valid, we should have a saved search_path --- * check to see if that matches the current environment. If not, we want - * to force replan. + * to force replan. We should also have a valid planUserId. */ if (plansource->is_valid) { + Assert(OidIsValid(plansource->planUserId)); + Assert(plansource->search_path != NULL); if (!OverrideSearchPathMatchesCurrent(plansource->search_path)) { @@ -661,6 +656,14 @@ RevalidateCachedQuery(CachedPlanSource *plansource) plansource->search_path = NULL; /* + * The plan is invalid, possibly due to row security, so we need to reset + * row_security_env and planUserId as we're about to re-plan with the + * current settings. + */ + plansource->row_security_env = row_security; + plansource->planUserId = GetUserId(); + + /* * Free the query_context. We don't really expect MemoryContextDelete to * fail, but just in case, make sure the CachedPlanSource is left in a * reasonably sane state. (The generic plan won't get unlinked yet, but @@ -1412,6 +1415,14 @@ CopyCachedPlan(CachedPlanSource *plansource) newsource->total_custom_cost = plansource->total_custom_cost; newsource->num_custom_plans = plansource->num_custom_plans; + /* + * Copy over the user the query was planned as, and under what RLS + * environment. We will check during RevalidateCachedQuery() if the user + * or environment has changed and, if so, will force a re-plan. + */ + newsource->planUserId = plansource->planUserId; + newsource->row_security_env = plansource->row_security_env; + MemoryContextSwitchTo(oldcxt); return newsource; diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index 4aaa88f2c3e..067aa8d5fb7 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -2334,8 +2334,10 @@ GRANT SELECT ON t1 TO rls_regress_user1, rls_regress_user2; CREATE POLICY p1 ON t1 TO rls_regress_user1 USING ((a % 2) = 0); CREATE POLICY p2 ON t1 TO rls_regress_user2 USING ((a % 4) = 0); ALTER TABLE t1 ENABLE ROW LEVEL SECURITY; +-- Prepare as rls_regress_user1 SET ROLE rls_regress_user1; PREPARE role_inval AS SELECT * FROM t1; +-- Check plan EXPLAIN (COSTS OFF) EXECUTE role_inval; QUERY PLAN ------------------------- @@ -2343,7 +2345,9 @@ EXPLAIN (COSTS OFF) EXECUTE role_inval; Filter: ((a % 2) = 0) (2 rows) +-- Change to rls_regress_user2 SET ROLE rls_regress_user2; +-- Check plan- should be different EXPLAIN (COSTS OFF) EXECUTE role_inval; QUERY PLAN ------------------------- @@ -2351,6 +2355,16 @@ EXPLAIN (COSTS OFF) EXECUTE role_inval; Filter: ((a % 4) = 0) (2 rows) +-- Change back to rls_regress_user1 +SET ROLE rls_regress_user1; +-- Check plan- should be back to original +EXPLAIN (COSTS OFF) EXECUTE role_inval; + QUERY PLAN +------------------------- + Seq Scan on t1 + Filter: ((a % 2) = 0) +(2 rows) + -- -- CTE and RLS -- diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql index b5f5bcf8dec..18d0a386523 100644 --- a/src/test/regress/sql/rowsecurity.sql +++ b/src/test/regress/sql/rowsecurity.sql @@ -852,11 +852,20 @@ CREATE POLICY p2 ON t1 TO rls_regress_user2 USING ((a % 4) = 0); ALTER TABLE t1 ENABLE ROW LEVEL SECURITY; +-- Prepare as rls_regress_user1 SET ROLE rls_regress_user1; PREPARE role_inval AS SELECT * FROM t1; +-- Check plan EXPLAIN (COSTS OFF) EXECUTE role_inval; +-- Change to rls_regress_user2 SET ROLE rls_regress_user2; +-- Check plan- should be different +EXPLAIN (COSTS OFF) EXECUTE role_inval; + +-- Change back to rls_regress_user1 +SET ROLE rls_regress_user1; +-- Check plan- should be back to original EXPLAIN (COSTS OFF) EXECUTE role_inval; -- |