* Andres Freund (and...@anarazel.de) wrote:
> On 2015-08-28 08:49:24 -0400, Stephen Frost wrote:
> >  
> > +                           /*
> > +                            * get_row_security_policies just added to 
> > securityQuals and/or
> > +                            * withCheckOptions, and there were SubLinks, 
> > so make sure
> > +                            * we lock any relations which were added as a 
> > result.
> > +                            */
> 
> Very minor comment: Strictly speaking the quals/wces haven't yet been
> added to the Query, that happens only few lines down. I think it makes
> sense to mention that we normally rely on the parser to acquire locks,
> but that can't work here since sec quals/wces aren't visible to the
> parser.

Better?

        Thanks!

                Stephen
From 4cd1a52b1a869e2357f7cf0a65788883690a89b7 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfr...@snowman.net>
Date: Fri, 28 Aug 2015 08:10:22 -0400
Subject: [PATCH] Ensure locks are acquired on RLS-added relations

During fireRIRrules(), get_row_security_policies can add to
securityQuals and withCheckOptions.  Make sure to lock any relations
added at that point and before firing RIR rules on those expressions.

Back-patch to 9.5 where RLS was added.
---
 src/backend/rewrite/rewriteHandler.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 1734e48..a238cff 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1787,6 +1787,8 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
 		{
 			if (hasSubLinks)
 			{
+				acquireLocksOnSubLinks_context context;
+
 				/*
 				 * Recursively process the new quals, checking for infinite
 				 * recursion.
@@ -1799,6 +1801,23 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
 
 				activeRIRs = lcons_oid(RelationGetRelid(rel), activeRIRs);
 
+				/*
+				 * get_row_security_policies just passed back securityQuals
+				 * and/or withCheckOptions, and there were SubLinks, make sure
+				 * we lock any relations which are referenced.
+				 *
+				 * These locks would normally be acquired by the parser, but
+				 * securityQuals and withCheckOptions are added post-parsing.
+				 */
+				context.for_execute = true;
+				(void) acquireLocksOnSubLinks((Node *) securityQuals, &context);
+				(void) acquireLocksOnSubLinks((Node *) withCheckOptions,
+											  &context);
+
+				/*
+				 * Now that we have the locks on anything added by
+				 * get_row_security_policies, fire any RIR rules for them.
+				 */
 				expression_tree_walker((Node *) securityQuals,
 									   fireRIRonSubLink, (void *) activeRIRs);
 
-- 
1.9.1

Attachment: signature.asc
Description: Digital signature

Reply via email to