RLS open items are vague and unactionable

diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
new file mode 100644
index 45326a3..8851fe7
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -186,9 +186,6 @@ policy_role_list_to_array(List *roles, i
 /*
  * Load row security policy from the catalog, and store it in
  * the relation's relcache entry.
- *
- * We will always set up some kind of policy here.  If no explicit policies
- * are found then an implicit default-deny policy is created.
  */
 void
 RelationBuildRowSecurity(Relation relation)
@@ -246,7 +243,6 @@ RelationBuildRowSecurity(Relation relati
 			char	   *with_check_value;
 			Expr	   *with_check_qual;
 			char	   *policy_name_value;
-			Oid			policy_id;
 			bool		isnull;
 			RowSecurityPolicy *policy;
 
@@ -298,14 +294,11 @@ RelationBuildRowSecurity(Relation relati
 			else
 				with_check_qual = NULL;
 
-			policy_id = HeapTupleGetOid(tuple);
-
 			/* Now copy everything into the cache context */
 			MemoryContextSwitchTo(rscxt);
 
 			policy = palloc0(sizeof(RowSecurityPolicy));
 			policy->policy_name = pstrdup(policy_name_value);
-			policy->policy_id = policy_id;
 			policy->polcmd = cmd_value;
 			policy->roles = DatumGetArrayTypePCopy(roles_datum);
 			policy->qual = copyObject(qual_expr);
@@ -326,40 +319,6 @@ RelationBuildRowSecurity(Relation relati
 
 		systable_endscan(sscan);
 		heap_close(catalog, AccessShareLock);
-
-		/*
-		 * Check if no policies were added
-		 *
-		 * If no policies exist in pg_policy for this relation, then we need
-		 * to create a single default-deny policy.  We use InvalidOid for the
-		 * Oid to indicate that this is the default-deny policy (we may decide
-		 * to ignore the default policy if an extension adds policies).
-		 */
-		if (rsdesc->policies == NIL)
-		{
-			RowSecurityPolicy *policy;
-			Datum		role;
-
-			MemoryContextSwitchTo(rscxt);
-
-			role = ObjectIdGetDatum(ACL_ID_PUBLIC);
-
-			policy = palloc0(sizeof(RowSecurityPolicy));
-			policy->policy_name = pstrdup("default-deny policy");
-			policy->policy_id = InvalidOid;
-			policy->polcmd = '*';
-			policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true,
-											'i');
-			policy->qual = (Expr *) makeConst(BOOLOID, -1, InvalidOid,
-										   sizeof(bool), BoolGetDatum(false),
-											  false, true);
-			policy->with_check_qual = copyObject(policy->qual);
-			policy->hassublinks = false;
-
-			rsdesc->policies = lcons(policy, rsdesc->policies);
-
-			MemoryContextSwitchTo(oldcxt);
-		}
 	}
 	PG_CATCH();
 	{
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
new file mode 100644
index 2c65a90..c28eb2b
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1815,14 +1815,26 @@ ExecWithCheckOptions(WCOKind kind, Resul
 					break;
 				case WCO_RLS_INSERT_CHECK:
 				case WCO_RLS_UPDATE_CHECK:
-					ereport(ERROR,
-							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					if (wco->polname != NULL)
+						ereport(ERROR,
+								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+							 errmsg("new row violates row level security policy \"%s\" for \"%s\"",
+									wco->polname, wco->relname)));
+					else
+						ereport(ERROR,
+								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 							 errmsg("new row violates row level security policy for \"%s\"",
 									wco->relname)));
 					break;
 				case WCO_RLS_CONFLICT_CHECK:
-					ereport(ERROR,
-							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					if (wco->polname != NULL)
+						ereport(ERROR,
+								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+							 errmsg("new row violates row level security policy \"%s\" (USING expression) for \"%s\"",
+									wco->polname, wco->relname)));
+					else
+						ereport(ERROR,
+								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 							 errmsg("new row violates row level security policy (USING expression) for \"%s\"",
 									wco->relname)));
 					break;
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
new file mode 100644
index bd2e80e..1c801f5
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2168,6 +2168,7 @@ _copyWithCheckOption(const WithCheckOpti
 
 	COPY_SCALAR_FIELD(kind);
 	COPY_STRING_FIELD(relname);
+	COPY_STRING_FIELD(polname);
 	COPY_NODE_FIELD(qual);
 	COPY_SCALAR_FIELD(cascaded);
 
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
new file mode 100644
index 19412fe..8f16833
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2455,6 +2455,7 @@ _equalWithCheckOption(const WithCheckOpt
 {
 	COMPARE_SCALAR_FIELD(kind);
 	COMPARE_STRING_FIELD(relname);
+	COMPARE_STRING_FIELD(polname);
 	COMPARE_NODE_FIELD(qual);
 	COMPARE_SCALAR_FIELD(cascaded);
 
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
new file mode 100644
index a878498..79b7179
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2403,6 +2403,7 @@ _outWithCheckOption(StringInfo str, cons
 
 	WRITE_ENUM_FIELD(kind, WCOKind);
 	WRITE_STRING_FIELD(relname);
+	WRITE_STRING_FIELD(polname);
 	WRITE_NODE_FIELD(qual);
 	WRITE_BOOL_FIELD(cascaded);
 }
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
new file mode 100644
index 23e0b36..df55b76
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -270,6 +270,7 @@ _readWithCheckOption(void)
 
 	READ_ENUM_FIELD(kind, WCOKind);
 	READ_STRING_FIELD(relname);
+	READ_STRING_FIELD(polname);
 	READ_NODE_FIELD(qual);
 	READ_BOOL_FIELD(cascaded);
 
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
new file mode 100644
index db3c2c7..1b8e7b0
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1786,8 +1786,8 @@ fireRIRrules(Query *parsetree, List *act
 		/*
 		 * Fetch any new security quals that must be applied to this RTE.
 		 */
-		get_row_security_policies(parsetree, parsetree->commandType, rte,
-								  rt_index, &securityQuals, &withCheckOptions,
+		get_row_security_policies(parsetree, rte, rt_index,
+								  &securityQuals, &withCheckOptions,
 								  &hasRowSecurity, &hasSubLinks);
 
 		if (securityQuals != NIL || withCheckOptions != NIL)
@@ -3026,6 +3026,7 @@ rewriteTargetView(Query *parsetree, Rela
 			wco = makeNode(WithCheckOption);
 			wco->kind = WCO_VIEW_CHECK;
 			wco->relname = pstrdup(RelationGetRelationName(view));
+			wco->polname = NULL;
 			wco->qual = NULL;
 			wco->cascaded = cascaded;
 
diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
new file mode 100644
index 5a81db3..1140ebd
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -13,11 +13,12 @@
  * Any part of the system which is returning records back to the user, or
  * which is accepting records from the user to add to a table, needs to
  * consider the policies associated with the table (if any).  For normal
- * queries, this is handled by calling prepend_row_security_policies() during
- * rewrite, which looks at each RTE and adds the expressions defined by the
- * policies to the securityQuals list for the RTE.  For queries which modify
- * the relation, any WITH CHECK policies are added to the list of
- * WithCheckOptions for the Query and checked against each row which is being
+ * queries, this is handled by calling get_row_security_policies() during
+ * rewrite, for each RTE in the query.  This returns the expressions defined
+ * by the table's policies as a list that is prepended to the securityQuals
+ * list for the RTE.  For queries which modify the table, any WITH CHECK
+ * clauses from the table's policies are also returned and prepended to the
+ * list of WithCheckOptions for the Query to check each row that is being
  * added to the table.  Other parts of the system (eg: COPY) simply construct
  * a normal query and use that, if RLS is to be applied.
  *
@@ -56,13 +57,29 @@
 #include "utils/syscache.h"
 #include "tcop/utility.h"
 
-static List *pull_row_security_policies(CmdType cmd, Relation relation,
-						   Oid user_id);
-static void process_policies(Query *root, List *policies, int rt_index,
-				 Expr **final_qual,
-				 Expr **final_with_check_qual,
-				 bool *hassublinks,
-				 BoolExprType boolop);
+static void get_policies_for_relation(Relation relation,
+						  CmdType cmd, Oid user_id,
+						  List **permissive_policies,
+						  List **restrictive_policies);
+
+static List *sort_policies_by_name(List *policies);
+
+static int row_security_policy_cmp(const void *a, const void *b);
+
+static void build_security_quals(int rt_index,
+					 List *permissive_policies,
+					 List *restrictive_policies,
+					 List **securityQuals,
+					 bool *hasSubLinks);
+
+static void build_with_check_options(Relation rel,
+						 int rt_index,
+						 WCOKind kind,
+						 List *permissive_policies,
+						 List *restrictive_policies,
+						 List **withCheckOptions,
+						 bool *hasSubLinks);
+
 static bool check_role_for_policy(ArrayType *policy_roles, Oid user_id);
 
 /*
@@ -73,42 +90,29 @@ static bool check_role_for_policy(ArrayT
  *
  * row_security_policy_hook_restrictive can be used to add policies which
  * are enforced, regardless of other policies (they are "AND"d).
- *
- * See below where the hook is called in prepend_row_security_policies for
- * insight into how to use this hook.
  */
 row_security_policy_hook_type row_security_policy_hook_permissive = NULL;
 row_security_policy_hook_type row_security_policy_hook_restrictive = NULL;
 
 /*
- * Get any row security quals and check quals that should be applied to the
- * specified RTE.
+ * Get any row security quals and WithCheckOption checks that should be
+ * applied to the specified RTE.
  *
  * In addition, hasRowSecurity is set to true if row level security is enabled
  * (even if this RTE doesn't have any row security quals), and hasSubLinks is
  * set to true if any of the quals returned contain sublinks.
  */
 void
-get_row_security_policies(Query *root, CmdType commandType, RangeTblEntry *rte,
-						  int rt_index, List **securityQuals,
-						  List **withCheckOptions, bool *hasRowSecurity,
-						  bool *hasSubLinks)
+get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
+						  List **securityQuals, List **withCheckOptions,
+						  bool *hasRowSecurity, bool *hasSubLinks)
 {
-	Expr	   *rowsec_expr = NULL;
-	Expr	   *rowsec_with_check_expr = NULL;
-	Expr	   *hook_expr_restrictive = NULL;
-	Expr	   *hook_with_check_expr_restrictive = NULL;
-	Expr	   *hook_expr_permissive = NULL;
-	Expr	   *hook_with_check_expr_permissive = NULL;
-
-	List	   *rowsec_policies;
-	List	   *hook_policies_restrictive = NIL;
-	List	   *hook_policies_permissive = NIL;
-
-	Relation	rel;
 	Oid			user_id;
 	int			rls_status;
-	bool		defaultDeny = false;
+	Relation	rel;
+	CmdType		commandType;
+	List	   *permissive_policies;
+	List	   *restrictive_policies;
 
 	/* Defaults for the return values */
 	*securityQuals = NIL;
@@ -157,257 +161,87 @@ get_row_security_policies(Query *root, C
 	 * policies and t2's SELECT policies.
 	 */
 	rel = heap_open(rte->relid, NoLock);
-	if (rt_index != root->resultRelation)
-		commandType = CMD_SELECT;
-
-	rowsec_policies = pull_row_security_policies(commandType, rel,
-												 user_id);
-
-	/*
-	 * Check if this is only the default-deny policy.
-	 *
-	 * Normally, if the table has row security enabled but there are no
-	 * policies, we use a default-deny policy and not allow anything. However,
-	 * when an extension uses the hook to add their own policies, we don't
-	 * want to include the default deny policy or there won't be any way for a
-	 * user to use an extension exclusively for the policies to be used.
-	 */
-	if (((RowSecurityPolicy *) linitial(rowsec_policies))->policy_id
-		== InvalidOid)
-		defaultDeny = true;
-
-	/* Now that we have our policies, build the expressions from them. */
-	process_policies(root, rowsec_policies, rt_index, &rowsec_expr,
-					 &rowsec_with_check_expr, hasSubLinks, OR_EXPR);
-
-	/*
-	 * Also, allow extensions to add their own policies.
-	 *
-	 * extensions can add either permissive or restrictive policies.
-	 *
-	 * Note that, as with the internal policies, if multiple policies are
-	 * returned then they will be combined into a single expression with all
-	 * of them OR'd (for permissive) or AND'd (for restrictive) together.
-	 *
-	 * If only a USING policy is returned by the extension then it will be
-	 * used for WITH CHECK as well, similar to how internal policies are
-	 * handled.
-	 *
-	 * The only caveat to this is that if there are NO internal policies
-	 * defined, there ARE policies returned by the extension, and RLS is
-	 * enabled on the table, then we will ignore the internally-generated
-	 * default-deny policy and use only the policies returned by the
-	 * extension.
-	 */
-	if (row_security_policy_hook_restrictive)
-	{
-		hook_policies_restrictive = (*row_security_policy_hook_restrictive) (commandType, rel);
-
-		/* Build the expression from any policies returned. */
-		if (hook_policies_restrictive != NIL)
-			process_policies(root, hook_policies_restrictive, rt_index,
-							 &hook_expr_restrictive,
-							 &hook_with_check_expr_restrictive,
-							 hasSubLinks,
-							 AND_EXPR);
-	}
 
-	if (row_security_policy_hook_permissive)
-	{
-		hook_policies_permissive = (*row_security_policy_hook_permissive) (commandType, rel);
+	commandType = rt_index == root->resultRelation ?
+				  root->commandType : CMD_SELECT;
 
-		/* Build the expression from any policies returned. */
-		if (hook_policies_permissive != NIL)
-			process_policies(root, hook_policies_permissive, rt_index,
-							 &hook_expr_permissive,
-							 &hook_with_check_expr_permissive, hasSubLinks,
-							 OR_EXPR);
-	}
+	get_policies_for_relation(rel, commandType, user_id,
+							  &permissive_policies, &restrictive_policies);
 
 	/*
-	 * If the only built-in policy is the default-deny one, and permissive hook
-	 * policies exist, then use the hook policies only and do not apply the
-	 * default-deny policy.  Otherwise, we will apply both sets below.
+	 * For SELECT, UPDATE and DELETE, build security quals to enforce these
+	 * policies.  These security quals control access to existing table rows.
+	 * Restrictive policies are "AND"d together, and permissive policies are
+	 * "OR"d together.
 	 *
-	 * Note that we do not remove the defaultDeny policy if only *restrictive*
-	 * policies exist as restrictive policies should only ever be reducing what
-	 * is visible.  Therefore, at least one permissive policy must exist which
-	 * allows records to be seen before restrictive policies can remove rows
-	 * from that set.  A single "true" policy can be created to address this
-	 * requirement, if necessary.
+	 * If there are no policy clauses controlling access to the table, this
+	 * will add a single always-false clause (a default-deny policy).
 	 */
-	if (defaultDeny && hook_policies_permissive != NIL)
+	if (commandType == CMD_SELECT ||
+		commandType == CMD_UPDATE ||
+		commandType == CMD_DELETE)
 	{
-		rowsec_expr = NULL;
-		rowsec_with_check_expr = NULL;
+		build_security_quals(rt_index,
+							 permissive_policies,
+							 restrictive_policies,
+							 securityQuals,
+							 hasSubLinks);
 	}
 
 	/*
-	 * For INSERT or UPDATE, we need to add the WITH CHECK quals to Query's
-	 * withCheckOptions to verify that any new records pass the WITH CHECK
-	 * policy (this will be a copy of the USING policy, if no explicit WITH
-	 * CHECK policy exists).
+	 * For INSERT and UPDATE add withCheckOptions to verify that any new
+	 * records added are consistent with the security policies.  This will use
+	 * each policy's WITH CHECK clause, or its USING clause if no explicit
+	 * WITH CHECK clause is defined.
 	 */
 	if (commandType == CMD_INSERT || commandType == CMD_UPDATE)
 	{
-		/*
-		 * WITH CHECK OPTIONS wants a WCO node which wraps each Expr, so
-		 * create them as necessary.
-		 */
-
-		/*
-		 * Handle any restrictive policies first.
-		 *
-		 * They can simply be added.
-		 */
-		if (hook_with_check_expr_restrictive)
-		{
-			WithCheckOption *wco;
-
-			wco = (WithCheckOption *) makeNode(WithCheckOption);
-			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
-				WCO_RLS_UPDATE_CHECK;
-			wco->relname = pstrdup(RelationGetRelationName(rel));
-			wco->qual = (Node *) hook_with_check_expr_restrictive;
-			wco->cascaded = false;
-			*withCheckOptions = lappend(*withCheckOptions, wco);
-		}
-
-		/*
-		 * Handle built-in policies, if there are no permissive policies from
-		 * the hook.
-		 */
-		if (rowsec_with_check_expr && !hook_with_check_expr_permissive)
-		{
-			WithCheckOption *wco;
-
-			wco = (WithCheckOption *) makeNode(WithCheckOption);
-			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
-				WCO_RLS_UPDATE_CHECK;
-			wco->relname = pstrdup(RelationGetRelationName(rel));
-			wco->qual = (Node *) rowsec_with_check_expr;
-			wco->cascaded = false;
-			*withCheckOptions = lappend(*withCheckOptions, wco);
-		}
-		/* Handle the hook policies, if there are no built-in ones. */
-		else if (!rowsec_with_check_expr && hook_with_check_expr_permissive)
-		{
-			WithCheckOption *wco;
-
-			wco = (WithCheckOption *) makeNode(WithCheckOption);
-			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
-				WCO_RLS_UPDATE_CHECK;
-			wco->relname = pstrdup(RelationGetRelationName(rel));
-			wco->qual = (Node *) hook_with_check_expr_permissive;
-			wco->cascaded = false;
-			*withCheckOptions = lappend(*withCheckOptions, wco);
-		}
-		/* Handle the case where there are both. */
-		else if (rowsec_with_check_expr && hook_with_check_expr_permissive)
-		{
-			WithCheckOption *wco;
-			List	   *combined_quals = NIL;
-			Expr	   *combined_qual_eval;
-
-			combined_quals = lcons(copyObject(rowsec_with_check_expr),
-								   combined_quals);
-
-			combined_quals = lcons(copyObject(hook_with_check_expr_permissive),
-								   combined_quals);
-
-			combined_qual_eval = makeBoolExpr(OR_EXPR, combined_quals, -1);
+		/* This should be the target relation */
+		Assert(rt_index == root->resultRelation);
 
-			wco = (WithCheckOption *) makeNode(WithCheckOption);
-			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
-				WCO_RLS_UPDATE_CHECK;
-			wco->relname = pstrdup(RelationGetRelationName(rel));
-			wco->qual = (Node *) combined_qual_eval;
-			wco->cascaded = false;
-			*withCheckOptions = lappend(*withCheckOptions, wco);
-		}
+		build_with_check_options(rel, rt_index,
+								 commandType == CMD_INSERT ?
+								 WCO_RLS_INSERT_CHECK : WCO_RLS_UPDATE_CHECK,
+								 permissive_policies,
+								 restrictive_policies,
+								 withCheckOptions,
+								 hasSubLinks);
 
 		/*
-		 * ON CONFLICT DO UPDATE has an RTE that is subject to both INSERT and
-		 * UPDATE RLS enforcement.  Those are enforced (as a special, distinct
-		 * kind of WCO) on the target tuple.
-		 *
-		 * Make a second, recursive pass over the RTE for this, gathering
-		 * UPDATE-applicable RLS checks/WCOs, and gathering and converting
-		 * UPDATE-applicable security quals into WCO_RLS_CONFLICT_CHECK RLS
-		 * checks/WCOs.  Finally, these distinct kinds of RLS checks/WCOs are
-		 * concatenated with our own INSERT-applicable list.
+		 * For INSERT ... ON CONFLICT DO UPDATE we need additional policy
+		 * checks for the UPDATE which may be applied to the same RTE.
 		 */
-		if (root->onConflict && root->onConflict->action == ONCONFLICT_UPDATE &&
-			commandType == CMD_INSERT)
+		if (commandType == CMD_INSERT &&
+			root->onConflict && root->onConflict->action == ONCONFLICT_UPDATE)
 		{
-			List	   *conflictSecurityQuals = NIL;
-			List	   *conflictWCOs = NIL;
-			ListCell   *item;
-			bool		conflictHasRowSecurity = false;
-			bool		conflictHasSublinks = false;
-
-			/* Assume that RTE is target resultRelation */
-			get_row_security_policies(root, CMD_UPDATE, rte, rt_index,
-									  &conflictSecurityQuals, &conflictWCOs,
-									  &conflictHasRowSecurity,
-									  &conflictHasSublinks);
+			List	   *conflict_permissive_policies;
+			List	   *conflict_restrictive_policies;
 
-			if (conflictHasRowSecurity)
-				*hasRowSecurity = true;
-			if (conflictHasSublinks)
-				*hasSubLinks = true;
+			/* Get the policies that apply to the auxiliary UPDATE */
+			get_policies_for_relation(rel, CMD_UPDATE, user_id,
+									  &conflict_permissive_policies,
+									  &conflict_restrictive_policies);
 
 			/*
-			 * Append WITH CHECK OPTIONs/RLS checks, which should not conflict
-			 * between this INSERT and the auxiliary UPDATE
+			 * Enforce the USING clauses of the UPDATE policies using WCOs
+			 * rather than security quals.  This ensures that an error is
+			 * raised if the conflicting row cannot be updated due to RLS,
+			 * rather than it being silently skipped.
 			 */
-			*withCheckOptions = list_concat(*withCheckOptions,
-											conflictWCOs);
-
-			foreach(item, conflictSecurityQuals)
-			{
-				Expr	   *conflict_rowsec_expr = (Expr *) lfirst(item);
-				WithCheckOption *wco;
-
-				wco = (WithCheckOption *) makeNode(WithCheckOption);
-
-				wco->kind = WCO_RLS_CONFLICT_CHECK;
-				wco->relname = pstrdup(RelationGetRelationName(rel));
-				wco->qual = (Node *) copyObject(conflict_rowsec_expr);
-				wco->cascaded = false;
-				*withCheckOptions = lappend(*withCheckOptions, wco);
-			}
-		}
-	}
-
-	/* For SELECT, UPDATE, and DELETE, set the security quals */
-	if (commandType == CMD_SELECT
-		|| commandType == CMD_UPDATE
-		|| commandType == CMD_DELETE)
-	{
-		/* restrictive policies can simply be added to the list first */
-		if (hook_expr_restrictive)
-			*securityQuals = lappend(*securityQuals, hook_expr_restrictive);
-
-		/* If we only have internal permissive, then just add those */
-		if (rowsec_expr && !hook_expr_permissive)
-			*securityQuals = lappend(*securityQuals, rowsec_expr);
-		/* .. and if we have only permissive policies from the hook */
-		else if (!rowsec_expr && hook_expr_permissive)
-			*securityQuals = lappend(*securityQuals, hook_expr_permissive);
-		/* if we have both, we have to combine them with an OR */
-		else if (rowsec_expr && hook_expr_permissive)
-		{
-			List	   *combined_quals = NIL;
-			Expr	   *combined_qual_eval;
-
-			combined_quals = lcons(copyObject(rowsec_expr), combined_quals);
-			combined_quals = lcons(copyObject(hook_expr_permissive),
-								   combined_quals);
-
-			combined_qual_eval = makeBoolExpr(OR_EXPR, combined_quals, -1);
+			build_with_check_options(rel, rt_index,
+									 WCO_RLS_CONFLICT_CHECK,
+									 conflict_permissive_policies,
+									 conflict_restrictive_policies,
+									 withCheckOptions,
+									 hasSubLinks);
 
-			*securityQuals = lappend(*securityQuals, combined_qual_eval);
+			/* Enforce the WITH CHECK clauses of the UPDATE policies */
+			build_with_check_options(rel, rt_index,
+									 WCO_RLS_UPDATE_CHECK,
+									 conflict_permissive_policies,
+									 conflict_restrictive_policies,
+									 withCheckOptions,
+									 hasSubLinks);
 		}
 	}
 
@@ -423,199 +257,367 @@ get_row_security_policies(Query *root, C
 }
 
 /*
- * pull_row_security_policies
+ * get_policies_for_relation
  *
- * Returns the list of policies to be added for this relation, based on the
- * type of command and the roles to which it applies, from the relation cache.
+ * Returns lists of permissive and restrictive policies to be applied to the
+ * specified relation, based on the command type and role.
  *
+ * This includes any policies added by extensions.
  */
-static List *
-pull_row_security_policies(CmdType cmd, Relation relation, Oid user_id)
+static void
+get_policies_for_relation(Relation relation,
+						  CmdType cmd, Oid user_id,
+						  List **permissive_policies,
+						  List **restrictive_policies)
 {
-	List	   *policies = NIL;
 	ListCell   *item;
 
+	*permissive_policies = NIL;
+	*restrictive_policies = NIL;
+
 	/*
-	 * Row security is enabled for the relation and the row security GUC is
-	 * either 'on' or 'force' here, so find the policies to apply to the
-	 * table. There must always be at least one policy defined (may be the
-	 * simple 'default-deny' policy, if none are explicitly defined on the
-	 * table).
+	 * First find all internal policies for the relation.  CREATE POLICY does
+	 * not currently support defining restrictive policies, so for now all
+	 * internal policies are permissive.
 	 */
 	foreach(item, relation->rd_rsdesc->policies)
 	{
 		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+		bool		cmd_matches = false;
 
-		/* Always add ALL policies, if they exist. */
-		if (policy->polcmd == '*' &&
-			check_role_for_policy(policy->roles, user_id))
-			policies = lcons(policy, policies);
+		/* Check whether the policy applies to the specified command type */
+		if (policy->polcmd == '*')
+			cmd_matches = true;
+		else
+		{
+			switch (cmd)
+			{
+				case CMD_SELECT:
+					if (policy->polcmd == ACL_SELECT_CHR)
+						cmd_matches = true;
+					break;
+				case CMD_INSERT:
+					if (policy->polcmd == ACL_INSERT_CHR)
+						cmd_matches = true;
+					break;
+				case CMD_UPDATE:
+					if (policy->polcmd == ACL_UPDATE_CHR)
+						cmd_matches = true;
+					break;
+				case CMD_DELETE:
+					if (policy->polcmd == ACL_DELETE_CHR)
+						cmd_matches = true;
+					break;
+				default:
+					elog(ERROR, "unrecognized policy command type %d", (int) cmd);
+					break;
+			}
+		}
 
-		/* Add relevant command-specific policies to the list. */
-		switch (cmd)
+		if (cmd_matches)
 		{
-			case CMD_SELECT:
-				if (policy->polcmd == ACL_SELECT_CHR
-					&& check_role_for_policy(policy->roles, user_id))
-					policies = lcons(policy, policies);
-				break;
-			case CMD_INSERT:
-				/* If INSERT then only need to add the WITH CHECK qual */
-				if (policy->polcmd == ACL_INSERT_CHR
-					&& check_role_for_policy(policy->roles, user_id))
-					policies = lcons(policy, policies);
-				break;
-			case CMD_UPDATE:
-				if (policy->polcmd == ACL_UPDATE_CHR
-					&& check_role_for_policy(policy->roles, user_id))
-					policies = lcons(policy, policies);
-				break;
-			case CMD_DELETE:
-				if (policy->polcmd == ACL_DELETE_CHR
-					&& check_role_for_policy(policy->roles, user_id))
-					policies = lcons(policy, policies);
-				break;
-			default:
-				elog(ERROR, "unrecognized policy command type %d", (int) cmd);
-				break;
+			/*
+			 * Add this policy to the list of permissive policies if it
+			 * applies to the specified role
+			 */
+			if (check_role_for_policy(policy->roles, user_id))
+				*permissive_policies = lappend(*permissive_policies, policy);
 		}
 	}
 
 	/*
-	 * There should always be a policy applied.  If there are none found then
-	 * create a simply defauly-deny policy (might be that policies exist but
-	 * that none of them apply to the role which is querying the table).
+	 * Then add any permissive and restrictive policies defined by extensions.
+	 * These are simply appended to the lists of internal policies, if they
+	 * apply to the specified role.
 	 */
-	if (policies == NIL)
+	if (row_security_policy_hook_restrictive)
 	{
-		RowSecurityPolicy *policy = NULL;
-		Datum		role;
+		List	   *hook_policies = (*row_security_policy_hook_restrictive) (cmd, relation);
 
-		role = ObjectIdGetDatum(ACL_ID_PUBLIC);
+		/*
+		 * We sort restrictive policies by name so that any WCOs they generate
+		 * are checked in a well-defined order.
+		 */
+		hook_policies = sort_policies_by_name(hook_policies);
 
-		policy = palloc0(sizeof(RowSecurityPolicy));
-		policy->policy_name = pstrdup("default-deny policy");
-		policy->policy_id = InvalidOid;
-		policy->polcmd = '*';
-		policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true,
-										'i');
-		policy->qual = (Expr *) makeConst(BOOLOID, -1, InvalidOid,
-										  sizeof(bool), BoolGetDatum(false),
-										  false, true);
-		policy->with_check_qual = copyObject(policy->qual);
-		policy->hassublinks = false;
+		foreach(item, hook_policies)
+		{
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
 
-		policies = list_make1(policy);
+			if (check_role_for_policy(policy->roles, user_id))
+				*restrictive_policies = lappend(*restrictive_policies, policy);
+		}
 	}
 
-	Assert(policies != NIL);
+	if (row_security_policy_hook_permissive)
+	{
+		List	   *hook_policies = (*row_security_policy_hook_permissive) (cmd, relation);
+
+		foreach(item, hook_policies)
+		{
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+
+			if (check_role_for_policy(policy->roles, user_id))
+				*permissive_policies = lappend(*permissive_policies, policy);
+		}
+	}
+}
+
+/*
+ * sort_policies_by_name
+ *
+ * This is only used for restrictive policies, ensuring that any
+ * WithCheckOptions they generate are applied in a well-defined order.
+ * This is not necessary for permissive policies, since they are all "OR"d
+ * together into a single WithCheckOption check.
+ */
+static List *
+sort_policies_by_name(List *policies)
+{
+	int			npol = list_length(policies);
+	RowSecurityPolicy *pols;
+	ListCell   *item;
+	int			ii = 0;
+
+	if (npol <= 1)
+		return policies;
+
+	pols = (RowSecurityPolicy *) palloc(sizeof(RowSecurityPolicy) * npol);
+
+	foreach(item, policies)
+	{
+		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+		pols[ii++] = *policy;
+	}
+
+	qsort(pols, npol, sizeof(RowSecurityPolicy), row_security_policy_cmp);
+
+	policies = NIL;
+	for (ii = 0; ii < npol; ii++)
+		policies = lappend(policies, &pols[ii]);
 
 	return policies;
 }
 
 /*
- * process_policies
+ * qsort comparator to sort RowSecurityPolicy entries by name
+ */
+static int
+row_security_policy_cmp(const void *a, const void *b)
+{
+	const RowSecurityPolicy *pa = (const RowSecurityPolicy *) a;
+	const RowSecurityPolicy *pb = (const RowSecurityPolicy *) b;
+
+	/* Guard against NULL policy names from extensions */
+	if (pa->policy_name == NULL)
+		return pb->policy_name == NULL ? 0 : 1;
+	if (pb->policy_name == NULL)
+		return -1;
+
+	return strcmp(pa->policy_name, pb->policy_name);
+}
+
+/*
+ * build_security_quals
  *
- * This will step through the policies which are passed in (which would come
- * from either the built-in ones created on a table, or from policies provided
- * by an extension through the hook provided), work out how to combine them,
- * rewrite them as necessary, and produce an Expr for the normal security
- * quals and an Expr for the with check quals.
+ * Build security quals to enforce the specified RLS policies, restricting
+ * access to existing data in a table.  If there are no policies controlling
+ * access to the table, then all access is prohibited --- i.e., an implicit
+ * default-deny policy is used.
  *
- * qual_eval, with_check_eval, and hassublinks are output variables
+ * New security quals are added to securityQuals, and hasSubLinks is set to
+ * true if any of the quals added contain sublink subqueries.
  */
 static void
-process_policies(Query *root, List *policies, int rt_index, Expr **qual_eval,
-				 Expr **with_check_eval, bool *hassublinks,
-				 BoolExprType boolop)
+build_security_quals(int rt_index,
+					 List *permissive_policies,
+					 List *restrictive_policies,
+					 List **securityQuals,
+					 bool *hasSubLinks)
 {
 	ListCell   *item;
-	List	   *quals = NIL;
-	List	   *with_check_quals = NIL;
+	List	   *permissive_quals;
+	Expr	   *rowsec_expr;
 
 	/*
-	 * Extract the USING and WITH CHECK quals from each of the policies and
-	 * add them to our lists.  We only want WITH CHECK quals if this RTE is
-	 * the query's result relation.
+	 * First deal with any permissive policies.  This adds a single security
+	 * qual "OR"ing together the USING clauses from all the permissive
+	 * policies.  If there are no permissive policy clauses granting access to
+	 * the table, an always-false clause is added (a default-deny policy).
 	 */
-	foreach(item, policies)
+	permissive_quals = NIL;
+
+	foreach(item, permissive_policies)
 	{
 		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
 
 		if (policy->qual != NULL)
-			quals = lcons(copyObject(policy->qual), quals);
+		{
+			permissive_quals = lappend(permissive_quals,
+									   copyObject(policy->qual));
+			*hasSubLinks |= policy->hassublinks;
+		}
+	}
 
-		if (policy->with_check_qual != NULL &&
-			rt_index == root->resultRelation)
-			with_check_quals = lcons(copyObject(policy->with_check_qual),
-									 with_check_quals);
+	if (permissive_quals == NIL)
+	{
+		/* Default deny policy (and skip any restrictive policies) */
+		*securityQuals = lappend(*securityQuals,
+								 makeConst(BOOLOID, -1, InvalidOid,
+										   sizeof(bool), BoolGetDatum(false),
+										   false, true));
+	}
+	else
+	{
+		/* OR together the permissive policy clauses */
+		if (list_length(permissive_quals) == 1)
+			rowsec_expr = (Expr *) linitial(permissive_quals);
+		else
+			rowsec_expr = makeBoolExpr(OR_EXPR, permissive_quals, -1);
+
+		ChangeVarNodes((Node *) rowsec_expr, 1, rt_index, 0);
+		*securityQuals = lappend(*securityQuals, rowsec_expr);
 
 		/*
-		 * For each policy, if there is only a USING clause then copy/use it
-		 * for the WITH CHECK policy also, if this RTE is the query's result
-		 * relation.
+		 * Then add security quals based on the USING clauses from any
+		 * restrictive policies.  These are effectively "AND"d together, so we
+		 * can just add them one at a time.
 		 */
-		if (policy->qual != NULL && policy->with_check_qual == NULL &&
-			rt_index == root->resultRelation)
-			with_check_quals = lcons(copyObject(policy->qual),
-									 with_check_quals);
+		foreach(item, restrictive_policies)
+		{
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+			Expr	   *qual;
 
+			if (policy->qual != NULL)
+			{
+				qual = copyObject(policy->qual);
+				ChangeVarNodes((Node *) qual, 1, rt_index, 0);
 
-		if (policy->hassublinks)
-			*hassublinks = true;
+				*securityQuals = lappend(*securityQuals, qual);
+				*hasSubLinks |= policy->hassublinks;
+			}
+		}
 	}
+}
 
-	/*
-	 * If we end up without any normal quals (perhaps the only policy matched
-	 * was for INSERT), then create a single all-false one.
-	 */
-	if (quals == NIL)
-		quals = lcons(makeConst(BOOLOID, -1, InvalidOid, sizeof(bool),
-								BoolGetDatum(false), false, true), quals);
+/*
+ * build_with_check_options
+ *
+ * Build WithCheckOptions of the specified kind to check that new records
+ * added by an INSERT or UPDATE are consistent with the specified RLS
+ * policies.  Normally new data must satisfy the WITH CHECK clauses from the
+ * policies.  If a policy has no explicit WITH CHECK clause, its USING clause
+ * is used instead.  In the special case of an UPDATE arising from an
+ * INSERT ... ON CONFLICT DO UPDATE, existing records are first checked using
+ * a WCO_RLS_CONFLICT_CHECK WithCheckOption, which always uses the USING
+ * clauses from RLS policies.
+ *
+ * New WCOs are added to withCheckOptions, and hasSubLinks is set to true if
+ * any of the check clauses added contain sublink subqueries.
+ */
+static void
+build_with_check_options(Relation rel,
+						 int rt_index,
+						 WCOKind kind,
+						 List *permissive_policies,
+						 List *restrictive_policies,
+						 List **withCheckOptions,
+						 bool *hasSubLinks)
+{
+	ListCell   *item;
+	List	   *permissive_quals;
+	WithCheckOption *wco;
+	Expr	   *rowsec_expr;
+
+#define QUAL_FOR_WCO(policy) \
+	( kind != WCO_RLS_CONFLICT_CHECK &&	\
+	  (policy)->with_check_qual != NULL ? \
+	  (policy)->with_check_qual : (policy)->qual )
 
 	/*
-	 * Row security quals always have the target table as varno 1, as no joins
-	 * are permitted in row security expressions. We must walk the expression,
-	 * updating any references to varno 1 to the varno the table has in the
-	 * outer query.
-	 *
-	 * We rewrite the expression in-place.
-	 *
-	 * We must have some quals at this point; the default-deny policy, if
-	 * nothing else.  Note that we might not have any WITH CHECK quals- that's
-	 * fine, as this might not be the resultRelation.
+	 * First deal with any permissive policies.  This adds a single
+	 * WithCheckOption "OR"ing together all the permissive policy clauses.
+	 * This check has no policy name, since if the check fails it means that
+	 * no policy granted permission to perform the update, rather than any
+	 * particular policy being violated.  If there are no permissive policy
+	 * clauses granting permission to add new data, a single always-false
+	 * check is added (a default-deny policy).
 	 */
-	Assert(quals != NIL);
+	permissive_quals = NIL;
 
-	ChangeVarNodes((Node *) quals, 1, rt_index, 0);
+	foreach(item, permissive_policies)
+	{
+		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+		Expr	   *qual = QUAL_FOR_WCO(policy);
 
-	if (with_check_quals != NIL)
-		ChangeVarNodes((Node *) with_check_quals, 1, rt_index, 0);
+		if (qual != NULL)
+		{
+			permissive_quals = lappend(permissive_quals, copyObject(qual));
+			*hasSubLinks |= policy->hassublinks;
+		}
+	}
 
-	/*
-	 * If more than one security qual is returned, then they need to be
-	 * combined together.
-	 */
-	if (list_length(quals) > 1)
-		*qual_eval = makeBoolExpr(boolop, quals, -1);
-	else
-		*qual_eval = (Expr *) linitial(quals);
+	if (permissive_quals == NIL)
+	{
+		/* Default deny policy (and skip any restrictive policies) */
+		wco = (WithCheckOption *) makeNode(WithCheckOption);
+		wco->kind = kind;
+		wco->relname = pstrdup(RelationGetRelationName(rel));
+		wco->polname = NULL;
+		wco->qual = (Node *) makeConst(BOOLOID, -1, InvalidOid,
+									   sizeof(bool), BoolGetDatum(false),
+									   false, true);
+		wco->cascaded = false;
 
-	/*
-	 * Similarly, if more than one WITH CHECK qual is returned, then they need
-	 * to be combined together.
-	 *
-	 * with_check_quals is allowed to be NIL here since this might not be the
-	 * resultRelation (see above).
-	 */
-	if (list_length(with_check_quals) > 1)
-		*with_check_eval = makeBoolExpr(boolop, with_check_quals, -1);
-	else if (with_check_quals != NIL)
-		*with_check_eval = (Expr *) linitial(with_check_quals);
+		*withCheckOptions = lappend(*withCheckOptions, wco);
+	}
 	else
-		*with_check_eval = NULL;
+	{
+		/* OR together the permissive policy clauses */
+		if (list_length(permissive_quals) == 1)
+			rowsec_expr = (Expr *) linitial(permissive_quals);
+		else
+			rowsec_expr = makeBoolExpr(OR_EXPR, permissive_quals, -1);
 
-	return;
+		ChangeVarNodes((Node *) rowsec_expr, 1, rt_index, 0);
+
+		wco = (WithCheckOption *) makeNode(WithCheckOption);
+		wco->kind = kind;
+		wco->relname = pstrdup(RelationGetRelationName(rel));
+		wco->polname = NULL;
+		wco->qual = (Node *) rowsec_expr;
+		wco->cascaded = false;
+
+		*withCheckOptions = lappend(*withCheckOptions, wco);
+
+		/*
+		 * Then add WithCheckOptions for each of the restrictive policy
+		 * clauses (which need to be "AND"d together).  We use a separate
+		 * WithCheckOption for each restrictive policy to allow the policy
+		 * name to be included in error reports if the policy is violated.
+		 */
+		foreach(item, restrictive_policies)
+		{
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+			Expr	   *qual = QUAL_FOR_WCO(policy);
+
+			if (qual != NULL)
+			{
+				qual = copyObject(qual);
+				ChangeVarNodes((Node *) qual, 1, rt_index, 0);
+
+				wco = (WithCheckOption *) makeNode(WithCheckOption);
+				wco->kind = kind;
+				wco->relname = pstrdup(RelationGetRelationName(rel));
+				wco->polname = pstrdup(policy->policy_name);
+				wco->qual = (Node *) qual;
+				wco->cascaded = false;
+
+				*withCheckOptions = lappend(*withCheckOptions, wco);
+				*hasSubLinks |= policy->hassublinks;
+			}
+		}
+	}
 }
 
 /*
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
new file mode 100644
index 420ef3d..9c3d096
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -859,8 +859,6 @@ equalPolicy(RowSecurityPolicy *policy1,
 		if (policy2 == NULL)
 			return false;
 
-		if (policy1->policy_id != policy2->policy_id)
-			return false;
 		if (policy1->polcmd != policy2->polcmd)
 			return false;
 		if (policy1->hassublinks != policy2->hassublinks)
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
new file mode 100644
index f0dcd2f..940cc32
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -928,6 +928,7 @@ typedef struct WithCheckOption
 	NodeTag		type;
 	WCOKind		kind;			/* kind of WCO */
 	char	   *relname;		/* name of relation that specified the WCO */
+	char	   *polname;		/* name of RLS policy being checked */
 	Node	   *qual;			/* constraint qual to check */
 	bool		cascaded;		/* true for a cascaded WCO on a view */
 } WithCheckOption;
diff --git a/src/include/rewrite/rowsecurity.h b/src/include/rewrite/rowsecurity.h
new file mode 100644
index 523c56e..4af244d
--- a/src/include/rewrite/rowsecurity.h
+++ b/src/include/rewrite/rowsecurity.h
@@ -19,7 +19,6 @@
 
 typedef struct RowSecurityPolicy
 {
-	Oid			policy_id;		/* OID of the policy */
 	char	   *policy_name;	/* Name of the policy */
 	char		polcmd;			/* Type of command policy is for */
 	ArrayType  *roles;			/* Array of roles policy is for */
@@ -41,7 +40,7 @@ extern PGDLLIMPORT row_security_policy_h
 
 extern PGDLLIMPORT row_security_policy_hook_type row_security_policy_hook_restrictive;
 
-extern void get_row_security_policies(Query *root, CmdType commandType,
+extern void get_row_security_policies(Query *root,
 						  RangeTblEntry *rte, int rt_index,
 						  List **securityQuals, List **withCheckOptions,
 						  bool *hasRowSecurity, bool *hasSubLinks);
diff --git a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
new file mode 100644
index 4587eb0..fb8c425
--- a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
+++ b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
@@ -83,7 +83,7 @@ SELECT * FROM rls_test_restrictive;
 INSERT INTO rls_test_restrictive VALUES ('r1','s1',10);
 -- failure
 INSERT INTO rls_test_restrictive VALUES ('r4','s4',10);
-ERROR:  new row violates row level security policy for "rls_test_restrictive"
+ERROR:  new row violates row level security policy "extension policy" for "rls_test_restrictive"
 SET ROLE s1;
 -- With only the hook's policies, both
 -- permissive hook's policy is current_user = username
@@ -93,7 +93,7 @@ EXPLAIN (costs off) SELECT * FROM rls_te
                                   QUERY PLAN                                   
 -------------------------------------------------------------------------------
  Seq Scan on rls_test_both
-   Filter: ((supervisor = "current_user"()) AND (username = "current_user"()))
+   Filter: ((username = "current_user"()) AND (supervisor = "current_user"()))
 (2 rows)
 
 SELECT * FROM rls_test_both;
@@ -124,7 +124,7 @@ EXPLAIN (costs off) SELECT * FROM rls_te
                           QUERY PLAN                           
 ---------------------------------------------------------------
  Seq Scan on rls_test_permissive
-   Filter: (("current_user"() = username) OR ((data % 2) = 0))
+   Filter: (((data % 2) = 0) OR ("current_user"() = username))
 (2 rows)
 
 SELECT * FROM rls_test_permissive;
@@ -145,13 +145,11 @@ ERROR:  new row violates row level secur
 SET ROLE s1;
 -- With both internal and hook policies, restrictive
 EXPLAIN (costs off) SELECT * FROM rls_test_restrictive;
-                          QUERY PLAN                           
----------------------------------------------------------------
- Subquery Scan on rls_test_restrictive
-   Filter: ((rls_test_restrictive.data % 2) = 0)
-   ->  Seq Scan on rls_test_restrictive rls_test_restrictive_1
-         Filter: ("current_user"() = supervisor)
-(4 rows)
+                            QUERY PLAN                            
+------------------------------------------------------------------
+ Seq Scan on rls_test_restrictive
+   Filter: (((data % 2) = 0) AND ("current_user"() = supervisor))
+(2 rows)
 
 SELECT * FROM rls_test_restrictive;
  username | supervisor | data 
@@ -163,7 +161,7 @@ SELECT * FROM rls_test_restrictive;
 INSERT INTO rls_test_restrictive VALUES ('r1','s1',8);
 -- failure
 INSERT INTO rls_test_restrictive VALUES ('r3','s3',10);
-ERROR:  new row violates row level security policy for "rls_test_restrictive"
+ERROR:  new row violates row level security policy "extension policy" for "rls_test_restrictive"
 -- failure
 INSERT INTO rls_test_restrictive VALUES ('r1','s1',7);
 ERROR:  new row violates row level security policy for "rls_test_restrictive"
@@ -173,13 +171,11 @@ ERROR:  new row violates row level secur
 -- With both internal and hook policies, both permissive
 -- and restrictive hook policies
 EXPLAIN (costs off) SELECT * FROM rls_test_both;
-                                        QUERY PLAN                                         
--------------------------------------------------------------------------------------------
- Subquery Scan on rls_test_both
-   Filter: (("current_user"() = rls_test_both.username) OR ((rls_test_both.data % 2) = 0))
-   ->  Seq Scan on rls_test_both rls_test_both_1
-         Filter: ("current_user"() = supervisor)
-(4 rows)
+                                             QUERY PLAN                                              
+-----------------------------------------------------------------------------------------------------
+ Seq Scan on rls_test_both
+   Filter: (("current_user"() = supervisor) AND (((data % 2) = 0) OR ("current_user"() = username)))
+(2 rows)
 
 SELECT * FROM rls_test_both;
  username | supervisor | data 
@@ -190,7 +186,7 @@ SELECT * FROM rls_test_both;
 INSERT INTO rls_test_both VALUES ('r1','s1',8);
 -- failure
 INSERT INTO rls_test_both VALUES ('r3','s3',10);
-ERROR:  new row violates row level security policy for "rls_test_both"
+ERROR:  new row violates row level security policy "extension policy" for "rls_test_both"
 -- failure
 INSERT INTO rls_test_both VALUES ('r1','s1',7);
 ERROR:  new row violates row level security policy for "rls_test_both"
diff --git a/src/test/modules/test_rls_hooks/test_rls_hooks.c b/src/test/modules/test_rls_hooks/test_rls_hooks.c
new file mode 100644
index b96dbff..cc865cd
--- a/src/test/modules/test_rls_hooks/test_rls_hooks.c
+++ b/src/test/modules/test_rls_hooks/test_rls_hooks.c
@@ -87,7 +87,6 @@ test_rls_hooks_permissive(CmdType cmdtyp
 	role = ObjectIdGetDatum(ACL_ID_PUBLIC);
 
 	policy->policy_name = pstrdup("extension policy");
-	policy->policy_id = InvalidOid;
 	policy->polcmd = '*';
 	policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true, 'i');
 
@@ -151,7 +150,6 @@ test_rls_hooks_restrictive(CmdType cmdty
 	role = ObjectIdGetDatum(ACL_ID_PUBLIC);
 
 	policy->policy_name = pstrdup("extension policy");
-	policy->policy_id = InvalidOid;
 	policy->polcmd = '*';
 	policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true, 'i');
 

From 0b632073f073ea9ae7708e7756e444777a8bca49 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Sun, 13 Sep 2015 09:03:09 -0400
Subject: [PATCH] RLS refactoring

This refactors rewrite/rowsecurity.c to simplify the handling of the
default deny case (reducing the number of places where we check for and
add the default deny policy from three to one) by splitting up the
retrival of the policies from the application of them.

This also allowed us to do away with the policy_id field.  A policy_name
field was added for WithCheckOption policies and is used in error
reporting, when available.

Lastly, this also handles the UPDATE/DELETE RETURNING case by filtering
out the records which are not visible to the user through ALL or SELECT
policies from those considered for UPDATE or DELETE.  This is similar to
how the GRANT system works, which prevents RETURNING unless the caller
has SELECT rights on the relation.

Per discussion with Robert, Dean, Tom, and Kevin.

Patch by Dean Rasheed, modifications for UPDATE/DELETE RETURNING by me,
along with various other mostly cosmetic changes.

Back-patch to 9.5 where RLS was introduced to avoid unnecessary
differences, since we're still in alpha, per discussion with Robert.
---
 src/backend/commands/policy.c                      |  41 -
 src/backend/executor/execMain.c                    |  20 +-
 src/backend/nodes/copyfuncs.c                      |   1 +
 src/backend/nodes/equalfuncs.c                     |   1 +
 src/backend/nodes/outfuncs.c                       |   1 +
 src/backend/nodes/readfuncs.c                      |   1 +
 src/backend/rewrite/rewriteHandler.c               |   5 +-
 src/backend/rewrite/rowsecurity.c                  | 973 +++++++++++++--------
 src/backend/utils/cache/relcache.c                 |   2 -
 src/include/nodes/parsenodes.h                     |   1 +
 src/include/rewrite/rowsecurity.h                  |   5 +-
 .../test_rls_hooks/expected/test_rls_hooks.out     |  10 +-
 src/test/modules/test_rls_hooks/test_rls_hooks.c   |   2 -
 src/test/regress/expected/rowsecurity.out          | 213 ++---
 src/test/regress/sql/rowsecurity.sql               |  76 +-
 15 files changed, 759 insertions(+), 593 deletions(-)

diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 45326a3..8851fe7 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -186,9 +186,6 @@ policy_role_list_to_array(List *roles, int *num_roles)
 /*
  * Load row security policy from the catalog, and store it in
  * the relation's relcache entry.
- *
- * We will always set up some kind of policy here.  If no explicit policies
- * are found then an implicit default-deny policy is created.
  */
 void
 RelationBuildRowSecurity(Relation relation)
@@ -246,7 +243,6 @@ RelationBuildRowSecurity(Relation relation)
 			char	   *with_check_value;
 			Expr	   *with_check_qual;
 			char	   *policy_name_value;
-			Oid			policy_id;
 			bool		isnull;
 			RowSecurityPolicy *policy;
 
@@ -298,14 +294,11 @@ RelationBuildRowSecurity(Relation relation)
 			else
 				with_check_qual = NULL;
 
-			policy_id = HeapTupleGetOid(tuple);
-
 			/* Now copy everything into the cache context */
 			MemoryContextSwitchTo(rscxt);
 
 			policy = palloc0(sizeof(RowSecurityPolicy));
 			policy->policy_name = pstrdup(policy_name_value);
-			policy->policy_id = policy_id;
 			policy->polcmd = cmd_value;
 			policy->roles = DatumGetArrayTypePCopy(roles_datum);
 			policy->qual = copyObject(qual_expr);
@@ -326,40 +319,6 @@ RelationBuildRowSecurity(Relation relation)
 
 		systable_endscan(sscan);
 		heap_close(catalog, AccessShareLock);
-
-		/*
-		 * Check if no policies were added
-		 *
-		 * If no policies exist in pg_policy for this relation, then we need
-		 * to create a single default-deny policy.  We use InvalidOid for the
-		 * Oid to indicate that this is the default-deny policy (we may decide
-		 * to ignore the default policy if an extension adds policies).
-		 */
-		if (rsdesc->policies == NIL)
-		{
-			RowSecurityPolicy *policy;
-			Datum		role;
-
-			MemoryContextSwitchTo(rscxt);
-
-			role = ObjectIdGetDatum(ACL_ID_PUBLIC);
-
-			policy = palloc0(sizeof(RowSecurityPolicy));
-			policy->policy_name = pstrdup("default-deny policy");
-			policy->policy_id = InvalidOid;
-			policy->polcmd = '*';
-			policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true,
-											'i');
-			policy->qual = (Expr *) makeConst(BOOLOID, -1, InvalidOid,
-										   sizeof(bool), BoolGetDatum(false),
-											  false, true);
-			policy->with_check_qual = copyObject(policy->qual);
-			policy->hassublinks = false;
-
-			rsdesc->policies = lcons(policy, rsdesc->policies);
-
-			MemoryContextSwitchTo(oldcxt);
-		}
 	}
 	PG_CATCH();
 	{
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 2c65a90..c28eb2b 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1815,14 +1815,26 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
 					break;
 				case WCO_RLS_INSERT_CHECK:
 				case WCO_RLS_UPDATE_CHECK:
-					ereport(ERROR,
-							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					if (wco->polname != NULL)
+						ereport(ERROR,
+								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+							 errmsg("new row violates row level security policy \"%s\" for \"%s\"",
+									wco->polname, wco->relname)));
+					else
+						ereport(ERROR,
+								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 							 errmsg("new row violates row level security policy for \"%s\"",
 									wco->relname)));
 					break;
 				case WCO_RLS_CONFLICT_CHECK:
-					ereport(ERROR,
-							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					if (wco->polname != NULL)
+						ereport(ERROR,
+								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+							 errmsg("new row violates row level security policy \"%s\" (USING expression) for \"%s\"",
+									wco->polname, wco->relname)));
+					else
+						ereport(ERROR,
+								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 							 errmsg("new row violates row level security policy (USING expression) for \"%s\"",
 									wco->relname)));
 					break;
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index bd2e80e..1c801f5 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2168,6 +2168,7 @@ _copyWithCheckOption(const WithCheckOption *from)
 
 	COPY_SCALAR_FIELD(kind);
 	COPY_STRING_FIELD(relname);
+	COPY_STRING_FIELD(polname);
 	COPY_NODE_FIELD(qual);
 	COPY_SCALAR_FIELD(cascaded);
 
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 19412fe..8f16833 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2455,6 +2455,7 @@ _equalWithCheckOption(const WithCheckOption *a, const WithCheckOption *b)
 {
 	COMPARE_SCALAR_FIELD(kind);
 	COMPARE_STRING_FIELD(relname);
+	COMPARE_STRING_FIELD(polname);
 	COMPARE_NODE_FIELD(qual);
 	COMPARE_SCALAR_FIELD(cascaded);
 
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index a878498..79b7179 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2403,6 +2403,7 @@ _outWithCheckOption(StringInfo str, const WithCheckOption *node)
 
 	WRITE_ENUM_FIELD(kind, WCOKind);
 	WRITE_STRING_FIELD(relname);
+	WRITE_STRING_FIELD(polname);
 	WRITE_NODE_FIELD(qual);
 	WRITE_BOOL_FIELD(cascaded);
 }
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 23e0b36..df55b76 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -270,6 +270,7 @@ _readWithCheckOption(void)
 
 	READ_ENUM_FIELD(kind, WCOKind);
 	READ_STRING_FIELD(relname);
+	READ_STRING_FIELD(polname);
 	READ_NODE_FIELD(qual);
 	READ_BOOL_FIELD(cascaded);
 
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index db3c2c7..1b8e7b0 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1786,8 +1786,8 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
 		/*
 		 * Fetch any new security quals that must be applied to this RTE.
 		 */
-		get_row_security_policies(parsetree, parsetree->commandType, rte,
-								  rt_index, &securityQuals, &withCheckOptions,
+		get_row_security_policies(parsetree, rte, rt_index,
+								  &securityQuals, &withCheckOptions,
 								  &hasRowSecurity, &hasSubLinks);
 
 		if (securityQuals != NIL || withCheckOptions != NIL)
@@ -3026,6 +3026,7 @@ rewriteTargetView(Query *parsetree, Relation view)
 			wco = makeNode(WithCheckOption);
 			wco->kind = WCO_VIEW_CHECK;
 			wco->relname = pstrdup(RelationGetRelationName(view));
+			wco->polname = NULL;
 			wco->qual = NULL;
 			wco->cascaded = cascaded;
 
diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index 5a81db3..6b08405 100644
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -13,11 +13,12 @@
  * Any part of the system which is returning records back to the user, or
  * which is accepting records from the user to add to a table, needs to
  * consider the policies associated with the table (if any).  For normal
- * queries, this is handled by calling prepend_row_security_policies() during
- * rewrite, which looks at each RTE and adds the expressions defined by the
- * policies to the securityQuals list for the RTE.  For queries which modify
- * the relation, any WITH CHECK policies are added to the list of
- * WithCheckOptions for the Query and checked against each row which is being
+ * queries, this is handled by calling get_row_security_policies() during
+ * rewrite, for each RTE in the query.  This returns the expressions defined
+ * by the table's policies as a list that is prepended to the securityQuals
+ * list for the RTE.  For queries which modify the table, any WITH CHECK
+ * clauses from the table's policies are also returned and prepended to the
+ * list of WithCheckOptions for the Query to check each row that is being
  * added to the table.  Other parts of the system (eg: COPY) simply construct
  * a normal query and use that, if RLS is to be applied.
  *
@@ -56,13 +57,34 @@
 #include "utils/syscache.h"
 #include "tcop/utility.h"
 
-static List *pull_row_security_policies(CmdType cmd, Relation relation,
-						   Oid user_id);
-static void process_policies(Query *root, List *policies, int rt_index,
-				 Expr **final_qual,
-				 Expr **final_with_check_qual,
-				 bool *hassublinks,
-				 BoolExprType boolop);
+static void get_policies_for_relation(Relation relation,
+						  CmdType cmd, bool returning, Oid user_id,
+						  List **permissive_policies,
+						  List **restrictive_policies,
+						  List **returning_policies);
+
+static List *sort_policies_by_name(List *policies);
+
+static int row_security_policy_cmp(const void *a, const void *b);
+
+static void build_security_quals(int rt_index,
+					 List *permissive_policies,
+					 List *restrictive_policies,
+					 List *returning_policies,
+					 bool returning,
+					 List **securityQuals,
+					 bool *hasSubLinks);
+
+static void build_with_check_options(Relation rel,
+						 int rt_index,
+						 WCOKind kind,
+						 List *permissive_policies,
+						 List *restrictive_policies,
+						 List *returning_policies,
+						 bool returning,
+						 List **withCheckOptions,
+						 bool *hasSubLinks);
+
 static bool check_role_for_policy(ArrayType *policy_roles, Oid user_id);
 
 /*
@@ -74,41 +96,36 @@ static bool check_role_for_policy(ArrayType *policy_roles, Oid user_id);
  * row_security_policy_hook_restrictive can be used to add policies which
  * are enforced, regardless of other policies (they are "AND"d).
  *
- * See below where the hook is called in prepend_row_security_policies for
- * insight into how to use this hook.
+ * row_security_policy_hook_returning can be used to add restrictive policies
+ * which are added when a RETURNING is used with UPDATE or DELETE; this is used
+ * to ensure that rows to be updated or deleted and returned to the user are
+ * visibile to the user through an ALL or SELECT policy.
  */
 row_security_policy_hook_type row_security_policy_hook_permissive = NULL;
 row_security_policy_hook_type row_security_policy_hook_restrictive = NULL;
+row_security_policy_hook_type row_security_policy_hook_returning = NULL;
 
 /*
- * Get any row security quals and check quals that should be applied to the
- * specified RTE.
+ * Get any row security quals and WithCheckOption checks that should be
+ * applied to the specified RTE.
  *
  * In addition, hasRowSecurity is set to true if row level security is enabled
  * (even if this RTE doesn't have any row security quals), and hasSubLinks is
  * set to true if any of the quals returned contain sublinks.
  */
 void
-get_row_security_policies(Query *root, CmdType commandType, RangeTblEntry *rte,
-						  int rt_index, List **securityQuals,
-						  List **withCheckOptions, bool *hasRowSecurity,
-						  bool *hasSubLinks)
+get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
+						  List **securityQuals, List **withCheckOptions,
+						  bool *hasRowSecurity, bool *hasSubLinks)
 {
-	Expr	   *rowsec_expr = NULL;
-	Expr	   *rowsec_with_check_expr = NULL;
-	Expr	   *hook_expr_restrictive = NULL;
-	Expr	   *hook_with_check_expr_restrictive = NULL;
-	Expr	   *hook_expr_permissive = NULL;
-	Expr	   *hook_with_check_expr_permissive = NULL;
-
-	List	   *rowsec_policies;
-	List	   *hook_policies_restrictive = NIL;
-	List	   *hook_policies_permissive = NIL;
-
-	Relation	rel;
 	Oid			user_id;
 	int			rls_status;
-	bool		defaultDeny = false;
+	bool		returning;
+	Relation	rel;
+	CmdType		commandType;
+	List	   *permissive_policies;
+	List	   *restrictive_policies;
+	List	   *returning_policies;
 
 	/* Defaults for the return values */
 	*securityQuals = NIL;
@@ -157,465 +174,643 @@ get_row_security_policies(Query *root, CmdType commandType, RangeTblEntry *rte,
 	 * policies and t2's SELECT policies.
 	 */
 	rel = heap_open(rte->relid, NoLock);
-	if (rt_index != root->resultRelation)
-		commandType = CMD_SELECT;
 
-	rowsec_policies = pull_row_security_policies(commandType, rel,
-												 user_id);
+	commandType = rt_index == root->resultRelation ?
+				  root->commandType : CMD_SELECT;
 
 	/*
-	 * Check if this is only the default-deny policy.
+	 * For the target relation, when there is a returning list, we need to
+	 * collect up returning policies to pass to build_security_quals and
+	 * build_with_check_options.  This is because, for the RETURNING case, we
+	 * have to filter any records which are not visible through an ALL or SELECT
+	 * USING policy.
 	 *
-	 * Normally, if the table has row security enabled but there are no
-	 * policies, we use a default-deny policy and not allow anything. However,
-	 * when an extension uses the hook to add their own policies, we don't
-	 * want to include the default deny policy or there won't be any way for a
-	 * user to use an extension exclusively for the policies to be used.
-	 */
-	if (((RowSecurityPolicy *) linitial(rowsec_policies))->policy_id
-		== InvalidOid)
-		defaultDeny = true;
-
-	/* Now that we have our policies, build the expressions from them. */
-	process_policies(root, rowsec_policies, rt_index, &rowsec_expr,
-					 &rowsec_with_check_expr, hasSubLinks, OR_EXPR);
-
-	/*
-	 * Also, allow extensions to add their own policies.
-	 *
-	 * extensions can add either permissive or restrictive policies.
-	 *
-	 * Note that, as with the internal policies, if multiple policies are
-	 * returned then they will be combined into a single expression with all
-	 * of them OR'd (for permissive) or AND'd (for restrictive) together.
-	 *
-	 * If only a USING policy is returned by the extension then it will be
-	 * used for WITH CHECK as well, similar to how internal policies are
-	 * handled.
+	 * When returning is true, get_policies_for_relation will return the set of
+	 * ALL and SELECT USING policies in returning_policies, even though the
+	 * commandType isn't SELECT.
 	 *
-	 * The only caveat to this is that if there are NO internal policies
-	 * defined, there ARE policies returned by the extension, and RLS is
-	 * enabled on the table, then we will ignore the internally-generated
-	 * default-deny policy and use only the policies returned by the
-	 * extension.
+	 * We don't need to worry about the non-target relation case because we are
+	 * checking the ALL and SELECT policies for those relations anyway (see
+	 * above).
 	 */
-	if (row_security_policy_hook_restrictive)
-	{
-		hook_policies_restrictive = (*row_security_policy_hook_restrictive) (commandType, rel);
-
-		/* Build the expression from any policies returned. */
-		if (hook_policies_restrictive != NIL)
-			process_policies(root, hook_policies_restrictive, rt_index,
-							 &hook_expr_restrictive,
-							 &hook_with_check_expr_restrictive,
-							 hasSubLinks,
-							 AND_EXPR);
-	}
+	returning = rt_index == root->resultRelation && root->returningList != NIL;
 
-	if (row_security_policy_hook_permissive)
-	{
-		hook_policies_permissive = (*row_security_policy_hook_permissive) (commandType, rel);
-
-		/* Build the expression from any policies returned. */
-		if (hook_policies_permissive != NIL)
-			process_policies(root, hook_policies_permissive, rt_index,
-							 &hook_expr_permissive,
-							 &hook_with_check_expr_permissive, hasSubLinks,
-							 OR_EXPR);
-	}
+	get_policies_for_relation(rel, commandType, returning, user_id,
+							  &permissive_policies, &restrictive_policies,
+							  &returning_policies);
 
 	/*
-	 * If the only built-in policy is the default-deny one, and permissive hook
-	 * policies exist, then use the hook policies only and do not apply the
-	 * default-deny policy.  Otherwise, we will apply both sets below.
+	 * For SELECT, UPDATE and DELETE, build security quals to enforce these
+	 * policies.  These security quals control access to existing table rows.
+	 * Restrictive policies are "AND"d together, and permissive policies are
+	 * "OR"d together.
 	 *
-	 * Note that we do not remove the defaultDeny policy if only *restrictive*
-	 * policies exist as restrictive policies should only ever be reducing what
-	 * is visible.  Therefore, at least one permissive policy must exist which
-	 * allows records to be seen before restrictive policies can remove rows
-	 * from that set.  A single "true" policy can be created to address this
-	 * requirement, if necessary.
+	 * If there are no policy clauses controlling access to the table, this
+	 * will add a single always-false clause (a default-deny policy).
 	 */
-	if (defaultDeny && hook_policies_permissive != NIL)
-	{
-		rowsec_expr = NULL;
-		rowsec_with_check_expr = NULL;
-	}
+	if (commandType == CMD_SELECT ||
+		commandType == CMD_UPDATE ||
+		commandType == CMD_DELETE)
+		build_security_quals(rt_index,
+							 permissive_policies,
+							 restrictive_policies,
+							 returning_policies,
+							 returning,
+							 securityQuals,
+							 hasSubLinks);
 
 	/*
-	 * For INSERT or UPDATE, we need to add the WITH CHECK quals to Query's
-	 * withCheckOptions to verify that any new records pass the WITH CHECK
-	 * policy (this will be a copy of the USING policy, if no explicit WITH
-	 * CHECK policy exists).
+	 * For INSERT and UPDATE, add withCheckOptions to verify that any new
+	 * records added are consistent with the security policies.  This will use
+	 * each policy's WITH CHECK clause, or its USING clause if no explicit
+	 * WITH CHECK clause is defined.
 	 */
 	if (commandType == CMD_INSERT || commandType == CMD_UPDATE)
 	{
-		/*
-		 * WITH CHECK OPTIONS wants a WCO node which wraps each Expr, so
-		 * create them as necessary.
-		 */
+		/* This should be the target relation */
+		Assert(rt_index == root->resultRelation);
+
+		build_with_check_options(rel, rt_index,
+								 commandType == CMD_INSERT ?
+								 WCO_RLS_INSERT_CHECK : WCO_RLS_UPDATE_CHECK,
+								 permissive_policies,
+								 restrictive_policies,
+								 NULL, false,
+								 withCheckOptions,
+								 hasSubLinks);
 
 		/*
-		 * Handle any restrictive policies first.
-		 *
-		 * They can simply be added.
+		 * For INSERT ... ON CONFLICT DO UPDATE we need additional policy
+		 * checks for the UPDATE which may be applied to the same RTE.
 		 */
-		if (hook_with_check_expr_restrictive)
+		if (commandType == CMD_INSERT &&
+			root->onConflict && root->onConflict->action == ONCONFLICT_UPDATE)
 		{
-			WithCheckOption *wco;
+			List	   *conflict_permissive_policies;
+			List	   *conflict_restrictive_policies;
+			List	   *conflict_returning_policies;
 
-			wco = (WithCheckOption *) makeNode(WithCheckOption);
-			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
-				WCO_RLS_UPDATE_CHECK;
-			wco->relname = pstrdup(RelationGetRelationName(rel));
-			wco->qual = (Node *) hook_with_check_expr_restrictive;
-			wco->cascaded = false;
-			*withCheckOptions = lappend(*withCheckOptions, wco);
+			/* Get the policies that apply to the auxiliary UPDATE */
+			get_policies_for_relation(rel, CMD_UPDATE, returning, user_id,
+									  &conflict_permissive_policies,
+									  &conflict_restrictive_policies,
+									  &conflict_returning_policies);
+
+			/*
+			 * Enforce the USING clauses of the UPDATE policies using WCOs
+			 * rather than security quals.  This ensures that an error is
+			 * raised if the conflicting row cannot be updated due to RLS,
+			 * rather than the change being silently dropped.
+			 *
+			 * The returning policies are also added here as WCO policies,
+			 * again, to avoid silently dropping data.
+			 */
+			build_with_check_options(rel, rt_index,
+									 WCO_RLS_CONFLICT_CHECK,
+									 conflict_permissive_policies,
+									 conflict_restrictive_policies,
+									 conflict_returning_policies,
+									 returning,
+									 withCheckOptions,
+									 hasSubLinks);
+
+			/* Enforce the WITH CHECK clauses of the UPDATE policies */
+			build_with_check_options(rel, rt_index,
+									 WCO_RLS_UPDATE_CHECK,
+									 conflict_permissive_policies,
+									 conflict_restrictive_policies,
+									 NULL, false,
+									 withCheckOptions,
+									 hasSubLinks);
 		}
+	}
 
-		/*
-		 * Handle built-in policies, if there are no permissive policies from
-		 * the hook.
-		 */
-		if (rowsec_with_check_expr && !hook_with_check_expr_permissive)
-		{
-			WithCheckOption *wco;
+	heap_close(rel, NoLock);
 
-			wco = (WithCheckOption *) makeNode(WithCheckOption);
-			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
-				WCO_RLS_UPDATE_CHECK;
-			wco->relname = pstrdup(RelationGetRelationName(rel));
-			wco->qual = (Node *) rowsec_with_check_expr;
-			wco->cascaded = false;
-			*withCheckOptions = lappend(*withCheckOptions, wco);
-		}
-		/* Handle the hook policies, if there are no built-in ones. */
-		else if (!rowsec_with_check_expr && hook_with_check_expr_permissive)
-		{
-			WithCheckOption *wco;
+	/*
+	 * Mark this query as having row security, so plancache can invalidate it
+	 * when necessary (eg: role changes)
+	 */
+	*hasRowSecurity = true;
 
-			wco = (WithCheckOption *) makeNode(WithCheckOption);
-			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
-				WCO_RLS_UPDATE_CHECK;
-			wco->relname = pstrdup(RelationGetRelationName(rel));
-			wco->qual = (Node *) hook_with_check_expr_permissive;
-			wco->cascaded = false;
-			*withCheckOptions = lappend(*withCheckOptions, wco);
-		}
-		/* Handle the case where there are both. */
-		else if (rowsec_with_check_expr && hook_with_check_expr_permissive)
-		{
-			WithCheckOption *wco;
-			List	   *combined_quals = NIL;
-			Expr	   *combined_qual_eval;
+	return;
+}
 
-			combined_quals = lcons(copyObject(rowsec_with_check_expr),
-								   combined_quals);
+/*
+ * get_policies_for_relation
+ *
+ * Returns lists of permissive, restrictive, and returning policies to be
+ * applied to the specified relation, based on the command type, if RETURNING
+ * is being used, and role.
+ *
+ * This includes any policies added by extensions.
+ */
+static void
+get_policies_for_relation(Relation relation,
+						  CmdType cmd, bool returning, Oid user_id,
+						  List **permissive_policies,
+						  List **restrictive_policies,
+						  List **returning_policies)
+{
+	ListCell   *item;
 
-			combined_quals = lcons(copyObject(hook_with_check_expr_permissive),
-								   combined_quals);
+	*permissive_policies = NIL;
+	*restrictive_policies = NIL;
+	*returning_policies = NIL;
 
-			combined_qual_eval = makeBoolExpr(OR_EXPR, combined_quals, -1);
+	/*
+	 * First find all internal policies for the relation.  CREATE POLICY does
+	 * not currently support defining restrictive policies, so for now all
+	 * internal policies are permissive.
+	 */
+	foreach(item, relation->rd_rsdesc->policies)
+	{
+		bool				cmd_matches = false;
+		bool				returning_matches = false;
+		RowSecurityPolicy  *policy = (RowSecurityPolicy *) lfirst(item);
 
-			wco = (WithCheckOption *) makeNode(WithCheckOption);
-			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
-				WCO_RLS_UPDATE_CHECK;
-			wco->relname = pstrdup(RelationGetRelationName(rel));
-			wco->qual = (Node *) combined_qual_eval;
-			wco->cascaded = false;
-			*withCheckOptions = lappend(*withCheckOptions, wco);
+		/* Always add ALL policies, if they exist. */
+		if (policy->polcmd == '*')
+		{
+			cmd_matches = true;
+			/* Also add to returning_policies, see discussion below. */
+			if (returning)
+				*returning_policies = lappend(*returning_policies,
+											  policy);
+		}
+		else
+		{
+			/*
+			 * Check whether the policy applies to the specified command type.
+			 *
+			 * Note that when RETURNING is used with UPDATE or DELETE, then
+			 * we collect up any ALL or SELECT policies which apply and return
+			 * them in returning_policies.  They will be combined together
+			 * using OR and then added as a restrictive policy, to ensure that
+			 * only rows visible through an ALL or SELECT USING policy are
+			 * visible to an UPDATE/DELETE RETURNING.
+			 */
+			switch (cmd)
+			{
+				case CMD_SELECT:
+					if (policy->polcmd == ACL_SELECT_CHR)
+						cmd_matches = true;
+					break;
+				case CMD_INSERT:
+					if (policy->polcmd == ACL_INSERT_CHR)
+						cmd_matches = true;
+					break;
+				case CMD_UPDATE:
+					if (policy->polcmd == ACL_UPDATE_CHR)
+						cmd_matches = true;
+					if (returning && policy->polcmd == ACL_SELECT_CHR)
+						returning_matches = true;
+					break;
+				case CMD_DELETE:
+					if (policy->polcmd == ACL_DELETE_CHR)
+						cmd_matches = true;
+					if (returning && policy->polcmd == ACL_SELECT_CHR)
+						returning_matches = true;
+					break;
+				default:
+					elog(ERROR, "unrecognized policy command type %d",
+						 (int) cmd);
+					break;
+			}
 		}
 
 		/*
-		 * ON CONFLICT DO UPDATE has an RTE that is subject to both INSERT and
-		 * UPDATE RLS enforcement.  Those are enforced (as a special, distinct
-		 * kind of WCO) on the target tuple.
-		 *
-		 * Make a second, recursive pass over the RTE for this, gathering
-		 * UPDATE-applicable RLS checks/WCOs, and gathering and converting
-		 * UPDATE-applicable security quals into WCO_RLS_CONFLICT_CHECK RLS
-		 * checks/WCOs.  Finally, these distinct kinds of RLS checks/WCOs are
-		 * concatenated with our own INSERT-applicable list.
+		 * Add this policy to the list of permissive policies if it
+		 * applies to the specified role
 		 */
-		if (root->onConflict && root->onConflict->action == ONCONFLICT_UPDATE &&
-			commandType == CMD_INSERT)
-		{
-			List	   *conflictSecurityQuals = NIL;
-			List	   *conflictWCOs = NIL;
-			ListCell   *item;
-			bool		conflictHasRowSecurity = false;
-			bool		conflictHasSublinks = false;
-
-			/* Assume that RTE is target resultRelation */
-			get_row_security_policies(root, CMD_UPDATE, rte, rt_index,
-									  &conflictSecurityQuals, &conflictWCOs,
-									  &conflictHasRowSecurity,
-									  &conflictHasSublinks);
-
-			if (conflictHasRowSecurity)
-				*hasRowSecurity = true;
-			if (conflictHasSublinks)
-				*hasSubLinks = true;
+		if (cmd_matches && check_role_for_policy(policy->roles, user_id))
+			*permissive_policies = lappend(*permissive_policies, policy);
 
-			/*
-			 * Append WITH CHECK OPTIONs/RLS checks, which should not conflict
-			 * between this INSERT and the auxiliary UPDATE
-			 */
-			*withCheckOptions = list_concat(*withCheckOptions,
-											conflictWCOs);
+		/* Ditto for returning policies. */
+		if (returning_matches && check_role_for_policy(policy->roles, user_id))
+			*returning_policies = lappend(*returning_policies, policy);
+	}
 
-			foreach(item, conflictSecurityQuals)
-			{
-				Expr	   *conflict_rowsec_expr = (Expr *) lfirst(item);
-				WithCheckOption *wco;
+	/*
+	 * Then add any permissive, restrictive, or returning policies defined by
+	 * extensions.  These are simply appended to the lists of internal policies,
+	 * if they apply to the specified role.
+	 */
+	if (row_security_policy_hook_restrictive)
+	{
+		List	   *hook_policies =
+			(*row_security_policy_hook_restrictive) (cmd, relation);
 
-				wco = (WithCheckOption *) makeNode(WithCheckOption);
+		/*
+		 * We sort restrictive policies by name so that any WCOs they generate
+		 * are checked in a well-defined order.
+		 */
+		hook_policies = sort_policies_by_name(hook_policies);
 
-				wco->kind = WCO_RLS_CONFLICT_CHECK;
-				wco->relname = pstrdup(RelationGetRelationName(rel));
-				wco->qual = (Node *) copyObject(conflict_rowsec_expr);
-				wco->cascaded = false;
-				*withCheckOptions = lappend(*withCheckOptions, wco);
-			}
+		foreach(item, hook_policies)
+		{
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+
+			if (check_role_for_policy(policy->roles, user_id))
+				*restrictive_policies = lappend(*restrictive_policies, policy);
 		}
 	}
 
-	/* For SELECT, UPDATE, and DELETE, set the security quals */
-	if (commandType == CMD_SELECT
-		|| commandType == CMD_UPDATE
-		|| commandType == CMD_DELETE)
+	if (row_security_policy_hook_permissive)
 	{
-		/* restrictive policies can simply be added to the list first */
-		if (hook_expr_restrictive)
-			*securityQuals = lappend(*securityQuals, hook_expr_restrictive);
+		List	   *hook_policies =
+			(*row_security_policy_hook_permissive) (cmd, relation);
 
-		/* If we only have internal permissive, then just add those */
-		if (rowsec_expr && !hook_expr_permissive)
-			*securityQuals = lappend(*securityQuals, rowsec_expr);
-		/* .. and if we have only permissive policies from the hook */
-		else if (!rowsec_expr && hook_expr_permissive)
-			*securityQuals = lappend(*securityQuals, hook_expr_permissive);
-		/* if we have both, we have to combine them with an OR */
-		else if (rowsec_expr && hook_expr_permissive)
+		foreach(item, hook_policies)
 		{
-			List	   *combined_quals = NIL;
-			Expr	   *combined_qual_eval;
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
 
-			combined_quals = lcons(copyObject(rowsec_expr), combined_quals);
-			combined_quals = lcons(copyObject(hook_expr_permissive),
-								   combined_quals);
+			if (check_role_for_policy(policy->roles, user_id))
+				*permissive_policies = lappend(*permissive_policies, policy);
+		}
+	}
 
-			combined_qual_eval = makeBoolExpr(OR_EXPR, combined_quals, -1);
+	if (returning && row_security_policy_hook_returning)
+	{
+		List	   *hook_policies =
+			(*row_security_policy_hook_returning) (cmd, relation);
+
+		foreach(item, hook_policies)
+		{
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
 
-			*securityQuals = lappend(*securityQuals, combined_qual_eval);
+			if (check_role_for_policy(policy->roles, user_id))
+				*returning_policies = lappend(*returning_policies, policy);
 		}
 	}
+}
 
-	heap_close(rel, NoLock);
+/*
+ * sort_policies_by_name
+ *
+ * This is only used for restrictive policies, ensuring that any
+ * WithCheckOptions they generate are applied in a well-defined order.
+ * This is not necessary for permissive policies, since they are all "OR"d
+ * together into a single WithCheckOption check.
+ */
+static List *
+sort_policies_by_name(List *policies)
+{
+	int			npol = list_length(policies);
+	RowSecurityPolicy *pols;
+	ListCell   *item;
+	int			ii = 0;
 
-	/*
-	 * Mark this query as having row security, so plancache can invalidate it
-	 * when necessary (eg: role changes)
-	 */
-	*hasRowSecurity = true;
+	if (npol <= 1)
+		return policies;
 
-	return;
+	pols = (RowSecurityPolicy *) palloc(sizeof(RowSecurityPolicy) * npol);
+
+	foreach(item, policies)
+	{
+		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+		pols[ii++] = *policy;
+	}
+
+	qsort(pols, npol, sizeof(RowSecurityPolicy), row_security_policy_cmp);
+
+	policies = NIL;
+	for (ii = 0; ii < npol; ii++)
+		policies = lappend(policies, &pols[ii]);
+
+	return policies;
+}
+
+/*
+ * qsort comparator to sort RowSecurityPolicy entries by name
+ */
+static int
+row_security_policy_cmp(const void *a, const void *b)
+{
+	const RowSecurityPolicy *pa = (const RowSecurityPolicy *) a;
+	const RowSecurityPolicy *pb = (const RowSecurityPolicy *) b;
+
+	/* Guard against NULL policy names from extensions */
+	if (pa->policy_name == NULL)
+		return pb->policy_name == NULL ? 0 : 1;
+	if (pb->policy_name == NULL)
+		return -1;
+
+	return strcmp(pa->policy_name, pb->policy_name);
 }
 
 /*
- * pull_row_security_policies
+ * build_security_quals
  *
- * Returns the list of policies to be added for this relation, based on the
- * type of command and the roles to which it applies, from the relation cache.
+ * Build security quals to enforce the specified RLS policies, restricting
+ * access to existing data in a table.  If there are no policies controlling
+ * access to the table, then all access is prohibited --- i.e., an implicit
+ * default-deny policy is used.
  *
+ * New security quals are added to securityQuals, and hasSubLinks is set to
+ * true if any of the quals added contain sublink subqueries.
  */
-static List *
-pull_row_security_policies(CmdType cmd, Relation relation, Oid user_id)
+static void
+build_security_quals(int rt_index,
+					 List *permissive_policies,
+					 List *restrictive_policies,
+					 List *returning_policies,
+					 bool returning,
+					 List **securityQuals,
+					 bool *hasSubLinks)
 {
-	List	   *policies = NIL;
 	ListCell   *item;
+	List	   *permissive_quals = NIL;
+	List	   *returning_quals = NIL;
+	Expr	   *rowsec_expr;
 
 	/*
-	 * Row security is enabled for the relation and the row security GUC is
-	 * either 'on' or 'force' here, so find the policies to apply to the
-	 * table. There must always be at least one policy defined (may be the
-	 * simple 'default-deny' policy, if none are explicitly defined on the
-	 * table).
+	 * First collect up the permissive quals.  If we do not find any permissive
+	 * policies then no rows are visible (this is handled below).
 	 */
-	foreach(item, relation->rd_rsdesc->policies)
+	foreach(item, permissive_policies)
 	{
 		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
 
-		/* Always add ALL policies, if they exist. */
-		if (policy->polcmd == '*' &&
-			check_role_for_policy(policy->roles, user_id))
-			policies = lcons(policy, policies);
-
-		/* Add relevant command-specific policies to the list. */
-		switch (cmd)
+		if (policy->qual != NULL)
 		{
-			case CMD_SELECT:
-				if (policy->polcmd == ACL_SELECT_CHR
-					&& check_role_for_policy(policy->roles, user_id))
-					policies = lcons(policy, policies);
-				break;
-			case CMD_INSERT:
-				/* If INSERT then only need to add the WITH CHECK qual */
-				if (policy->polcmd == ACL_INSERT_CHR
-					&& check_role_for_policy(policy->roles, user_id))
-					policies = lcons(policy, policies);
-				break;
-			case CMD_UPDATE:
-				if (policy->polcmd == ACL_UPDATE_CHR
-					&& check_role_for_policy(policy->roles, user_id))
-					policies = lcons(policy, policies);
-				break;
-			case CMD_DELETE:
-				if (policy->polcmd == ACL_DELETE_CHR
-					&& check_role_for_policy(policy->roles, user_id))
-					policies = lcons(policy, policies);
-				break;
-			default:
-				elog(ERROR, "unrecognized policy command type %d", (int) cmd);
-				break;
+			permissive_quals = lappend(permissive_quals,
+									   copyObject(policy->qual));
+			*hasSubLinks |= policy->hassublinks;
 		}
 	}
 
 	/*
-	 * There should always be a policy applied.  If there are none found then
-	 * create a simply defauly-deny policy (might be that policies exist but
-	 * that none of them apply to the role which is querying the table).
+	 * Now collect up the returning quals.  If we do not find any returning
+	 * policies then no rows are visible (this is handled below).
 	 */
-	if (policies == NIL)
+	if (returning)
 	{
-		RowSecurityPolicy *policy = NULL;
-		Datum		role;
-
-		role = ObjectIdGetDatum(ACL_ID_PUBLIC);
-
-		policy = palloc0(sizeof(RowSecurityPolicy));
-		policy->policy_name = pstrdup("default-deny policy");
-		policy->policy_id = InvalidOid;
-		policy->polcmd = '*';
-		policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true,
-										'i');
-		policy->qual = (Expr *) makeConst(BOOLOID, -1, InvalidOid,
-										  sizeof(bool), BoolGetDatum(false),
-										  false, true);
-		policy->with_check_qual = copyObject(policy->qual);
-		policy->hassublinks = false;
-
-		policies = list_make1(policy);
+		foreach(item, returning_policies)
+		{
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+
+			if (policy->qual != NULL)
+			{
+				returning_quals = lappend(returning_quals,
+										  copyObject(policy->qual));
+				*hasSubLinks |= policy->hassublinks;
+			}
+		}
 	}
 
-	Assert(policies != NIL);
+	/*
+	 * We must have permissive quals, always, or no rows are visible.
+	 *
+	 * Further, if this is a returning case, then we must also have returning
+	 * quals which allow rows to be visible to the UPDATE/DELETE RETURNING.
+	 *
+	 * If we do not, then we simply return a single 'false' qual which results
+	 * in no rows being visible.
+	 */
+	if (permissive_quals != NIL && (!returning || returning_quals != NIL))
+	{
+		/*
+		 * We now know that permissive policies exist, so we can now add
+		 * security quals based on the USING clauses from the restrictive
+		 * policies.  Since these need to be "AND"d together, we can
+		 * just add them one at a time.
+		 */
+		foreach(item, restrictive_policies)
+		{
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+			Expr	   *qual;
 
-	return policies;
+			if (policy->qual != NULL)
+			{
+				qual = copyObject(policy->qual);
+				ChangeVarNodes((Node *) qual, 1, rt_index, 0);
+
+				*securityQuals = lappend(*securityQuals, qual);
+				*hasSubLinks |= policy->hassublinks;
+			}
+		}
+
+		/*
+		 * Then add a single security qual "OR"ing together the USING clauses
+		 * from all the permissive policies.
+		 */
+		if (list_length(permissive_quals) == 1)
+			rowsec_expr = (Expr *) linitial(permissive_quals);
+		else
+			rowsec_expr = makeBoolExpr(OR_EXPR, permissive_quals, -1);
+
+		ChangeVarNodes((Node *) rowsec_expr, 1, rt_index, 0);
+		*securityQuals = lappend(*securityQuals, rowsec_expr);
+
+		/*
+		 * Finally, build a single security qual "OR"ing together the USING
+		 * clauses for the RETURNING case (ALL and SELECT policies).  This is
+		 * then added as another, additional, qual which is then AND'd into
+		 * the overall expression, preventing rows from being returned through
+		 * UPDATE/DELETE RETURNING which are not visible through ALL or SELECT
+		 * policies.
+		 */
+		if (returning)
+		{
+			if (list_length(returning_quals) == 1)
+				rowsec_expr = (Expr *) linitial(returning_quals);
+			else
+				rowsec_expr = makeBoolExpr(OR_EXPR, returning_quals, -1);
+
+			ChangeVarNodes((Node *) rowsec_expr, 1, rt_index, 0);
+			*securityQuals = lappend(*securityQuals, rowsec_expr);
+		}
+	}
+	else
+		/*
+		 * A permissive policy must exist for rows to be visible at all.
+		 * Therefore, if there were no permissive policies found, return a
+		 * single always-false clause.
+		 */
+		*securityQuals = lappend(*securityQuals,
+								 makeConst(BOOLOID, -1, InvalidOid,
+										   sizeof(bool), BoolGetDatum(false),
+										   false, true));
 }
 
 /*
- * process_policies
+ * build_with_check_options
  *
- * This will step through the policies which are passed in (which would come
- * from either the built-in ones created on a table, or from policies provided
- * by an extension through the hook provided), work out how to combine them,
- * rewrite them as necessary, and produce an Expr for the normal security
- * quals and an Expr for the with check quals.
+ * Build WithCheckOptions of the specified kind to check that new records
+ * added by an INSERT or UPDATE are consistent with the specified RLS
+ * policies.  Normally new data must satisfy the WITH CHECK clauses from the
+ * policies.  If a policy has no explicit WITH CHECK clause, its USING clause
+ * is used instead.  In the special case of an UPDATE arising from an
+ * INSERT ... ON CONFLICT DO UPDATE, existing records are first checked using
+ * a WCO_RLS_CONFLICT_CHECK WithCheckOption, which always uses the USING
+ * clauses from RLS policies.
  *
- * qual_eval, with_check_eval, and hassublinks are output variables
+ * New WCOs are added to withCheckOptions, and hasSubLinks is set to true if
+ * any of the check clauses added contain sublink subqueries.
  */
 static void
-process_policies(Query *root, List *policies, int rt_index, Expr **qual_eval,
-				 Expr **with_check_eval, bool *hassublinks,
-				 BoolExprType boolop)
+build_with_check_options(Relation rel,
+						 int rt_index,
+						 WCOKind kind,
+						 List *permissive_policies,
+						 List *restrictive_policies,
+						 List *returning_policies,
+						 bool returning,
+						 List **withCheckOptions,
+						 bool *hasSubLinks)
 {
 	ListCell   *item;
-	List	   *quals = NIL;
-	List	   *with_check_quals = NIL;
+	List	   *permissive_quals = NIL;
+	List	   *returning_quals = NIL;
+
+#define QUAL_FOR_WCO(policy) \
+	( kind != WCO_RLS_CONFLICT_CHECK &&	\
+	  (policy)->with_check_qual != NULL ? \
+	  (policy)->with_check_qual : (policy)->qual )
 
 	/*
-	 * Extract the USING and WITH CHECK quals from each of the policies and
-	 * add them to our lists.  We only want WITH CHECK quals if this RTE is
-	 * the query's result relation.
+	 * First collect up the permissive policy clauses, similar to
+	 * build_security_quals.
 	 */
-	foreach(item, policies)
+	foreach(item, permissive_policies)
 	{
 		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+		Expr	   *qual = QUAL_FOR_WCO(policy);
 
-		if (policy->qual != NULL)
-			quals = lcons(copyObject(policy->qual), quals);
-
-		if (policy->with_check_qual != NULL &&
-			rt_index == root->resultRelation)
-			with_check_quals = lcons(copyObject(policy->with_check_qual),
-									 with_check_quals);
-
-		/*
-		 * For each policy, if there is only a USING clause then copy/use it
-		 * for the WITH CHECK policy also, if this RTE is the query's result
-		 * relation.
-		 */
-		if (policy->qual != NULL && policy->with_check_qual == NULL &&
-			rt_index == root->resultRelation)
-			with_check_quals = lcons(copyObject(policy->qual),
-									 with_check_quals);
-
-
-		if (policy->hassublinks)
-			*hassublinks = true;
+		if (qual != NULL)
+		{
+			permissive_quals = lappend(permissive_quals, copyObject(qual));
+			*hasSubLinks |= policy->hassublinks;
+		}
 	}
 
 	/*
-	 * If we end up without any normal quals (perhaps the only policy matched
-	 * was for INSERT), then create a single all-false one.
+	 * Then collect up the returning policy clauses, again, similar to
+	 * build_security_quals.
 	 */
-	if (quals == NIL)
-		quals = lcons(makeConst(BOOLOID, -1, InvalidOid, sizeof(bool),
-								BoolGetDatum(false), false, true), quals);
+	foreach(item, returning_policies)
+	{
+		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+		Expr	   *qual = QUAL_FOR_WCO(policy);
+
+		if (qual != NULL)
+		{
+			returning_quals = lappend(returning_quals, copyObject(qual));
+			*hasSubLinks |= policy->hassublinks;
+		}
+	}
 
 	/*
-	 * Row security quals always have the target table as varno 1, as no joins
-	 * are permitted in row security expressions. We must walk the expression,
-	 * updating any references to varno 1 to the varno the table has in the
-	 * outer query.
-	 *
-	 * We rewrite the expression in-place.
+	 * There must be at least one permissive qual found, and in the RETURNING
+	 * case there must be at least one RETURNING qual found, or no rows are
+	 * allowed to be added.  This is the same as in build_security_quals.
 	 *
-	 * We must have some quals at this point; the default-deny policy, if
-	 * nothing else.  Note that we might not have any WITH CHECK quals- that's
-	 * fine, as this might not be the resultRelation.
+	 * If there are no permissive_quals or, with returning, there are no
+	 * returning_quals, then we fall through and return a single 'false' WCO,
+	 * preventing all new rows.
 	 */
-	Assert(quals != NIL);
+	if (permissive_quals != NIL && (!returning || returning_quals != NIL))
+	{
+		/*
+		 * Add a single WithCheckOption for all the permissive policy clauses
+		 * "OR"d together.  This check has no policy name, since if the check
+		 * fails it means that no policy granted permission to perform the
+		 * update, rather than any particular policy being violated.
+		 */
+		WithCheckOption *wco;
 
-	ChangeVarNodes((Node *) quals, 1, rt_index, 0);
+		wco = (WithCheckOption *) makeNode(WithCheckOption);
+		wco->kind = kind;
+		wco->relname = pstrdup(RelationGetRelationName(rel));
+		wco->polname = NULL;
+		wco->cascaded = false;
 
-	if (with_check_quals != NIL)
-		ChangeVarNodes((Node *) with_check_quals, 1, rt_index, 0);
+		if (list_length(permissive_quals) == 1)
+			wco->qual = (Node *) linitial(permissive_quals);
+		else
+			wco->qual = (Node *) makeBoolExpr(OR_EXPR, permissive_quals, -1);
 
-	/*
-	 * If more than one security qual is returned, then they need to be
-	 * combined together.
-	 */
-	if (list_length(quals) > 1)
-		*qual_eval = makeBoolExpr(boolop, quals, -1);
-	else
-		*qual_eval = (Expr *) linitial(quals);
+		ChangeVarNodes(wco->qual, 1, rt_index, 0);
 
-	/*
-	 * Similarly, if more than one WITH CHECK qual is returned, then they need
-	 * to be combined together.
-	 *
-	 * with_check_quals is allowed to be NIL here since this might not be the
-	 * resultRelation (see above).
-	 */
-	if (list_length(with_check_quals) > 1)
-		*with_check_eval = makeBoolExpr(boolop, with_check_quals, -1);
-	else if (with_check_quals != NIL)
-		*with_check_eval = (Expr *) linitial(with_check_quals);
-	else
-		*with_check_eval = NULL;
+		*withCheckOptions = lappend(*withCheckOptions, wco);
 
-	return;
+		if (returning)
+		{
+			/*
+			 * Similarly, if there is a returning clause, then add a
+			 * WithCheckOption for all the returning policy clauses "OR"d
+			 * together.  This check also has no policy name, since if the check
+			 * fails it means that no policy granted permission to perform the
+			 * update-with-returning, rather than any particular policy being
+			 * violated.
+			 */
+			WithCheckOption *wco;
+
+			wco = (WithCheckOption *) makeNode(WithCheckOption);
+			wco->kind = kind;
+			wco->relname = pstrdup(RelationGetRelationName(rel));
+			wco->polname = NULL;
+			wco->cascaded = false;
+
+			if (list_length(returning_quals) == 1)
+				wco->qual = (Node *) linitial(returning_quals);
+			else
+				wco->qual = (Node *) makeBoolExpr(OR_EXPR, returning_quals, -1);
+
+			ChangeVarNodes(wco->qual, 1, rt_index, 0);
+
+			*withCheckOptions = lappend(*withCheckOptions, wco);
+		}
+
+		/*
+		 * Now add WithCheckOptions for each of the restrictive policy clauses
+		 * (which will be "AND"d together).  We use a separate WithCheckOption
+		 * for each restrictive policy to allow the policy name to be included
+		 * in error reports if the policy is violated.
+		 */
+		foreach(item, restrictive_policies)
+		{
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+			Expr	   *qual = QUAL_FOR_WCO(policy);
+			WithCheckOption *wco;
+
+			if (qual != NULL)
+			{
+				qual = copyObject(qual);
+				ChangeVarNodes((Node *) qual, 1, rt_index, 0);
+
+				wco = (WithCheckOption *) makeNode(WithCheckOption);
+				wco->kind = kind;
+				wco->relname = pstrdup(RelationGetRelationName(rel));
+				wco->polname = pstrdup(policy->policy_name);
+				wco->qual = (Node *) qual;
+				wco->cascaded = false;
+
+				*withCheckOptions = lappend(*withCheckOptions, wco);
+				*hasSubLinks |= policy->hassublinks;
+			}
+		}
+	}
+	else
+	{
+		/*
+		 * If there were no policy clauses to check new data, add a single
+		 * always-false WCO (a default-deny policy).
+		 */
+		WithCheckOption *wco;
+
+		wco = (WithCheckOption *) makeNode(WithCheckOption);
+		wco->kind = kind;
+		wco->relname = pstrdup(RelationGetRelationName(rel));
+		wco->polname = NULL;
+		wco->qual = (Node *) makeConst(BOOLOID, -1, InvalidOid,
+									   sizeof(bool), BoolGetDatum(false),
+									   false, true);
+		wco->cascaded = false;
+
+		*withCheckOptions = lappend(*withCheckOptions, wco);
+	}
 }
 
 /*
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 420ef3d..9c3d096 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -859,8 +859,6 @@ equalPolicy(RowSecurityPolicy *policy1, RowSecurityPolicy *policy2)
 		if (policy2 == NULL)
 			return false;
 
-		if (policy1->policy_id != policy2->policy_id)
-			return false;
 		if (policy1->polcmd != policy2->polcmd)
 			return false;
 		if (policy1->hassublinks != policy2->hassublinks)
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index f0dcd2f..940cc32 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -928,6 +928,7 @@ typedef struct WithCheckOption
 	NodeTag		type;
 	WCOKind		kind;			/* kind of WCO */
 	char	   *relname;		/* name of relation that specified the WCO */
+	char	   *polname;		/* name of RLS policy being checked */
 	Node	   *qual;			/* constraint qual to check */
 	bool		cascaded;		/* true for a cascaded WCO on a view */
 } WithCheckOption;
diff --git a/src/include/rewrite/rowsecurity.h b/src/include/rewrite/rowsecurity.h
index 523c56e..0c3ef53 100644
--- a/src/include/rewrite/rowsecurity.h
+++ b/src/include/rewrite/rowsecurity.h
@@ -19,7 +19,6 @@
 
 typedef struct RowSecurityPolicy
 {
-	Oid			policy_id;		/* OID of the policy */
 	char	   *policy_name;	/* Name of the policy */
 	char		polcmd;			/* Type of command policy is for */
 	ArrayType  *roles;			/* Array of roles policy is for */
@@ -41,7 +40,9 @@ extern PGDLLIMPORT row_security_policy_hook_type row_security_policy_hook_permis
 
 extern PGDLLIMPORT row_security_policy_hook_type row_security_policy_hook_restrictive;
 
-extern void get_row_security_policies(Query *root, CmdType commandType,
+extern PGDLLIMPORT row_security_policy_hook_type row_security_policy_hook_returning;
+
+extern void get_row_security_policies(Query *root,
 						  RangeTblEntry *rte, int rt_index,
 						  List **securityQuals, List **withCheckOptions,
 						  bool *hasRowSecurity, bool *hasSubLinks);
diff --git a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
index 4587eb0..8885464 100644
--- a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
+++ b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
@@ -83,7 +83,7 @@ SELECT * FROM rls_test_restrictive;
 INSERT INTO rls_test_restrictive VALUES ('r1','s1',10);
 -- failure
 INSERT INTO rls_test_restrictive VALUES ('r4','s4',10);
-ERROR:  new row violates row level security policy for "rls_test_restrictive"
+ERROR:  new row violates row level security policy "extension policy" for "rls_test_restrictive"
 SET ROLE s1;
 -- With only the hook's policies, both
 -- permissive hook's policy is current_user = username
@@ -124,7 +124,7 @@ EXPLAIN (costs off) SELECT * FROM rls_test_permissive;
                           QUERY PLAN                           
 ---------------------------------------------------------------
  Seq Scan on rls_test_permissive
-   Filter: (("current_user"() = username) OR ((data % 2) = 0))
+   Filter: (((data % 2) = 0) OR ("current_user"() = username))
 (2 rows)
 
 SELECT * FROM rls_test_permissive;
@@ -163,7 +163,7 @@ SELECT * FROM rls_test_restrictive;
 INSERT INTO rls_test_restrictive VALUES ('r1','s1',8);
 -- failure
 INSERT INTO rls_test_restrictive VALUES ('r3','s3',10);
-ERROR:  new row violates row level security policy for "rls_test_restrictive"
+ERROR:  new row violates row level security policy "extension policy" for "rls_test_restrictive"
 -- failure
 INSERT INTO rls_test_restrictive VALUES ('r1','s1',7);
 ERROR:  new row violates row level security policy for "rls_test_restrictive"
@@ -176,7 +176,7 @@ EXPLAIN (costs off) SELECT * FROM rls_test_both;
                                         QUERY PLAN                                         
 -------------------------------------------------------------------------------------------
  Subquery Scan on rls_test_both
-   Filter: (("current_user"() = rls_test_both.username) OR ((rls_test_both.data % 2) = 0))
+   Filter: (((rls_test_both.data % 2) = 0) OR ("current_user"() = rls_test_both.username))
    ->  Seq Scan on rls_test_both rls_test_both_1
          Filter: ("current_user"() = supervisor)
 (4 rows)
@@ -190,7 +190,7 @@ SELECT * FROM rls_test_both;
 INSERT INTO rls_test_both VALUES ('r1','s1',8);
 -- failure
 INSERT INTO rls_test_both VALUES ('r3','s3',10);
-ERROR:  new row violates row level security policy for "rls_test_both"
+ERROR:  new row violates row level security policy "extension policy" for "rls_test_both"
 -- failure
 INSERT INTO rls_test_both VALUES ('r1','s1',7);
 ERROR:  new row violates row level security policy for "rls_test_both"
diff --git a/src/test/modules/test_rls_hooks/test_rls_hooks.c b/src/test/modules/test_rls_hooks/test_rls_hooks.c
index b96dbff..cc865cd 100644
--- a/src/test/modules/test_rls_hooks/test_rls_hooks.c
+++ b/src/test/modules/test_rls_hooks/test_rls_hooks.c
@@ -87,7 +87,6 @@ test_rls_hooks_permissive(CmdType cmdtype, Relation relation)
 	role = ObjectIdGetDatum(ACL_ID_PUBLIC);
 
 	policy->policy_name = pstrdup("extension policy");
-	policy->policy_id = InvalidOid;
 	policy->polcmd = '*';
 	policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true, 'i');
 
@@ -151,7 +150,6 @@ test_rls_hooks_restrictive(CmdType cmdtype, Relation relation)
 	role = ObjectIdGetDatum(ACL_ID_PUBLIC);
 
 	policy->policy_name = pstrdup("extension policy");
-	policy->policy_id = InvalidOid;
 	policy->polcmd = '*';
 	policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true, 'i');
 
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 6fc80af..78cc058 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -1246,21 +1246,23 @@ NOTICE:  f_leak => cde
 EXPLAIN (COSTS OFF) UPDATE t2 t2_1 SET b = t2_2.b FROM t2 t2_2
 WHERE t2_1.a = 3 AND t2_2.a = t2_1.a AND t2_2.b = t2_1.b
 AND f_leak(t2_1.b) AND f_leak(t2_2.b) RETURNING *, t2_1, t2_2;
-                          QUERY PLAN                           
----------------------------------------------------------------
+                             QUERY PLAN                              
+---------------------------------------------------------------------
  Update on t2 t2_1_1
    ->  Nested Loop
          Join Filter: (t2_1.b = t2_2.b)
          ->  Subquery Scan on t2_1
                Filter: f_leak(t2_1.b)
-               ->  LockRows
-                     ->  Seq Scan on t2 t2_1_2
-                           Filter: ((a = 3) AND ((a % 2) = 1))
+               ->  Subquery Scan on t2_1_2
+                     Filter: ((t2_1_2.a % 2) = 1)
+                     ->  LockRows
+                           ->  Seq Scan on t2 t2_1_3
+                                 Filter: ((a = 3) AND ((a % 2) = 1))
          ->  Subquery Scan on t2_2
                Filter: f_leak(t2_2.b)
                ->  Seq Scan on t2 t2_2_1
                      Filter: ((a = 3) AND ((a % 2) = 1))
-(12 rows)
+(14 rows)
 
 UPDATE t2 t2_1 SET b = t2_2.b FROM t2 t2_2
 WHERE t2_1.a = 3 AND t2_2.a = t2_1.a AND t2_2.b = t2_1.b
@@ -1275,8 +1277,8 @@ NOTICE:  f_leak => cde
 EXPLAIN (COSTS OFF) UPDATE t1 t1_1 SET b = t1_2.b FROM t1 t1_2
 WHERE t1_1.a = 4 AND t1_2.a = t1_1.a AND t1_2.b = t1_1.b
 AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
-                          QUERY PLAN                           
----------------------------------------------------------------
+                             QUERY PLAN                              
+---------------------------------------------------------------------
  Update on t1 t1_1_3
    Update on t1 t1_1_3
    Update on t2 t1_1
@@ -1285,9 +1287,11 @@ AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
          Join Filter: (t1_1.b = t1_2.b)
          ->  Subquery Scan on t1_1
                Filter: f_leak(t1_1.b)
-               ->  LockRows
-                     ->  Seq Scan on t1 t1_1_4
-                           Filter: ((a = 4) AND ((a % 2) = 0))
+               ->  Subquery Scan on t1_1_4
+                     Filter: ((t1_1_4.a % 2) = 0)
+                     ->  LockRows
+                           ->  Seq Scan on t1 t1_1_5
+                                 Filter: ((a = 4) AND ((a % 2) = 0))
          ->  Subquery Scan on t1_2
                Filter: f_leak(t1_2.b)
                ->  Append
@@ -1301,9 +1305,11 @@ AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
          Join Filter: (t1_1_1.b = t1_2_1.b)
          ->  Subquery Scan on t1_1_1
                Filter: f_leak(t1_1_1.b)
-               ->  LockRows
-                     ->  Seq Scan on t2 t1_1_5
-                           Filter: ((a = 4) AND ((a % 2) = 0))
+               ->  Subquery Scan on t1_1_6
+                     Filter: ((t1_1_6.a % 2) = 0)
+                     ->  LockRows
+                           ->  Seq Scan on t2 t1_1_7
+                                 Filter: ((a = 4) AND ((a % 2) = 0))
          ->  Subquery Scan on t1_2_1
                Filter: f_leak(t1_2_1.b)
                ->  Append
@@ -1317,9 +1323,11 @@ AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
          Join Filter: (t1_1_2.b = t1_2_2.b)
          ->  Subquery Scan on t1_1_2
                Filter: f_leak(t1_1_2.b)
-               ->  LockRows
-                     ->  Seq Scan on t3 t1_1_6
-                           Filter: ((a = 4) AND ((a % 2) = 0))
+               ->  Subquery Scan on t1_1_8
+                     Filter: ((t1_1_8.a % 2) = 0)
+                     ->  LockRows
+                           ->  Seq Scan on t3 t1_1_9
+                                 Filter: ((a = 4) AND ((a % 2) = 0))
          ->  Subquery Scan on t1_2_2
                Filter: f_leak(t1_2_2.b)
                ->  Append
@@ -1329,7 +1337,7 @@ AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
                            Filter: ((a = 4) AND ((a % 2) = 0))
                      ->  Seq Scan on t3 t1_2_11
                            Filter: ((a = 4) AND ((a % 2) = 0))
-(52 rows)
+(58 rows)
 
 UPDATE t1 t1_1 SET b = t1_2.b FROM t1 t1_2
 WHERE t1_1.a = 4 AND t1_2.a = t1_1.a AND t1_2.b = t1_1.b
@@ -1422,6 +1430,86 @@ NOTICE:  f_leak => yyyyyy
 (3 rows)
 
 --
+-- Non-target relations are only subject to SELECT policies
+--
+SET SESSION AUTHORIZATION rls_regress_user0;
+CREATE TABLE r1 (a int);
+CREATE TABLE r2 (a int);
+INSERT INTO r1 VALUES (10), (20);
+INSERT INTO r2 VALUES (10), (20);
+GRANT ALL ON r1, r2 TO rls_regress_user1;
+CREATE POLICY p1 ON r1 USING (true);
+ALTER TABLE r1 ENABLE ROW LEVEL SECURITY;
+CREATE POLICY p1 ON r2 FOR SELECT USING (true);
+CREATE POLICY p2 ON r2 FOR INSERT WITH CHECK (false);
+CREATE POLICY p3 ON r2 FOR UPDATE USING (false);
+CREATE POLICY p4 ON r2 FOR DELETE USING (false);
+ALTER TABLE r2 ENABLE ROW LEVEL SECURITY;
+SET SESSION AUTHORIZATION rls_regress_user1;
+SELECT * FROM r1;
+ a  
+----
+ 10
+ 20
+(2 rows)
+
+SELECT * FROM r2;
+ a  
+----
+ 10
+ 20
+(2 rows)
+
+-- r2 is read-only
+INSERT INTO r2 VALUES (2); -- Not allowed
+ERROR:  new row violates row level security policy for "r2"
+UPDATE r2 SET a = 2 RETURNING *; -- Updates nothing
+ a 
+---
+(0 rows)
+
+DELETE FROM r2 RETURNING *; -- Deletes nothing
+ a 
+---
+(0 rows)
+
+-- r2 can be used as a non-target relation in DML
+INSERT INTO r1 SELECT a + 1 FROM r2 RETURNING *; -- OK
+ a  
+----
+ 11
+ 21
+(2 rows)
+
+UPDATE r1 SET a = r2.a + 2 FROM r2 WHERE r1.a = r2.a RETURNING *; -- OK
+ a  | a  
+----+----
+ 12 | 10
+ 22 | 20
+(2 rows)
+
+DELETE FROM r1 USING r2 WHERE r1.a = r2.a + 2 RETURNING *; -- OK
+ a  | a  
+----+----
+ 12 | 10
+ 22 | 20
+(2 rows)
+
+SELECT * FROM r1;
+ a  
+----
+ 11
+ 21
+(2 rows)
+
+SELECT * FROM r2;
+ a  
+----
+ 10
+ 20
+(2 rows)
+
+--
 -- S.b. view on top of Row-level security
 --
 SET SESSION AUTHORIZATION rls_regress_user0;
@@ -1960,8 +2048,6 @@ NOTICE:  f_leak => fgh_updt
 (6 rows)
 
 DELETE FROM x1 WHERE f_leak(b) RETURNING *;
-NOTICE:  f_leak => abc_updt
-NOTICE:  f_leak => efg_updt
 NOTICE:  f_leak => cde_updt
 NOTICE:  f_leak => fgh_updt
 NOTICE:  f_leak => bcd_updt_updt
@@ -1970,15 +2056,13 @@ NOTICE:  f_leak => fgh_updt_updt
 NOTICE:  f_leak => fgh_updt_updt
  a |       b       |         c         
 ---+---------------+-------------------
- 1 | abc_updt      | rls_regress_user1
- 5 | efg_updt      | rls_regress_user1
  3 | cde_updt      | rls_regress_user2
  7 | fgh_updt      | rls_regress_user2
  2 | bcd_updt_updt | rls_regress_user1
  4 | def_updt_updt | rls_regress_user2
  6 | fgh_updt_updt | rls_regress_user1
  8 | fgh_updt_updt | rls_regress_user2
-(8 rows)
+(6 rows)
 
 --
 -- Duplicate Policy Names
@@ -3033,89 +3117,6 @@ CREATE POLICY p ON t USING (max(c)); -- fails: aggregate functions are not allow
 ERROR:  aggregate functions are not allowed in policy expressions
 ROLLBACK;
 --
--- Non-target relations are only subject to SELECT policies
---
-SET SESSION AUTHORIZATION rls_regress_user0;
-CREATE TABLE r1 (a int);
-CREATE TABLE r2 (a int);
-INSERT INTO r1 VALUES (10), (20);
-INSERT INTO r2 VALUES (10), (20);
-GRANT ALL ON r1, r2 TO rls_regress_user1;
-CREATE POLICY p1 ON r1 USING (true);
-ALTER TABLE r1 ENABLE ROW LEVEL SECURITY;
-CREATE POLICY p1 ON r2 FOR SELECT USING (true);
-CREATE POLICY p2 ON r2 FOR INSERT WITH CHECK (false);
-CREATE POLICY p3 ON r2 FOR UPDATE USING (false);
-CREATE POLICY p4 ON r2 FOR DELETE USING (false);
-ALTER TABLE r2 ENABLE ROW LEVEL SECURITY;
-SET SESSION AUTHORIZATION rls_regress_user1;
-SELECT * FROM r1;
- a  
-----
- 10
- 20
-(2 rows)
-
-SELECT * FROM r2;
- a  
-----
- 10
- 20
-(2 rows)
-
--- r2 is read-only
-INSERT INTO r2 VALUES (2); -- Not allowed
-ERROR:  new row violates row level security policy for "r2"
-UPDATE r2 SET a = 2 RETURNING *; -- Updates nothing
- a 
----
-(0 rows)
-
-DELETE FROM r2 RETURNING *; -- Deletes nothing
- a 
----
-(0 rows)
-
--- r2 can be used as a non-target relation in DML
-INSERT INTO r1 SELECT a + 1 FROM r2 RETURNING *; -- OK
- a  
-----
- 11
- 21
-(2 rows)
-
-UPDATE r1 SET a = r2.a + 2 FROM r2 WHERE r1.a = r2.a RETURNING *; -- OK
- a  | a  
-----+----
- 12 | 10
- 22 | 20
-(2 rows)
-
-DELETE FROM r1 USING r2 WHERE r1.a = r2.a + 2 RETURNING *; -- OK
- a  | a  
-----+----
- 12 | 10
- 22 | 20
-(2 rows)
-
-SELECT * FROM r1;
- a  
-----
- 11
- 21
-(2 rows)
-
-SELECT * FROM r2;
- a  
-----
- 10
- 20
-(2 rows)
-
-SET SESSION AUTHORIZATION rls_regress_user0;
-DROP TABLE r1;
-DROP TABLE r2;
---
 -- Clean up objects
 --
 RESET SESSION AUTHORIZATION;
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index e8c09e9..414d010 100644
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -488,6 +488,42 @@ DELETE FROM only t1 WHERE f_leak(b) RETURNING oid, *, t1;
 DELETE FROM t1 WHERE f_leak(b) RETURNING oid, *, t1;
 
 --
+-- Non-target relations are only subject to SELECT policies
+--
+SET SESSION AUTHORIZATION rls_regress_user0;
+CREATE TABLE r1 (a int);
+CREATE TABLE r2 (a int);
+INSERT INTO r1 VALUES (10), (20);
+INSERT INTO r2 VALUES (10), (20);
+
+GRANT ALL ON r1, r2 TO rls_regress_user1;
+
+CREATE POLICY p1 ON r1 USING (true);
+ALTER TABLE r1 ENABLE ROW LEVEL SECURITY;
+
+CREATE POLICY p1 ON r2 FOR SELECT USING (true);
+CREATE POLICY p2 ON r2 FOR INSERT WITH CHECK (false);
+CREATE POLICY p3 ON r2 FOR UPDATE USING (false);
+CREATE POLICY p4 ON r2 FOR DELETE USING (false);
+ALTER TABLE r2 ENABLE ROW LEVEL SECURITY;
+
+SET SESSION AUTHORIZATION rls_regress_user1;
+SELECT * FROM r1;
+SELECT * FROM r2;
+
+-- r2 is read-only
+INSERT INTO r2 VALUES (2); -- Not allowed
+UPDATE r2 SET a = 2 RETURNING *; -- Updates nothing
+DELETE FROM r2 RETURNING *; -- Deletes nothing
+
+-- r2 can be used as a non-target relation in DML
+INSERT INTO r1 SELECT a + 1 FROM r2 RETURNING *; -- OK
+UPDATE r1 SET a = r2.a + 2 FROM r2 WHERE r1.a = r2.a RETURNING *; -- OK
+DELETE FROM r1 USING r2 WHERE r1.a = r2.a + 2 RETURNING *; -- OK
+SELECT * FROM r1;
+SELECT * FROM r2;
+
+--
 -- S.b. view on top of Row-level security
 --
 SET SESSION AUTHORIZATION rls_regress_user0;
@@ -1299,46 +1335,6 @@ CREATE POLICY p ON t USING (max(c)); -- fails: aggregate functions are not allow
 ROLLBACK;
 
 --
--- Non-target relations are only subject to SELECT policies
---
-SET SESSION AUTHORIZATION rls_regress_user0;
-CREATE TABLE r1 (a int);
-CREATE TABLE r2 (a int);
-INSERT INTO r1 VALUES (10), (20);
-INSERT INTO r2 VALUES (10), (20);
-
-GRANT ALL ON r1, r2 TO rls_regress_user1;
-
-CREATE POLICY p1 ON r1 USING (true);
-ALTER TABLE r1 ENABLE ROW LEVEL SECURITY;
-
-CREATE POLICY p1 ON r2 FOR SELECT USING (true);
-CREATE POLICY p2 ON r2 FOR INSERT WITH CHECK (false);
-CREATE POLICY p3 ON r2 FOR UPDATE USING (false);
-CREATE POLICY p4 ON r2 FOR DELETE USING (false);
-ALTER TABLE r2 ENABLE ROW LEVEL SECURITY;
-
-SET SESSION AUTHORIZATION rls_regress_user1;
-SELECT * FROM r1;
-SELECT * FROM r2;
-
--- r2 is read-only
-INSERT INTO r2 VALUES (2); -- Not allowed
-UPDATE r2 SET a = 2 RETURNING *; -- Updates nothing
-DELETE FROM r2 RETURNING *; -- Deletes nothing
-
--- r2 can be used as a non-target relation in DML
-INSERT INTO r1 SELECT a + 1 FROM r2 RETURNING *; -- OK
-UPDATE r1 SET a = r2.a + 2 FROM r2 WHERE r1.a = r2.a RETURNING *; -- OK
-DELETE FROM r1 USING r2 WHERE r1.a = r2.a + 2 RETURNING *; -- OK
-SELECT * FROM r1;
-SELECT * FROM r2;
-
-SET SESSION AUTHORIZATION rls_regress_user0;
-DROP TABLE r1;
-DROP TABLE r2;
-
---
 -- Clean up objects
 --
 RESET SESSION AUTHORIZATION;
-- 
1.9.1

From f6866952d2b5049acf8eecb45b990c3ab4916f04 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Sun, 13 Sep 2015 09:03:09 -0400
Subject: [PATCH 1/2] RLS refactoring

This refactors rewrite/rowsecurity.c to simplify the handling of the
default deny case (reducing the number of places where we check for and
add the default deny policy from three to one) by splitting up the
retrival of the policies from the application of them.

This also allowed us to do away with the policy_id field.  A policy_name
field was added for WithCheckOption policies and is used in error
reporting, when available.

Patch by Dean Rasheed, with various mostly cosmetic changes by me.

Back-patch to 9.5 where RLS was introduced to avoid unnecessary
differences, since we're still in alpha, per discussion with Robert.
---
 src/backend/commands/policy.c                      |  41 --
 src/backend/executor/execMain.c                    |  20 +-
 src/backend/nodes/copyfuncs.c                      |   1 +
 src/backend/nodes/equalfuncs.c                     |   1 +
 src/backend/nodes/outfuncs.c                       |   1 +
 src/backend/nodes/readfuncs.c                      |   1 +
 src/backend/rewrite/rewriteHandler.c               |   5 +-
 src/backend/rewrite/rowsecurity.c                  | 816 +++++++++++----------
 src/backend/utils/cache/relcache.c                 |   2 -
 src/include/nodes/parsenodes.h                     |   1 +
 src/include/rewrite/rowsecurity.h                  |   3 +-
 .../test_rls_hooks/expected/test_rls_hooks.out     |  10 +-
 src/test/modules/test_rls_hooks/test_rls_hooks.c   |   2 -
 13 files changed, 447 insertions(+), 457 deletions(-)

diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 45326a3..8851fe7 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -186,9 +186,6 @@ policy_role_list_to_array(List *roles, int *num_roles)
 /*
  * Load row security policy from the catalog, and store it in
  * the relation's relcache entry.
- *
- * We will always set up some kind of policy here.  If no explicit policies
- * are found then an implicit default-deny policy is created.
  */
 void
 RelationBuildRowSecurity(Relation relation)
@@ -246,7 +243,6 @@ RelationBuildRowSecurity(Relation relation)
 			char	   *with_check_value;
 			Expr	   *with_check_qual;
 			char	   *policy_name_value;
-			Oid			policy_id;
 			bool		isnull;
 			RowSecurityPolicy *policy;
 
@@ -298,14 +294,11 @@ RelationBuildRowSecurity(Relation relation)
 			else
 				with_check_qual = NULL;
 
-			policy_id = HeapTupleGetOid(tuple);
-
 			/* Now copy everything into the cache context */
 			MemoryContextSwitchTo(rscxt);
 
 			policy = palloc0(sizeof(RowSecurityPolicy));
 			policy->policy_name = pstrdup(policy_name_value);
-			policy->policy_id = policy_id;
 			policy->polcmd = cmd_value;
 			policy->roles = DatumGetArrayTypePCopy(roles_datum);
 			policy->qual = copyObject(qual_expr);
@@ -326,40 +319,6 @@ RelationBuildRowSecurity(Relation relation)
 
 		systable_endscan(sscan);
 		heap_close(catalog, AccessShareLock);
-
-		/*
-		 * Check if no policies were added
-		 *
-		 * If no policies exist in pg_policy for this relation, then we need
-		 * to create a single default-deny policy.  We use InvalidOid for the
-		 * Oid to indicate that this is the default-deny policy (we may decide
-		 * to ignore the default policy if an extension adds policies).
-		 */
-		if (rsdesc->policies == NIL)
-		{
-			RowSecurityPolicy *policy;
-			Datum		role;
-
-			MemoryContextSwitchTo(rscxt);
-
-			role = ObjectIdGetDatum(ACL_ID_PUBLIC);
-
-			policy = palloc0(sizeof(RowSecurityPolicy));
-			policy->policy_name = pstrdup("default-deny policy");
-			policy->policy_id = InvalidOid;
-			policy->polcmd = '*';
-			policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true,
-											'i');
-			policy->qual = (Expr *) makeConst(BOOLOID, -1, InvalidOid,
-										   sizeof(bool), BoolGetDatum(false),
-											  false, true);
-			policy->with_check_qual = copyObject(policy->qual);
-			policy->hassublinks = false;
-
-			rsdesc->policies = lcons(policy, rsdesc->policies);
-
-			MemoryContextSwitchTo(oldcxt);
-		}
 	}
 	PG_CATCH();
 	{
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 2c65a90..c28eb2b 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1815,14 +1815,26 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
 					break;
 				case WCO_RLS_INSERT_CHECK:
 				case WCO_RLS_UPDATE_CHECK:
-					ereport(ERROR,
-							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					if (wco->polname != NULL)
+						ereport(ERROR,
+								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+							 errmsg("new row violates row level security policy \"%s\" for \"%s\"",
+									wco->polname, wco->relname)));
+					else
+						ereport(ERROR,
+								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 							 errmsg("new row violates row level security policy for \"%s\"",
 									wco->relname)));
 					break;
 				case WCO_RLS_CONFLICT_CHECK:
-					ereport(ERROR,
-							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					if (wco->polname != NULL)
+						ereport(ERROR,
+								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+							 errmsg("new row violates row level security policy \"%s\" (USING expression) for \"%s\"",
+									wco->polname, wco->relname)));
+					else
+						ereport(ERROR,
+								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 							 errmsg("new row violates row level security policy (USING expression) for \"%s\"",
 									wco->relname)));
 					break;
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index bd2e80e..1c801f5 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2168,6 +2168,7 @@ _copyWithCheckOption(const WithCheckOption *from)
 
 	COPY_SCALAR_FIELD(kind);
 	COPY_STRING_FIELD(relname);
+	COPY_STRING_FIELD(polname);
 	COPY_NODE_FIELD(qual);
 	COPY_SCALAR_FIELD(cascaded);
 
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 19412fe..8f16833 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2455,6 +2455,7 @@ _equalWithCheckOption(const WithCheckOption *a, const WithCheckOption *b)
 {
 	COMPARE_SCALAR_FIELD(kind);
 	COMPARE_STRING_FIELD(relname);
+	COMPARE_STRING_FIELD(polname);
 	COMPARE_NODE_FIELD(qual);
 	COMPARE_SCALAR_FIELD(cascaded);
 
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index a878498..79b7179 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2403,6 +2403,7 @@ _outWithCheckOption(StringInfo str, const WithCheckOption *node)
 
 	WRITE_ENUM_FIELD(kind, WCOKind);
 	WRITE_STRING_FIELD(relname);
+	WRITE_STRING_FIELD(polname);
 	WRITE_NODE_FIELD(qual);
 	WRITE_BOOL_FIELD(cascaded);
 }
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 23e0b36..df55b76 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -270,6 +270,7 @@ _readWithCheckOption(void)
 
 	READ_ENUM_FIELD(kind, WCOKind);
 	READ_STRING_FIELD(relname);
+	READ_STRING_FIELD(polname);
 	READ_NODE_FIELD(qual);
 	READ_BOOL_FIELD(cascaded);
 
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index db3c2c7..1b8e7b0 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1786,8 +1786,8 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
 		/*
 		 * Fetch any new security quals that must be applied to this RTE.
 		 */
-		get_row_security_policies(parsetree, parsetree->commandType, rte,
-								  rt_index, &securityQuals, &withCheckOptions,
+		get_row_security_policies(parsetree, rte, rt_index,
+								  &securityQuals, &withCheckOptions,
 								  &hasRowSecurity, &hasSubLinks);
 
 		if (securityQuals != NIL || withCheckOptions != NIL)
@@ -3026,6 +3026,7 @@ rewriteTargetView(Query *parsetree, Relation view)
 			wco = makeNode(WithCheckOption);
 			wco->kind = WCO_VIEW_CHECK;
 			wco->relname = pstrdup(RelationGetRelationName(view));
+			wco->polname = NULL;
 			wco->qual = NULL;
 			wco->cascaded = cascaded;
 
diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index 5a81db3..b96c29d 100644
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -13,11 +13,12 @@
  * Any part of the system which is returning records back to the user, or
  * which is accepting records from the user to add to a table, needs to
  * consider the policies associated with the table (if any).  For normal
- * queries, this is handled by calling prepend_row_security_policies() during
- * rewrite, which looks at each RTE and adds the expressions defined by the
- * policies to the securityQuals list for the RTE.  For queries which modify
- * the relation, any WITH CHECK policies are added to the list of
- * WithCheckOptions for the Query and checked against each row which is being
+ * queries, this is handled by calling get_row_security_policies() during
+ * rewrite, for each RTE in the query.  This returns the expressions defined
+ * by the table's policies as a list that is prepended to the securityQuals
+ * list for the RTE.  For queries which modify the table, any WITH CHECK
+ * clauses from the table's policies are also returned and prepended to the
+ * list of WithCheckOptions for the Query to check each row that is being
  * added to the table.  Other parts of the system (eg: COPY) simply construct
  * a normal query and use that, if RLS is to be applied.
  *
@@ -56,13 +57,29 @@
 #include "utils/syscache.h"
 #include "tcop/utility.h"
 
-static List *pull_row_security_policies(CmdType cmd, Relation relation,
-						   Oid user_id);
-static void process_policies(Query *root, List *policies, int rt_index,
-				 Expr **final_qual,
-				 Expr **final_with_check_qual,
-				 bool *hassublinks,
-				 BoolExprType boolop);
+static void get_policies_for_relation(Relation relation,
+						  CmdType cmd, Oid user_id,
+						  List **permissive_policies,
+						  List **restrictive_policies);
+
+static List *sort_policies_by_name(List *policies);
+
+static int row_security_policy_cmp(const void *a, const void *b);
+
+static void add_security_quals(int rt_index,
+							   List *permissive_policies,
+							   List *restrictive_policies,
+							   List **securityQuals,
+							   bool *hasSubLinks);
+
+static void add_with_check_options(Relation rel,
+								   int rt_index,
+								   WCOKind kind,
+								   List *permissive_policies,
+								   List *restrictive_policies,
+								   List **withCheckOptions,
+								   bool *hasSubLinks);
+
 static bool check_role_for_policy(ArrayType *policy_roles, Oid user_id);
 
 /*
@@ -73,42 +90,29 @@ static bool check_role_for_policy(ArrayType *policy_roles, Oid user_id);
  *
  * row_security_policy_hook_restrictive can be used to add policies which
  * are enforced, regardless of other policies (they are "AND"d).
- *
- * See below where the hook is called in prepend_row_security_policies for
- * insight into how to use this hook.
  */
 row_security_policy_hook_type row_security_policy_hook_permissive = NULL;
 row_security_policy_hook_type row_security_policy_hook_restrictive = NULL;
 
 /*
- * Get any row security quals and check quals that should be applied to the
- * specified RTE.
+ * Get any row security quals and WithCheckOption checks that should be
+ * applied to the specified RTE.
  *
  * In addition, hasRowSecurity is set to true if row level security is enabled
  * (even if this RTE doesn't have any row security quals), and hasSubLinks is
  * set to true if any of the quals returned contain sublinks.
  */
 void
-get_row_security_policies(Query *root, CmdType commandType, RangeTblEntry *rte,
-						  int rt_index, List **securityQuals,
-						  List **withCheckOptions, bool *hasRowSecurity,
-						  bool *hasSubLinks)
+get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
+						  List **securityQuals, List **withCheckOptions,
+						  bool *hasRowSecurity, bool *hasSubLinks)
 {
-	Expr	   *rowsec_expr = NULL;
-	Expr	   *rowsec_with_check_expr = NULL;
-	Expr	   *hook_expr_restrictive = NULL;
-	Expr	   *hook_with_check_expr_restrictive = NULL;
-	Expr	   *hook_expr_permissive = NULL;
-	Expr	   *hook_with_check_expr_permissive = NULL;
-
-	List	   *rowsec_policies;
-	List	   *hook_policies_restrictive = NIL;
-	List	   *hook_policies_permissive = NIL;
-
-	Relation	rel;
 	Oid			user_id;
 	int			rls_status;
-	bool		defaultDeny = false;
+	Relation	rel;
+	CmdType		commandType;
+	List	   *permissive_policies;
+	List	   *restrictive_policies;
 
 	/* Defaults for the return values */
 	*securityQuals = NIL;
@@ -157,465 +161,479 @@ get_row_security_policies(Query *root, CmdType commandType, RangeTblEntry *rte,
 	 * policies and t2's SELECT policies.
 	 */
 	rel = heap_open(rte->relid, NoLock);
-	if (rt_index != root->resultRelation)
-		commandType = CMD_SELECT;
-
-	rowsec_policies = pull_row_security_policies(commandType, rel,
-												 user_id);
 
-	/*
-	 * Check if this is only the default-deny policy.
-	 *
-	 * Normally, if the table has row security enabled but there are no
-	 * policies, we use a default-deny policy and not allow anything. However,
-	 * when an extension uses the hook to add their own policies, we don't
-	 * want to include the default deny policy or there won't be any way for a
-	 * user to use an extension exclusively for the policies to be used.
-	 */
-	if (((RowSecurityPolicy *) linitial(rowsec_policies))->policy_id
-		== InvalidOid)
-		defaultDeny = true;
+	commandType = rt_index == root->resultRelation ?
+				  root->commandType : CMD_SELECT;
 
-	/* Now that we have our policies, build the expressions from them. */
-	process_policies(root, rowsec_policies, rt_index, &rowsec_expr,
-					 &rowsec_with_check_expr, hasSubLinks, OR_EXPR);
+	get_policies_for_relation(rel, commandType, user_id, &permissive_policies,
+							  &restrictive_policies);
 
 	/*
-	 * Also, allow extensions to add their own policies.
-	 *
-	 * extensions can add either permissive or restrictive policies.
+	 * For SELECT, UPDATE and DELETE, add security quals to enforce these
+	 * policies.  These security quals control access to existing table rows.
+	 * Restrictive policies are "AND"d together, and permissive policies are
+	 * "OR"d together.
 	 *
-	 * Note that, as with the internal policies, if multiple policies are
-	 * returned then they will be combined into a single expression with all
-	 * of them OR'd (for permissive) or AND'd (for restrictive) together.
-	 *
-	 * If only a USING policy is returned by the extension then it will be
-	 * used for WITH CHECK as well, similar to how internal policies are
-	 * handled.
-	 *
-	 * The only caveat to this is that if there are NO internal policies
-	 * defined, there ARE policies returned by the extension, and RLS is
-	 * enabled on the table, then we will ignore the internally-generated
-	 * default-deny policy and use only the policies returned by the
-	 * extension.
+	 * If there are no policy clauses controlling access to the table, this
+	 * will add a single always-false clause (a default-deny policy).
 	 */
-	if (row_security_policy_hook_restrictive)
-	{
-		hook_policies_restrictive = (*row_security_policy_hook_restrictive) (commandType, rel);
-
-		/* Build the expression from any policies returned. */
-		if (hook_policies_restrictive != NIL)
-			process_policies(root, hook_policies_restrictive, rt_index,
-							 &hook_expr_restrictive,
-							 &hook_with_check_expr_restrictive,
-							 hasSubLinks,
-							 AND_EXPR);
-	}
-
-	if (row_security_policy_hook_permissive)
-	{
-		hook_policies_permissive = (*row_security_policy_hook_permissive) (commandType, rel);
-
-		/* Build the expression from any policies returned. */
-		if (hook_policies_permissive != NIL)
-			process_policies(root, hook_policies_permissive, rt_index,
-							 &hook_expr_permissive,
-							 &hook_with_check_expr_permissive, hasSubLinks,
-							 OR_EXPR);
-	}
+	if (commandType == CMD_SELECT ||
+		commandType == CMD_UPDATE ||
+		commandType == CMD_DELETE)
+		add_security_quals(rt_index,
+						   permissive_policies,
+						   restrictive_policies,
+						   securityQuals,
+						   hasSubLinks);
 
 	/*
-	 * If the only built-in policy is the default-deny one, and permissive hook
-	 * policies exist, then use the hook policies only and do not apply the
-	 * default-deny policy.  Otherwise, we will apply both sets below.
-	 *
-	 * Note that we do not remove the defaultDeny policy if only *restrictive*
-	 * policies exist as restrictive policies should only ever be reducing what
-	 * is visible.  Therefore, at least one permissive policy must exist which
-	 * allows records to be seen before restrictive policies can remove rows
-	 * from that set.  A single "true" policy can be created to address this
-	 * requirement, if necessary.
-	 */
-	if (defaultDeny && hook_policies_permissive != NIL)
-	{
-		rowsec_expr = NULL;
-		rowsec_with_check_expr = NULL;
-	}
-
-	/*
-	 * For INSERT or UPDATE, we need to add the WITH CHECK quals to Query's
-	 * withCheckOptions to verify that any new records pass the WITH CHECK
-	 * policy (this will be a copy of the USING policy, if no explicit WITH
-	 * CHECK policy exists).
+	 * For INSERT and UPDATE, add withCheckOptions to verify that any new
+	 * records added are consistent with the security policies.  This will use
+	 * each policy's WITH CHECK clause, or its USING clause if no explicit
+	 * WITH CHECK clause is defined.
 	 */
 	if (commandType == CMD_INSERT || commandType == CMD_UPDATE)
 	{
-		/*
-		 * WITH CHECK OPTIONS wants a WCO node which wraps each Expr, so
-		 * create them as necessary.
-		 */
+		/* This should be the target relation */
+		Assert(rt_index == root->resultRelation);
+
+		add_with_check_options(rel, rt_index,
+							   commandType == CMD_INSERT ?
+							   WCO_RLS_INSERT_CHECK : WCO_RLS_UPDATE_CHECK,
+							   permissive_policies,
+							   restrictive_policies,
+							   withCheckOptions,
+							   hasSubLinks);
 
 		/*
-		 * Handle any restrictive policies first.
-		 *
-		 * They can simply be added.
+		 * For INSERT ... ON CONFLICT DO UPDATE we need additional policy
+		 * checks for the UPDATE which may be applied to the same RTE.
 		 */
-		if (hook_with_check_expr_restrictive)
+		if (commandType == CMD_INSERT &&
+			root->onConflict && root->onConflict->action == ONCONFLICT_UPDATE)
 		{
-			WithCheckOption *wco;
+			List	   *conflict_permissive_policies;
+			List	   *conflict_restrictive_policies;
+
+			/* Get the policies that apply to the auxiliary UPDATE */
+			get_policies_for_relation(rel, CMD_UPDATE, user_id,
+									  &conflict_permissive_policies,
+									  &conflict_restrictive_policies);
 
-			wco = (WithCheckOption *) makeNode(WithCheckOption);
-			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
-				WCO_RLS_UPDATE_CHECK;
-			wco->relname = pstrdup(RelationGetRelationName(rel));
-			wco->qual = (Node *) hook_with_check_expr_restrictive;
-			wco->cascaded = false;
-			*withCheckOptions = lappend(*withCheckOptions, wco);
+			/*
+			 * Enforce the USING clauses of the UPDATE policies using WCOs
+			 * rather than security quals.  This ensures that an error is
+			 * raised if the conflicting row cannot be updated due to RLS,
+			 * rather than the change being silently dropped.
+			 */
+			add_with_check_options(rel, rt_index,
+								   WCO_RLS_CONFLICT_CHECK,
+								   conflict_permissive_policies,
+								   conflict_restrictive_policies,
+								   withCheckOptions,
+								   hasSubLinks);
+
+			/* Enforce the WITH CHECK clauses of the UPDATE policies */
+			add_with_check_options(rel, rt_index,
+								   WCO_RLS_UPDATE_CHECK,
+								   conflict_permissive_policies,
+								   conflict_restrictive_policies,
+								   withCheckOptions,
+								   hasSubLinks);
 		}
+	}
 
-		/*
-		 * Handle built-in policies, if there are no permissive policies from
-		 * the hook.
-		 */
-		if (rowsec_with_check_expr && !hook_with_check_expr_permissive)
-		{
-			WithCheckOption *wco;
+	heap_close(rel, NoLock);
 
-			wco = (WithCheckOption *) makeNode(WithCheckOption);
-			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
-				WCO_RLS_UPDATE_CHECK;
-			wco->relname = pstrdup(RelationGetRelationName(rel));
-			wco->qual = (Node *) rowsec_with_check_expr;
-			wco->cascaded = false;
-			*withCheckOptions = lappend(*withCheckOptions, wco);
-		}
-		/* Handle the hook policies, if there are no built-in ones. */
-		else if (!rowsec_with_check_expr && hook_with_check_expr_permissive)
-		{
-			WithCheckOption *wco;
+	/*
+	 * Mark this query as having row security, so plancache can invalidate it
+	 * when necessary (eg: role changes)
+	 */
+	*hasRowSecurity = true;
 
-			wco = (WithCheckOption *) makeNode(WithCheckOption);
-			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
-				WCO_RLS_UPDATE_CHECK;
-			wco->relname = pstrdup(RelationGetRelationName(rel));
-			wco->qual = (Node *) hook_with_check_expr_permissive;
-			wco->cascaded = false;
-			*withCheckOptions = lappend(*withCheckOptions, wco);
-		}
-		/* Handle the case where there are both. */
-		else if (rowsec_with_check_expr && hook_with_check_expr_permissive)
-		{
-			WithCheckOption *wco;
-			List	   *combined_quals = NIL;
-			Expr	   *combined_qual_eval;
+	return;
+}
 
-			combined_quals = lcons(copyObject(rowsec_with_check_expr),
-								   combined_quals);
+/*
+ * get_policies_for_relation
+ *
+ * Returns lists of permissive and restrictive policies to be applied to the
+ * specified relation, based on the command type and role.
+ *
+ * This includes any policies added by extensions.
+ */
+static void
+get_policies_for_relation(Relation relation, CmdType cmd, Oid user_id,
+						  List **permissive_policies,
+						  List **restrictive_policies)
+{
+	ListCell   *item;
 
-			combined_quals = lcons(copyObject(hook_with_check_expr_permissive),
-								   combined_quals);
+	*permissive_policies = NIL;
+	*restrictive_policies = NIL;
 
-			combined_qual_eval = makeBoolExpr(OR_EXPR, combined_quals, -1);
+	/*
+	 * First find all internal policies for the relation.  CREATE POLICY does
+	 * not currently support defining restrictive policies, so for now all
+	 * internal policies are permissive.
+	 */
+	foreach(item, relation->rd_rsdesc->policies)
+	{
+		bool				cmd_matches = false;
+		RowSecurityPolicy  *policy = (RowSecurityPolicy *) lfirst(item);
 
-			wco = (WithCheckOption *) makeNode(WithCheckOption);
-			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
-				WCO_RLS_UPDATE_CHECK;
-			wco->relname = pstrdup(RelationGetRelationName(rel));
-			wco->qual = (Node *) combined_qual_eval;
-			wco->cascaded = false;
-			*withCheckOptions = lappend(*withCheckOptions, wco);
+		/* Always add ALL policies, if they exist. */
+		if (policy->polcmd == '*')
+			cmd_matches = true;
+		else
+		{
+			/* Check whether the policy applies to the specified command type */
+			switch (cmd)
+			{
+				case CMD_SELECT:
+					if (policy->polcmd == ACL_SELECT_CHR)
+						cmd_matches = true;
+					break;
+				case CMD_INSERT:
+					if (policy->polcmd == ACL_INSERT_CHR)
+						cmd_matches = true;
+					break;
+				case CMD_UPDATE:
+					if (policy->polcmd == ACL_UPDATE_CHR)
+						cmd_matches = true;
+					break;
+				case CMD_DELETE:
+					if (policy->polcmd == ACL_DELETE_CHR)
+						cmd_matches = true;
+					break;
+				default:
+					elog(ERROR, "unrecognized policy command type %d",
+						 (int) cmd);
+					break;
+			}
 		}
 
 		/*
-		 * ON CONFLICT DO UPDATE has an RTE that is subject to both INSERT and
-		 * UPDATE RLS enforcement.  Those are enforced (as a special, distinct
-		 * kind of WCO) on the target tuple.
-		 *
-		 * Make a second, recursive pass over the RTE for this, gathering
-		 * UPDATE-applicable RLS checks/WCOs, and gathering and converting
-		 * UPDATE-applicable security quals into WCO_RLS_CONFLICT_CHECK RLS
-		 * checks/WCOs.  Finally, these distinct kinds of RLS checks/WCOs are
-		 * concatenated with our own INSERT-applicable list.
+		 * Add this policy to the list of permissive policies if it
+		 * applies to the specified role.
 		 */
-		if (root->onConflict && root->onConflict->action == ONCONFLICT_UPDATE &&
-			commandType == CMD_INSERT)
-		{
-			List	   *conflictSecurityQuals = NIL;
-			List	   *conflictWCOs = NIL;
-			ListCell   *item;
-			bool		conflictHasRowSecurity = false;
-			bool		conflictHasSublinks = false;
-
-			/* Assume that RTE is target resultRelation */
-			get_row_security_policies(root, CMD_UPDATE, rte, rt_index,
-									  &conflictSecurityQuals, &conflictWCOs,
-									  &conflictHasRowSecurity,
-									  &conflictHasSublinks);
-
-			if (conflictHasRowSecurity)
-				*hasRowSecurity = true;
-			if (conflictHasSublinks)
-				*hasSubLinks = true;
+		if (cmd_matches && check_role_for_policy(policy->roles, user_id))
+			*permissive_policies = lappend(*permissive_policies, policy);
+	}
 
-			/*
-			 * Append WITH CHECK OPTIONs/RLS checks, which should not conflict
-			 * between this INSERT and the auxiliary UPDATE
-			 */
-			*withCheckOptions = list_concat(*withCheckOptions,
-											conflictWCOs);
+	/*
+	 * Then add any permissive or restrictive policies defined by extensions.
+	 * These are simply appended to the lists of internal policies, if they
+	 * apply to the specified role.
+	 */
+	if (row_security_policy_hook_restrictive)
+	{
+		List	   *hook_policies =
+			(*row_security_policy_hook_restrictive) (cmd, relation);
 
-			foreach(item, conflictSecurityQuals)
-			{
-				Expr	   *conflict_rowsec_expr = (Expr *) lfirst(item);
-				WithCheckOption *wco;
+		/*
+		 * We sort restrictive policies by name so that any WCOs they generate
+		 * are checked in a well-defined order.
+		 */
+		hook_policies = sort_policies_by_name(hook_policies);
 
-				wco = (WithCheckOption *) makeNode(WithCheckOption);
+		foreach(item, hook_policies)
+		{
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
 
-				wco->kind = WCO_RLS_CONFLICT_CHECK;
-				wco->relname = pstrdup(RelationGetRelationName(rel));
-				wco->qual = (Node *) copyObject(conflict_rowsec_expr);
-				wco->cascaded = false;
-				*withCheckOptions = lappend(*withCheckOptions, wco);
-			}
+			if (check_role_for_policy(policy->roles, user_id))
+				*restrictive_policies = lappend(*restrictive_policies, policy);
 		}
 	}
 
-	/* For SELECT, UPDATE, and DELETE, set the security quals */
-	if (commandType == CMD_SELECT
-		|| commandType == CMD_UPDATE
-		|| commandType == CMD_DELETE)
+	if (row_security_policy_hook_permissive)
 	{
-		/* restrictive policies can simply be added to the list first */
-		if (hook_expr_restrictive)
-			*securityQuals = lappend(*securityQuals, hook_expr_restrictive);
-
-		/* If we only have internal permissive, then just add those */
-		if (rowsec_expr && !hook_expr_permissive)
-			*securityQuals = lappend(*securityQuals, rowsec_expr);
-		/* .. and if we have only permissive policies from the hook */
-		else if (!rowsec_expr && hook_expr_permissive)
-			*securityQuals = lappend(*securityQuals, hook_expr_permissive);
-		/* if we have both, we have to combine them with an OR */
-		else if (rowsec_expr && hook_expr_permissive)
+		List	   *hook_policies =
+			(*row_security_policy_hook_permissive) (cmd, relation);
+
+		foreach(item, hook_policies)
 		{
-			List	   *combined_quals = NIL;
-			Expr	   *combined_qual_eval;
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
 
-			combined_quals = lcons(copyObject(rowsec_expr), combined_quals);
-			combined_quals = lcons(copyObject(hook_expr_permissive),
-								   combined_quals);
+			if (check_role_for_policy(policy->roles, user_id))
+				*permissive_policies = lappend(*permissive_policies, policy);
+		}
+	}
+}
 
-			combined_qual_eval = makeBoolExpr(OR_EXPR, combined_quals, -1);
+/*
+ * sort_policies_by_name
+ *
+ * This is only used for restrictive policies, ensuring that any
+ * WithCheckOptions they generate are applied in a well-defined order.
+ * This is not necessary for permissive policies, since they are all "OR"d
+ * together into a single WithCheckOption check.
+ */
+static List *
+sort_policies_by_name(List *policies)
+{
+	int			npol = list_length(policies);
+	RowSecurityPolicy *pols;
+	ListCell   *item;
+	int			ii = 0;
 
-			*securityQuals = lappend(*securityQuals, combined_qual_eval);
-		}
+	if (npol <= 1)
+		return policies;
+
+	pols = (RowSecurityPolicy *) palloc(sizeof(RowSecurityPolicy) * npol);
+
+	foreach(item, policies)
+	{
+		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+		pols[ii++] = *policy;
 	}
 
-	heap_close(rel, NoLock);
+	qsort(pols, npol, sizeof(RowSecurityPolicy), row_security_policy_cmp);
 
-	/*
-	 * Mark this query as having row security, so plancache can invalidate it
-	 * when necessary (eg: role changes)
-	 */
-	*hasRowSecurity = true;
+	policies = NIL;
+	for (ii = 0; ii < npol; ii++)
+		policies = lappend(policies, &pols[ii]);
 
-	return;
+	return policies;
+}
+
+/*
+ * qsort comparator to sort RowSecurityPolicy entries by name
+ */
+static int
+row_security_policy_cmp(const void *a, const void *b)
+{
+	const RowSecurityPolicy *pa = (const RowSecurityPolicy *) a;
+	const RowSecurityPolicy *pb = (const RowSecurityPolicy *) b;
+
+	/* Guard against NULL policy names from extensions */
+	if (pa->policy_name == NULL)
+		return pb->policy_name == NULL ? 0 : 1;
+	if (pb->policy_name == NULL)
+		return -1;
+
+	return strcmp(pa->policy_name, pb->policy_name);
 }
 
 /*
- * pull_row_security_policies
+ * add_security_quals
  *
- * Returns the list of policies to be added for this relation, based on the
- * type of command and the roles to which it applies, from the relation cache.
+ * Add security quals to enforce the specified RLS policies, restricting
+ * access to existing data in a table.  If there are no policies controlling
+ * access to the table, then all access is prohibited --- i.e., an implicit
+ * default-deny policy is used.
  *
+ * New security quals are added to securityQuals, and hasSubLinks is set to
+ * true if any of the quals added contain sublink subqueries.
  */
-static List *
-pull_row_security_policies(CmdType cmd, Relation relation, Oid user_id)
+static void
+add_security_quals(int rt_index,
+				   List *permissive_policies,
+				   List *restrictive_policies,
+				   List **securityQuals,
+				   bool *hasSubLinks)
 {
-	List	   *policies = NIL;
 	ListCell   *item;
+	List	   *permissive_quals = NIL;
+	Expr	   *rowsec_expr;
 
 	/*
-	 * Row security is enabled for the relation and the row security GUC is
-	 * either 'on' or 'force' here, so find the policies to apply to the
-	 * table. There must always be at least one policy defined (may be the
-	 * simple 'default-deny' policy, if none are explicitly defined on the
-	 * table).
+	 * First collect up the permissive quals.  If we do not find any permissive
+	 * policies then no rows are visible (this is handled below).
 	 */
-	foreach(item, relation->rd_rsdesc->policies)
+	foreach(item, permissive_policies)
 	{
 		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
 
-		/* Always add ALL policies, if they exist. */
-		if (policy->polcmd == '*' &&
-			check_role_for_policy(policy->roles, user_id))
-			policies = lcons(policy, policies);
-
-		/* Add relevant command-specific policies to the list. */
-		switch (cmd)
+		if (policy->qual != NULL)
 		{
-			case CMD_SELECT:
-				if (policy->polcmd == ACL_SELECT_CHR
-					&& check_role_for_policy(policy->roles, user_id))
-					policies = lcons(policy, policies);
-				break;
-			case CMD_INSERT:
-				/* If INSERT then only need to add the WITH CHECK qual */
-				if (policy->polcmd == ACL_INSERT_CHR
-					&& check_role_for_policy(policy->roles, user_id))
-					policies = lcons(policy, policies);
-				break;
-			case CMD_UPDATE:
-				if (policy->polcmd == ACL_UPDATE_CHR
-					&& check_role_for_policy(policy->roles, user_id))
-					policies = lcons(policy, policies);
-				break;
-			case CMD_DELETE:
-				if (policy->polcmd == ACL_DELETE_CHR
-					&& check_role_for_policy(policy->roles, user_id))
-					policies = lcons(policy, policies);
-				break;
-			default:
-				elog(ERROR, "unrecognized policy command type %d", (int) cmd);
-				break;
+			permissive_quals = lappend(permissive_quals,
+									   copyObject(policy->qual));
+			*hasSubLinks |= policy->hassublinks;
 		}
 	}
 
 	/*
-	 * There should always be a policy applied.  If there are none found then
-	 * create a simply defauly-deny policy (might be that policies exist but
-	 * that none of them apply to the role which is querying the table).
+	 * We must have permissive quals, always, or no rows are visible.
+	 *
+	 * If we do not, then we simply return a single 'false' qual which results
+	 * in no rows being visible.
 	 */
-	if (policies == NIL)
+	if (permissive_quals != NIL)
 	{
-		RowSecurityPolicy *policy = NULL;
-		Datum		role;
-
-		role = ObjectIdGetDatum(ACL_ID_PUBLIC);
-
-		policy = palloc0(sizeof(RowSecurityPolicy));
-		policy->policy_name = pstrdup("default-deny policy");
-		policy->policy_id = InvalidOid;
-		policy->polcmd = '*';
-		policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true,
-										'i');
-		policy->qual = (Expr *) makeConst(BOOLOID, -1, InvalidOid,
-										  sizeof(bool), BoolGetDatum(false),
-										  false, true);
-		policy->with_check_qual = copyObject(policy->qual);
-		policy->hassublinks = false;
-
-		policies = list_make1(policy);
-	}
+		/*
+		 * We now know that permissive policies exist, so we can now add
+		 * security quals based on the USING clauses from the restrictive
+		 * policies.  Since these need to be "AND"d together, we can
+		 * just add them one at a time.
+		 */
+		foreach(item, restrictive_policies)
+		{
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+			Expr	   *qual;
 
-	Assert(policies != NIL);
+			if (policy->qual != NULL)
+			{
+				qual = copyObject(policy->qual);
+				ChangeVarNodes((Node *) qual, 1, rt_index, 0);
 
-	return policies;
+				*securityQuals = lappend(*securityQuals, qual);
+				*hasSubLinks |= policy->hassublinks;
+			}
+		}
+
+		/*
+		 * Then add a single security qual "OR"ing together the USING clauses
+		 * from all the permissive policies.
+		 */
+		if (list_length(permissive_quals) == 1)
+			rowsec_expr = (Expr *) linitial(permissive_quals);
+		else
+			rowsec_expr = makeBoolExpr(OR_EXPR, permissive_quals, -1);
+
+		ChangeVarNodes((Node *) rowsec_expr, 1, rt_index, 0);
+		*securityQuals = lappend(*securityQuals, rowsec_expr);
+	}
+	else
+		/*
+		 * A permissive policy must exist for rows to be visible at all.
+		 * Therefore, if there were no permissive policies found, return a
+		 * single always-false clause.
+		 */
+		*securityQuals = lappend(*securityQuals,
+								 makeConst(BOOLOID, -1, InvalidOid,
+										   sizeof(bool), BoolGetDatum(false),
+										   false, true));
 }
 
 /*
- * process_policies
+ * add_with_check_options
  *
- * This will step through the policies which are passed in (which would come
- * from either the built-in ones created on a table, or from policies provided
- * by an extension through the hook provided), work out how to combine them,
- * rewrite them as necessary, and produce an Expr for the normal security
- * quals and an Expr for the with check quals.
+ * Add WithCheckOptions of the specified kind to check that new records
+ * added by an INSERT or UPDATE are consistent with the specified RLS
+ * policies.  Normally new data must satisfy the WITH CHECK clauses from the
+ * policies.  If a policy has no explicit WITH CHECK clause, its USING clause
+ * is used instead.  In the special case of an UPDATE arising from an
+ * INSERT ... ON CONFLICT DO UPDATE, existing records are first checked using
+ * a WCO_RLS_CONFLICT_CHECK WithCheckOption, which always uses the USING
+ * clauses from RLS policies.
  *
- * qual_eval, with_check_eval, and hassublinks are output variables
+ * New WCOs are added to withCheckOptions, and hasSubLinks is set to true if
+ * any of the check clauses added contain sublink subqueries.
  */
 static void
-process_policies(Query *root, List *policies, int rt_index, Expr **qual_eval,
-				 Expr **with_check_eval, bool *hassublinks,
-				 BoolExprType boolop)
+add_with_check_options(Relation rel,
+					   int rt_index,
+					   WCOKind kind,
+					   List *permissive_policies,
+					   List *restrictive_policies,
+					   List **withCheckOptions,
+					   bool *hasSubLinks)
 {
 	ListCell   *item;
-	List	   *quals = NIL;
-	List	   *with_check_quals = NIL;
+	List	   *permissive_quals = NIL;
+
+#define QUAL_FOR_WCO(policy) \
+	( kind != WCO_RLS_CONFLICT_CHECK &&	\
+	  (policy)->with_check_qual != NULL ? \
+	  (policy)->with_check_qual : (policy)->qual )
 
 	/*
-	 * Extract the USING and WITH CHECK quals from each of the policies and
-	 * add them to our lists.  We only want WITH CHECK quals if this RTE is
-	 * the query's result relation.
+	 * First collect up the permissive policy clauses, similar to
+	 * add_security_quals.
 	 */
-	foreach(item, policies)
+	foreach(item, permissive_policies)
 	{
 		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+		Expr	   *qual = QUAL_FOR_WCO(policy);
 
-		if (policy->qual != NULL)
-			quals = lcons(copyObject(policy->qual), quals);
-
-		if (policy->with_check_qual != NULL &&
-			rt_index == root->resultRelation)
-			with_check_quals = lcons(copyObject(policy->with_check_qual),
-									 with_check_quals);
+		if (qual != NULL)
+		{
+			permissive_quals = lappend(permissive_quals, copyObject(qual));
+			*hasSubLinks |= policy->hassublinks;
+		}
+	}
 
+	/*
+	 * There must be at least one permissive qual found or no rows are
+	 * allowed to be added.  This is the same as in add_security_quals.
+	 *
+	 * If there are no permissive_quals then we fall through and return a single
+	 * 'false' WCO, preventing all new rows.
+	 */
+	if (permissive_quals != NIL)
+	{
 		/*
-		 * For each policy, if there is only a USING clause then copy/use it
-		 * for the WITH CHECK policy also, if this RTE is the query's result
-		 * relation.
+		 * Add a single WithCheckOption for all the permissive policy clauses
+		 * "OR"d together.  This check has no policy name, since if the check
+		 * fails it means that no policy granted permission to perform the
+		 * update, rather than any particular policy being violated.
 		 */
-		if (policy->qual != NULL && policy->with_check_qual == NULL &&
-			rt_index == root->resultRelation)
-			with_check_quals = lcons(copyObject(policy->qual),
-									 with_check_quals);
+		WithCheckOption *wco;
 
+		wco = (WithCheckOption *) makeNode(WithCheckOption);
+		wco->kind = kind;
+		wco->relname = pstrdup(RelationGetRelationName(rel));
+		wco->polname = NULL;
+		wco->cascaded = false;
 
-		if (policy->hassublinks)
-			*hassublinks = true;
-	}
+		if (list_length(permissive_quals) == 1)
+			wco->qual = (Node *) linitial(permissive_quals);
+		else
+			wco->qual = (Node *) makeBoolExpr(OR_EXPR, permissive_quals, -1);
 
-	/*
-	 * If we end up without any normal quals (perhaps the only policy matched
-	 * was for INSERT), then create a single all-false one.
-	 */
-	if (quals == NIL)
-		quals = lcons(makeConst(BOOLOID, -1, InvalidOid, sizeof(bool),
-								BoolGetDatum(false), false, true), quals);
+		ChangeVarNodes(wco->qual, 1, rt_index, 0);
 
-	/*
-	 * Row security quals always have the target table as varno 1, as no joins
-	 * are permitted in row security expressions. We must walk the expression,
-	 * updating any references to varno 1 to the varno the table has in the
-	 * outer query.
-	 *
-	 * We rewrite the expression in-place.
-	 *
-	 * We must have some quals at this point; the default-deny policy, if
-	 * nothing else.  Note that we might not have any WITH CHECK quals- that's
-	 * fine, as this might not be the resultRelation.
-	 */
-	Assert(quals != NIL);
+		*withCheckOptions = lappend(*withCheckOptions, wco);
 
-	ChangeVarNodes((Node *) quals, 1, rt_index, 0);
+		/*
+		 * Now add WithCheckOptions for each of the restrictive policy clauses
+		 * (which will be "AND"d together).  We use a separate WithCheckOption
+		 * for each restrictive policy to allow the policy name to be included
+		 * in error reports if the policy is violated.
+		 */
+		foreach(item, restrictive_policies)
+		{
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+			Expr	   *qual = QUAL_FOR_WCO(policy);
+			WithCheckOption *wco;
 
-	if (with_check_quals != NIL)
-		ChangeVarNodes((Node *) with_check_quals, 1, rt_index, 0);
+			if (qual != NULL)
+			{
+				qual = copyObject(qual);
+				ChangeVarNodes((Node *) qual, 1, rt_index, 0);
 
-	/*
-	 * If more than one security qual is returned, then they need to be
-	 * combined together.
-	 */
-	if (list_length(quals) > 1)
-		*qual_eval = makeBoolExpr(boolop, quals, -1);
-	else
-		*qual_eval = (Expr *) linitial(quals);
+				wco = (WithCheckOption *) makeNode(WithCheckOption);
+				wco->kind = kind;
+				wco->relname = pstrdup(RelationGetRelationName(rel));
+				wco->polname = pstrdup(policy->policy_name);
+				wco->qual = (Node *) qual;
+				wco->cascaded = false;
 
-	/*
-	 * Similarly, if more than one WITH CHECK qual is returned, then they need
-	 * to be combined together.
-	 *
-	 * with_check_quals is allowed to be NIL here since this might not be the
-	 * resultRelation (see above).
-	 */
-	if (list_length(with_check_quals) > 1)
-		*with_check_eval = makeBoolExpr(boolop, with_check_quals, -1);
-	else if (with_check_quals != NIL)
-		*with_check_eval = (Expr *) linitial(with_check_quals);
+				*withCheckOptions = lappend(*withCheckOptions, wco);
+				*hasSubLinks |= policy->hassublinks;
+			}
+		}
+	}
 	else
-		*with_check_eval = NULL;
-
-	return;
+	{
+		/*
+		 * If there were no policy clauses to check new data, add a single
+		 * always-false WCO (a default-deny policy).
+		 */
+		WithCheckOption *wco;
+
+		wco = (WithCheckOption *) makeNode(WithCheckOption);
+		wco->kind = kind;
+		wco->relname = pstrdup(RelationGetRelationName(rel));
+		wco->polname = NULL;
+		wco->qual = (Node *) makeConst(BOOLOID, -1, InvalidOid,
+									   sizeof(bool), BoolGetDatum(false),
+									   false, true);
+		wco->cascaded = false;
+
+		*withCheckOptions = lappend(*withCheckOptions, wco);
+	}
 }
 
 /*
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 420ef3d..9c3d096 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -859,8 +859,6 @@ equalPolicy(RowSecurityPolicy *policy1, RowSecurityPolicy *policy2)
 		if (policy2 == NULL)
 			return false;
 
-		if (policy1->policy_id != policy2->policy_id)
-			return false;
 		if (policy1->polcmd != policy2->polcmd)
 			return false;
 		if (policy1->hassublinks != policy2->hassublinks)
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index f0dcd2f..940cc32 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -928,6 +928,7 @@ typedef struct WithCheckOption
 	NodeTag		type;
 	WCOKind		kind;			/* kind of WCO */
 	char	   *relname;		/* name of relation that specified the WCO */
+	char	   *polname;		/* name of RLS policy being checked */
 	Node	   *qual;			/* constraint qual to check */
 	bool		cascaded;		/* true for a cascaded WCO on a view */
 } WithCheckOption;
diff --git a/src/include/rewrite/rowsecurity.h b/src/include/rewrite/rowsecurity.h
index 523c56e..4af244d 100644
--- a/src/include/rewrite/rowsecurity.h
+++ b/src/include/rewrite/rowsecurity.h
@@ -19,7 +19,6 @@
 
 typedef struct RowSecurityPolicy
 {
-	Oid			policy_id;		/* OID of the policy */
 	char	   *policy_name;	/* Name of the policy */
 	char		polcmd;			/* Type of command policy is for */
 	ArrayType  *roles;			/* Array of roles policy is for */
@@ -41,7 +40,7 @@ extern PGDLLIMPORT row_security_policy_hook_type row_security_policy_hook_permis
 
 extern PGDLLIMPORT row_security_policy_hook_type row_security_policy_hook_restrictive;
 
-extern void get_row_security_policies(Query *root, CmdType commandType,
+extern void get_row_security_policies(Query *root,
 						  RangeTblEntry *rte, int rt_index,
 						  List **securityQuals, List **withCheckOptions,
 						  bool *hasRowSecurity, bool *hasSubLinks);
diff --git a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
index 4587eb0..8885464 100644
--- a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
+++ b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
@@ -83,7 +83,7 @@ SELECT * FROM rls_test_restrictive;
 INSERT INTO rls_test_restrictive VALUES ('r1','s1',10);
 -- failure
 INSERT INTO rls_test_restrictive VALUES ('r4','s4',10);
-ERROR:  new row violates row level security policy for "rls_test_restrictive"
+ERROR:  new row violates row level security policy "extension policy" for "rls_test_restrictive"
 SET ROLE s1;
 -- With only the hook's policies, both
 -- permissive hook's policy is current_user = username
@@ -124,7 +124,7 @@ EXPLAIN (costs off) SELECT * FROM rls_test_permissive;
                           QUERY PLAN                           
 ---------------------------------------------------------------
  Seq Scan on rls_test_permissive
-   Filter: (("current_user"() = username) OR ((data % 2) = 0))
+   Filter: (((data % 2) = 0) OR ("current_user"() = username))
 (2 rows)
 
 SELECT * FROM rls_test_permissive;
@@ -163,7 +163,7 @@ SELECT * FROM rls_test_restrictive;
 INSERT INTO rls_test_restrictive VALUES ('r1','s1',8);
 -- failure
 INSERT INTO rls_test_restrictive VALUES ('r3','s3',10);
-ERROR:  new row violates row level security policy for "rls_test_restrictive"
+ERROR:  new row violates row level security policy "extension policy" for "rls_test_restrictive"
 -- failure
 INSERT INTO rls_test_restrictive VALUES ('r1','s1',7);
 ERROR:  new row violates row level security policy for "rls_test_restrictive"
@@ -176,7 +176,7 @@ EXPLAIN (costs off) SELECT * FROM rls_test_both;
                                         QUERY PLAN                                         
 -------------------------------------------------------------------------------------------
  Subquery Scan on rls_test_both
-   Filter: (("current_user"() = rls_test_both.username) OR ((rls_test_both.data % 2) = 0))
+   Filter: (((rls_test_both.data % 2) = 0) OR ("current_user"() = rls_test_both.username))
    ->  Seq Scan on rls_test_both rls_test_both_1
          Filter: ("current_user"() = supervisor)
 (4 rows)
@@ -190,7 +190,7 @@ SELECT * FROM rls_test_both;
 INSERT INTO rls_test_both VALUES ('r1','s1',8);
 -- failure
 INSERT INTO rls_test_both VALUES ('r3','s3',10);
-ERROR:  new row violates row level security policy for "rls_test_both"
+ERROR:  new row violates row level security policy "extension policy" for "rls_test_both"
 -- failure
 INSERT INTO rls_test_both VALUES ('r1','s1',7);
 ERROR:  new row violates row level security policy for "rls_test_both"
diff --git a/src/test/modules/test_rls_hooks/test_rls_hooks.c b/src/test/modules/test_rls_hooks/test_rls_hooks.c
index b96dbff..cc865cd 100644
--- a/src/test/modules/test_rls_hooks/test_rls_hooks.c
+++ b/src/test/modules/test_rls_hooks/test_rls_hooks.c
@@ -87,7 +87,6 @@ test_rls_hooks_permissive(CmdType cmdtype, Relation relation)
 	role = ObjectIdGetDatum(ACL_ID_PUBLIC);
 
 	policy->policy_name = pstrdup("extension policy");
-	policy->policy_id = InvalidOid;
 	policy->polcmd = '*';
 	policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true, 'i');
 
@@ -151,7 +150,6 @@ test_rls_hooks_restrictive(CmdType cmdtype, Relation relation)
 	role = ObjectIdGetDatum(ACL_ID_PUBLIC);
 
 	policy->policy_name = pstrdup("extension policy");
-	policy->policy_id = InvalidOid;
 	policy->polcmd = '*';
 	policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true, 'i');
 
-- 
1.9.1


From 3541443191ddefe0d5c7ba7753c0fb7778975c40 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Mon, 14 Sep 2015 08:57:47 -0400
Subject: [PATCH 2/2] Enforce ALL/SELECT policies in RETURNING for RLS

For the UPDATE/DELETE RETURNING case, filter the records which are not
visible to the user through ALL or SELECT policies from those considered
for the UPDATE or DELETE.  This is similar to how the GRANT system
works, which prevents RETURNING unless the caller has SELECT rights on
the relation.

Per discussion with Robert, Dean, Tom, and Kevin.

Back-patch to 9.5 where RLS was introduced.
---
 src/backend/rewrite/rowsecurity.c         | 52 +++++++++++++++++++++++++++++++
 src/test/regress/expected/rowsecurity.out | 50 +++++++++++++++--------------
 2 files changed, 79 insertions(+), 23 deletions(-)

diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index b96c29d..c93a5cf 100644
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -109,10 +109,13 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
 {
 	Oid			user_id;
 	int			rls_status;
+	bool		returning;
 	Relation	rel;
 	CmdType		commandType;
 	List	   *permissive_policies;
 	List	   *restrictive_policies;
+	List	   *returning_permissive_policies;
+	List	   *returning_restrictive_policies;
 
 	/* Defaults for the return values */
 	*securityQuals = NIL;
@@ -169,6 +172,24 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
 							  &restrictive_policies);
 
 	/*
+	 * For the target relation, when there is a returning list, we need to
+	 * collect up CMD_SELECT policies to add via add_security_quals and
+	 * add_with_check_options.  This is because, for the RETURNING case, we
+	 * have to filter any records which are not visible through an ALL or SELECT
+	 * USING policy.
+	 *
+	 * We don't need to worry about the non-target relation case because we are
+	 * checking the ALL and SELECT policies for those relations anyway (see
+	 * above).
+	 */
+	returning = rt_index == root->resultRelation && root->returningList != NIL;
+
+	if (returning && (commandType == CMD_UPDATE || commandType == CMD_DELETE))
+		get_policies_for_relation(rel, CMD_SELECT, user_id,
+								  &returning_permissive_policies,
+								  &returning_restrictive_policies);
+
+	/*
 	 * For SELECT, UPDATE and DELETE, add security quals to enforce these
 	 * policies.  These security quals control access to existing table rows.
 	 * Restrictive policies are "AND"d together, and permissive policies are
@@ -187,6 +208,17 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
 						   hasSubLinks);
 
 	/*
+	 * For UPDATE and DELETE where there is a RETURNING, add the returning
+	 * policies (ALL and SELECT USING policies).
+	 */
+	if (returning && (commandType == CMD_UPDATE || commandType == CMD_DELETE))
+		add_security_quals(rt_index,
+						   returning_permissive_policies,
+						   returning_restrictive_policies,
+						   securityQuals,
+						   hasSubLinks);
+
+	/*
 	 * For INSERT and UPDATE, add withCheckOptions to verify that any new
 	 * records added are consistent with the security policies.  This will use
 	 * each policy's WITH CHECK clause, or its USING clause if no explicit
@@ -214,12 +246,20 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
 		{
 			List	   *conflict_permissive_policies;
 			List	   *conflict_restrictive_policies;
+			List	   *conflict_returning_permissive_policies = NIL;
+			List	   *conflict_returning_restrictive_policies = NIL;
 
 			/* Get the policies that apply to the auxiliary UPDATE */
 			get_policies_for_relation(rel, CMD_UPDATE, user_id,
 									  &conflict_permissive_policies,
 									  &conflict_restrictive_policies);
 
+			/* Get the ALL/SELECT policies, if there is a RETURNING clause */
+			if (returning)
+				get_policies_for_relation(rel, CMD_SELECT, user_id,
+									  &conflict_returning_permissive_policies,
+									  &conflict_returning_restrictive_policies);
+
 			/*
 			 * Enforce the USING clauses of the UPDATE policies using WCOs
 			 * rather than security quals.  This ensures that an error is
@@ -233,6 +273,18 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
 								   withCheckOptions,
 								   hasSubLinks);
 
+			/*
+			 * Add returning policies also as WCO policies, again, to avoid
+			 * silently dropping data.
+			 */
+			if (returning)
+				add_with_check_options(rel, rt_index,
+									   WCO_RLS_CONFLICT_CHECK,
+									   conflict_returning_permissive_policies,
+									   conflict_returning_restrictive_policies,
+									   withCheckOptions,
+									   hasSubLinks);
+
 			/* Enforce the WITH CHECK clauses of the UPDATE policies */
 			add_with_check_options(rel, rt_index,
 								   WCO_RLS_UPDATE_CHECK,
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 6fc80af..7628e52 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -1246,21 +1246,23 @@ NOTICE:  f_leak => cde
 EXPLAIN (COSTS OFF) UPDATE t2 t2_1 SET b = t2_2.b FROM t2 t2_2
 WHERE t2_1.a = 3 AND t2_2.a = t2_1.a AND t2_2.b = t2_1.b
 AND f_leak(t2_1.b) AND f_leak(t2_2.b) RETURNING *, t2_1, t2_2;
-                          QUERY PLAN                           
----------------------------------------------------------------
+                             QUERY PLAN                              
+---------------------------------------------------------------------
  Update on t2 t2_1_1
    ->  Nested Loop
          Join Filter: (t2_1.b = t2_2.b)
          ->  Subquery Scan on t2_1
                Filter: f_leak(t2_1.b)
-               ->  LockRows
-                     ->  Seq Scan on t2 t2_1_2
-                           Filter: ((a = 3) AND ((a % 2) = 1))
+               ->  Subquery Scan on t2_1_2
+                     Filter: ((t2_1_2.a % 2) = 1)
+                     ->  LockRows
+                           ->  Seq Scan on t2 t2_1_3
+                                 Filter: ((a = 3) AND ((a % 2) = 1))
          ->  Subquery Scan on t2_2
                Filter: f_leak(t2_2.b)
                ->  Seq Scan on t2 t2_2_1
                      Filter: ((a = 3) AND ((a % 2) = 1))
-(12 rows)
+(14 rows)
 
 UPDATE t2 t2_1 SET b = t2_2.b FROM t2 t2_2
 WHERE t2_1.a = 3 AND t2_2.a = t2_1.a AND t2_2.b = t2_1.b
@@ -1275,8 +1277,8 @@ NOTICE:  f_leak => cde
 EXPLAIN (COSTS OFF) UPDATE t1 t1_1 SET b = t1_2.b FROM t1 t1_2
 WHERE t1_1.a = 4 AND t1_2.a = t1_1.a AND t1_2.b = t1_1.b
 AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
-                          QUERY PLAN                           
----------------------------------------------------------------
+                             QUERY PLAN                              
+---------------------------------------------------------------------
  Update on t1 t1_1_3
    Update on t1 t1_1_3
    Update on t2 t1_1
@@ -1285,9 +1287,11 @@ AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
          Join Filter: (t1_1.b = t1_2.b)
          ->  Subquery Scan on t1_1
                Filter: f_leak(t1_1.b)
-               ->  LockRows
-                     ->  Seq Scan on t1 t1_1_4
-                           Filter: ((a = 4) AND ((a % 2) = 0))
+               ->  Subquery Scan on t1_1_4
+                     Filter: ((t1_1_4.a % 2) = 0)
+                     ->  LockRows
+                           ->  Seq Scan on t1 t1_1_5
+                                 Filter: ((a = 4) AND ((a % 2) = 0))
          ->  Subquery Scan on t1_2
                Filter: f_leak(t1_2.b)
                ->  Append
@@ -1301,9 +1305,11 @@ AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
          Join Filter: (t1_1_1.b = t1_2_1.b)
          ->  Subquery Scan on t1_1_1
                Filter: f_leak(t1_1_1.b)
-               ->  LockRows
-                     ->  Seq Scan on t2 t1_1_5
-                           Filter: ((a = 4) AND ((a % 2) = 0))
+               ->  Subquery Scan on t1_1_6
+                     Filter: ((t1_1_6.a % 2) = 0)
+                     ->  LockRows
+                           ->  Seq Scan on t2 t1_1_7
+                                 Filter: ((a = 4) AND ((a % 2) = 0))
          ->  Subquery Scan on t1_2_1
                Filter: f_leak(t1_2_1.b)
                ->  Append
@@ -1317,9 +1323,11 @@ AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
          Join Filter: (t1_1_2.b = t1_2_2.b)
          ->  Subquery Scan on t1_1_2
                Filter: f_leak(t1_1_2.b)
-               ->  LockRows
-                     ->  Seq Scan on t3 t1_1_6
-                           Filter: ((a = 4) AND ((a % 2) = 0))
+               ->  Subquery Scan on t1_1_8
+                     Filter: ((t1_1_8.a % 2) = 0)
+                     ->  LockRows
+                           ->  Seq Scan on t3 t1_1_9
+                                 Filter: ((a = 4) AND ((a % 2) = 0))
          ->  Subquery Scan on t1_2_2
                Filter: f_leak(t1_2_2.b)
                ->  Append
@@ -1329,7 +1337,7 @@ AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
                            Filter: ((a = 4) AND ((a % 2) = 0))
                      ->  Seq Scan on t3 t1_2_11
                            Filter: ((a = 4) AND ((a % 2) = 0))
-(52 rows)
+(58 rows)
 
 UPDATE t1 t1_1 SET b = t1_2.b FROM t1 t1_2
 WHERE t1_1.a = 4 AND t1_2.a = t1_1.a AND t1_2.b = t1_1.b
@@ -1960,8 +1968,6 @@ NOTICE:  f_leak => fgh_updt
 (6 rows)
 
 DELETE FROM x1 WHERE f_leak(b) RETURNING *;
-NOTICE:  f_leak => abc_updt
-NOTICE:  f_leak => efg_updt
 NOTICE:  f_leak => cde_updt
 NOTICE:  f_leak => fgh_updt
 NOTICE:  f_leak => bcd_updt_updt
@@ -1970,15 +1976,13 @@ NOTICE:  f_leak => fgh_updt_updt
 NOTICE:  f_leak => fgh_updt_updt
  a |       b       |         c         
 ---+---------------+-------------------
- 1 | abc_updt      | rls_regress_user1
- 5 | efg_updt      | rls_regress_user1
  3 | cde_updt      | rls_regress_user2
  7 | fgh_updt      | rls_regress_user2
  2 | bcd_updt_updt | rls_regress_user1
  4 | def_updt_updt | rls_regress_user2
  6 | fgh_updt_updt | rls_regress_user1
  8 | fgh_updt_updt | rls_regress_user2
-(8 rows)
+(6 rows)
 
 --
 -- Duplicate Policy Names
-- 
1.9.1

From f6866952d2b5049acf8eecb45b990c3ab4916f04 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Sun, 13 Sep 2015 09:03:09 -0400
Subject: [PATCH 1/2] RLS refactoring

This refactors rewrite/rowsecurity.c to simplify the handling of the
default deny case (reducing the number of places where we check for and
add the default deny policy from three to one) by splitting up the
retrival of the policies from the application of them.

This also allowed us to do away with the policy_id field.  A policy_name
field was added for WithCheckOption policies and is used in error
reporting, when available.

Patch by Dean Rasheed, with various mostly cosmetic changes by me.

Back-patch to 9.5 where RLS was introduced to avoid unnecessary
differences, since we're still in alpha, per discussion with Robert.
---
 src/backend/commands/policy.c                      |  41 --
 src/backend/executor/execMain.c                    |  20 +-
 src/backend/nodes/copyfuncs.c                      |   1 +
 src/backend/nodes/equalfuncs.c                     |   1 +
 src/backend/nodes/outfuncs.c                       |   1 +
 src/backend/nodes/readfuncs.c                      |   1 +
 src/backend/rewrite/rewriteHandler.c               |   5 +-
 src/backend/rewrite/rowsecurity.c                  | 816 +++++++++++----------
 src/backend/utils/cache/relcache.c                 |   2 -
 src/include/nodes/parsenodes.h                     |   1 +
 src/include/rewrite/rowsecurity.h                  |   3 +-
 .../test_rls_hooks/expected/test_rls_hooks.out     |  10 +-
 src/test/modules/test_rls_hooks/test_rls_hooks.c   |   2 -
 13 files changed, 447 insertions(+), 457 deletions(-)

diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 45326a3..8851fe7 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -186,9 +186,6 @@ policy_role_list_to_array(List *roles, int *num_roles)
 /*
  * Load row security policy from the catalog, and store it in
  * the relation's relcache entry.
- *
- * We will always set up some kind of policy here.  If no explicit policies
- * are found then an implicit default-deny policy is created.
  */
 void
 RelationBuildRowSecurity(Relation relation)
@@ -246,7 +243,6 @@ RelationBuildRowSecurity(Relation relation)
 			char	   *with_check_value;
 			Expr	   *with_check_qual;
 			char	   *policy_name_value;
-			Oid			policy_id;
 			bool		isnull;
 			RowSecurityPolicy *policy;
 
@@ -298,14 +294,11 @@ RelationBuildRowSecurity(Relation relation)
 			else
 				with_check_qual = NULL;
 
-			policy_id = HeapTupleGetOid(tuple);
-
 			/* Now copy everything into the cache context */
 			MemoryContextSwitchTo(rscxt);
 
 			policy = palloc0(sizeof(RowSecurityPolicy));
 			policy->policy_name = pstrdup(policy_name_value);
-			policy->policy_id = policy_id;
 			policy->polcmd = cmd_value;
 			policy->roles = DatumGetArrayTypePCopy(roles_datum);
 			policy->qual = copyObject(qual_expr);
@@ -326,40 +319,6 @@ RelationBuildRowSecurity(Relation relation)
 
 		systable_endscan(sscan);
 		heap_close(catalog, AccessShareLock);
-
-		/*
-		 * Check if no policies were added
-		 *
-		 * If no policies exist in pg_policy for this relation, then we need
-		 * to create a single default-deny policy.  We use InvalidOid for the
-		 * Oid to indicate that this is the default-deny policy (we may decide
-		 * to ignore the default policy if an extension adds policies).
-		 */
-		if (rsdesc->policies == NIL)
-		{
-			RowSecurityPolicy *policy;
-			Datum		role;
-
-			MemoryContextSwitchTo(rscxt);
-
-			role = ObjectIdGetDatum(ACL_ID_PUBLIC);
-
-			policy = palloc0(sizeof(RowSecurityPolicy));
-			policy->policy_name = pstrdup("default-deny policy");
-			policy->policy_id = InvalidOid;
-			policy->polcmd = '*';
-			policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true,
-											'i');
-			policy->qual = (Expr *) makeConst(BOOLOID, -1, InvalidOid,
-										   sizeof(bool), BoolGetDatum(false),
-											  false, true);
-			policy->with_check_qual = copyObject(policy->qual);
-			policy->hassublinks = false;
-
-			rsdesc->policies = lcons(policy, rsdesc->policies);
-
-			MemoryContextSwitchTo(oldcxt);
-		}
 	}
 	PG_CATCH();
 	{
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 2c65a90..c28eb2b 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1815,14 +1815,26 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
 					break;
 				case WCO_RLS_INSERT_CHECK:
 				case WCO_RLS_UPDATE_CHECK:
-					ereport(ERROR,
-							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					if (wco->polname != NULL)
+						ereport(ERROR,
+								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+							 errmsg("new row violates row level security policy \"%s\" for \"%s\"",
+									wco->polname, wco->relname)));
+					else
+						ereport(ERROR,
+								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 							 errmsg("new row violates row level security policy for \"%s\"",
 									wco->relname)));
 					break;
 				case WCO_RLS_CONFLICT_CHECK:
-					ereport(ERROR,
-							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					if (wco->polname != NULL)
+						ereport(ERROR,
+								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+							 errmsg("new row violates row level security policy \"%s\" (USING expression) for \"%s\"",
+									wco->polname, wco->relname)));
+					else
+						ereport(ERROR,
+								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 							 errmsg("new row violates row level security policy (USING expression) for \"%s\"",
 									wco->relname)));
 					break;
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index bd2e80e..1c801f5 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2168,6 +2168,7 @@ _copyWithCheckOption(const WithCheckOption *from)
 
 	COPY_SCALAR_FIELD(kind);
 	COPY_STRING_FIELD(relname);
+	COPY_STRING_FIELD(polname);
 	COPY_NODE_FIELD(qual);
 	COPY_SCALAR_FIELD(cascaded);
 
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 19412fe..8f16833 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2455,6 +2455,7 @@ _equalWithCheckOption(const WithCheckOption *a, const WithCheckOption *b)
 {
 	COMPARE_SCALAR_FIELD(kind);
 	COMPARE_STRING_FIELD(relname);
+	COMPARE_STRING_FIELD(polname);
 	COMPARE_NODE_FIELD(qual);
 	COMPARE_SCALAR_FIELD(cascaded);
 
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index a878498..79b7179 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2403,6 +2403,7 @@ _outWithCheckOption(StringInfo str, const WithCheckOption *node)
 
 	WRITE_ENUM_FIELD(kind, WCOKind);
 	WRITE_STRING_FIELD(relname);
+	WRITE_STRING_FIELD(polname);
 	WRITE_NODE_FIELD(qual);
 	WRITE_BOOL_FIELD(cascaded);
 }
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 23e0b36..df55b76 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -270,6 +270,7 @@ _readWithCheckOption(void)
 
 	READ_ENUM_FIELD(kind, WCOKind);
 	READ_STRING_FIELD(relname);
+	READ_STRING_FIELD(polname);
 	READ_NODE_FIELD(qual);
 	READ_BOOL_FIELD(cascaded);
 
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index db3c2c7..1b8e7b0 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1786,8 +1786,8 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
 		/*
 		 * Fetch any new security quals that must be applied to this RTE.
 		 */
-		get_row_security_policies(parsetree, parsetree->commandType, rte,
-								  rt_index, &securityQuals, &withCheckOptions,
+		get_row_security_policies(parsetree, rte, rt_index,
+								  &securityQuals, &withCheckOptions,
 								  &hasRowSecurity, &hasSubLinks);
 
 		if (securityQuals != NIL || withCheckOptions != NIL)
@@ -3026,6 +3026,7 @@ rewriteTargetView(Query *parsetree, Relation view)
 			wco = makeNode(WithCheckOption);
 			wco->kind = WCO_VIEW_CHECK;
 			wco->relname = pstrdup(RelationGetRelationName(view));
+			wco->polname = NULL;
 			wco->qual = NULL;
 			wco->cascaded = cascaded;
 
diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index 5a81db3..b96c29d 100644
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -13,11 +13,12 @@
  * Any part of the system which is returning records back to the user, or
  * which is accepting records from the user to add to a table, needs to
  * consider the policies associated with the table (if any).  For normal
- * queries, this is handled by calling prepend_row_security_policies() during
- * rewrite, which looks at each RTE and adds the expressions defined by the
- * policies to the securityQuals list for the RTE.  For queries which modify
- * the relation, any WITH CHECK policies are added to the list of
- * WithCheckOptions for the Query and checked against each row which is being
+ * queries, this is handled by calling get_row_security_policies() during
+ * rewrite, for each RTE in the query.  This returns the expressions defined
+ * by the table's policies as a list that is prepended to the securityQuals
+ * list for the RTE.  For queries which modify the table, any WITH CHECK
+ * clauses from the table's policies are also returned and prepended to the
+ * list of WithCheckOptions for the Query to check each row that is being
  * added to the table.  Other parts of the system (eg: COPY) simply construct
  * a normal query and use that, if RLS is to be applied.
  *
@@ -56,13 +57,29 @@
 #include "utils/syscache.h"
 #include "tcop/utility.h"
 
-static List *pull_row_security_policies(CmdType cmd, Relation relation,
-						   Oid user_id);
-static void process_policies(Query *root, List *policies, int rt_index,
-				 Expr **final_qual,
-				 Expr **final_with_check_qual,
-				 bool *hassublinks,
-				 BoolExprType boolop);
+static void get_policies_for_relation(Relation relation,
+						  CmdType cmd, Oid user_id,
+						  List **permissive_policies,
+						  List **restrictive_policies);
+
+static List *sort_policies_by_name(List *policies);
+
+static int row_security_policy_cmp(const void *a, const void *b);
+
+static void add_security_quals(int rt_index,
+							   List *permissive_policies,
+							   List *restrictive_policies,
+							   List **securityQuals,
+							   bool *hasSubLinks);
+
+static void add_with_check_options(Relation rel,
+								   int rt_index,
+								   WCOKind kind,
+								   List *permissive_policies,
+								   List *restrictive_policies,
+								   List **withCheckOptions,
+								   bool *hasSubLinks);
+
 static bool check_role_for_policy(ArrayType *policy_roles, Oid user_id);
 
 /*
@@ -73,42 +90,29 @@ static bool check_role_for_policy(ArrayType *policy_roles, Oid user_id);
  *
  * row_security_policy_hook_restrictive can be used to add policies which
  * are enforced, regardless of other policies (they are "AND"d).
- *
- * See below where the hook is called in prepend_row_security_policies for
- * insight into how to use this hook.
  */
 row_security_policy_hook_type row_security_policy_hook_permissive = NULL;
 row_security_policy_hook_type row_security_policy_hook_restrictive = NULL;
 
 /*
- * Get any row security quals and check quals that should be applied to the
- * specified RTE.
+ * Get any row security quals and WithCheckOption checks that should be
+ * applied to the specified RTE.
  *
  * In addition, hasRowSecurity is set to true if row level security is enabled
  * (even if this RTE doesn't have any row security quals), and hasSubLinks is
  * set to true if any of the quals returned contain sublinks.
  */
 void
-get_row_security_policies(Query *root, CmdType commandType, RangeTblEntry *rte,
-						  int rt_index, List **securityQuals,
-						  List **withCheckOptions, bool *hasRowSecurity,
-						  bool *hasSubLinks)
+get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
+						  List **securityQuals, List **withCheckOptions,
+						  bool *hasRowSecurity, bool *hasSubLinks)
 {
-	Expr	   *rowsec_expr = NULL;
-	Expr	   *rowsec_with_check_expr = NULL;
-	Expr	   *hook_expr_restrictive = NULL;
-	Expr	   *hook_with_check_expr_restrictive = NULL;
-	Expr	   *hook_expr_permissive = NULL;
-	Expr	   *hook_with_check_expr_permissive = NULL;
-
-	List	   *rowsec_policies;
-	List	   *hook_policies_restrictive = NIL;
-	List	   *hook_policies_permissive = NIL;
-
-	Relation	rel;
 	Oid			user_id;
 	int			rls_status;
-	bool		defaultDeny = false;
+	Relation	rel;
+	CmdType		commandType;
+	List	   *permissive_policies;
+	List	   *restrictive_policies;
 
 	/* Defaults for the return values */
 	*securityQuals = NIL;
@@ -157,465 +161,479 @@ get_row_security_policies(Query *root, CmdType commandType, RangeTblEntry *rte,
 	 * policies and t2's SELECT policies.
 	 */
 	rel = heap_open(rte->relid, NoLock);
-	if (rt_index != root->resultRelation)
-		commandType = CMD_SELECT;
-
-	rowsec_policies = pull_row_security_policies(commandType, rel,
-												 user_id);
 
-	/*
-	 * Check if this is only the default-deny policy.
-	 *
-	 * Normally, if the table has row security enabled but there are no
-	 * policies, we use a default-deny policy and not allow anything. However,
-	 * when an extension uses the hook to add their own policies, we don't
-	 * want to include the default deny policy or there won't be any way for a
-	 * user to use an extension exclusively for the policies to be used.
-	 */
-	if (((RowSecurityPolicy *) linitial(rowsec_policies))->policy_id
-		== InvalidOid)
-		defaultDeny = true;
+	commandType = rt_index == root->resultRelation ?
+				  root->commandType : CMD_SELECT;
 
-	/* Now that we have our policies, build the expressions from them. */
-	process_policies(root, rowsec_policies, rt_index, &rowsec_expr,
-					 &rowsec_with_check_expr, hasSubLinks, OR_EXPR);
+	get_policies_for_relation(rel, commandType, user_id, &permissive_policies,
+							  &restrictive_policies);
 
 	/*
-	 * Also, allow extensions to add their own policies.
-	 *
-	 * extensions can add either permissive or restrictive policies.
+	 * For SELECT, UPDATE and DELETE, add security quals to enforce these
+	 * policies.  These security quals control access to existing table rows.
+	 * Restrictive policies are "AND"d together, and permissive policies are
+	 * "OR"d together.
 	 *
-	 * Note that, as with the internal policies, if multiple policies are
-	 * returned then they will be combined into a single expression with all
-	 * of them OR'd (for permissive) or AND'd (for restrictive) together.
-	 *
-	 * If only a USING policy is returned by the extension then it will be
-	 * used for WITH CHECK as well, similar to how internal policies are
-	 * handled.
-	 *
-	 * The only caveat to this is that if there are NO internal policies
-	 * defined, there ARE policies returned by the extension, and RLS is
-	 * enabled on the table, then we will ignore the internally-generated
-	 * default-deny policy and use only the policies returned by the
-	 * extension.
+	 * If there are no policy clauses controlling access to the table, this
+	 * will add a single always-false clause (a default-deny policy).
 	 */
-	if (row_security_policy_hook_restrictive)
-	{
-		hook_policies_restrictive = (*row_security_policy_hook_restrictive) (commandType, rel);
-
-		/* Build the expression from any policies returned. */
-		if (hook_policies_restrictive != NIL)
-			process_policies(root, hook_policies_restrictive, rt_index,
-							 &hook_expr_restrictive,
-							 &hook_with_check_expr_restrictive,
-							 hasSubLinks,
-							 AND_EXPR);
-	}
-
-	if (row_security_policy_hook_permissive)
-	{
-		hook_policies_permissive = (*row_security_policy_hook_permissive) (commandType, rel);
-
-		/* Build the expression from any policies returned. */
-		if (hook_policies_permissive != NIL)
-			process_policies(root, hook_policies_permissive, rt_index,
-							 &hook_expr_permissive,
-							 &hook_with_check_expr_permissive, hasSubLinks,
-							 OR_EXPR);
-	}
+	if (commandType == CMD_SELECT ||
+		commandType == CMD_UPDATE ||
+		commandType == CMD_DELETE)
+		add_security_quals(rt_index,
+						   permissive_policies,
+						   restrictive_policies,
+						   securityQuals,
+						   hasSubLinks);
 
 	/*
-	 * If the only built-in policy is the default-deny one, and permissive hook
-	 * policies exist, then use the hook policies only and do not apply the
-	 * default-deny policy.  Otherwise, we will apply both sets below.
-	 *
-	 * Note that we do not remove the defaultDeny policy if only *restrictive*
-	 * policies exist as restrictive policies should only ever be reducing what
-	 * is visible.  Therefore, at least one permissive policy must exist which
-	 * allows records to be seen before restrictive policies can remove rows
-	 * from that set.  A single "true" policy can be created to address this
-	 * requirement, if necessary.
-	 */
-	if (defaultDeny && hook_policies_permissive != NIL)
-	{
-		rowsec_expr = NULL;
-		rowsec_with_check_expr = NULL;
-	}
-
-	/*
-	 * For INSERT or UPDATE, we need to add the WITH CHECK quals to Query's
-	 * withCheckOptions to verify that any new records pass the WITH CHECK
-	 * policy (this will be a copy of the USING policy, if no explicit WITH
-	 * CHECK policy exists).
+	 * For INSERT and UPDATE, add withCheckOptions to verify that any new
+	 * records added are consistent with the security policies.  This will use
+	 * each policy's WITH CHECK clause, or its USING clause if no explicit
+	 * WITH CHECK clause is defined.
 	 */
 	if (commandType == CMD_INSERT || commandType == CMD_UPDATE)
 	{
-		/*
-		 * WITH CHECK OPTIONS wants a WCO node which wraps each Expr, so
-		 * create them as necessary.
-		 */
+		/* This should be the target relation */
+		Assert(rt_index == root->resultRelation);
+
+		add_with_check_options(rel, rt_index,
+							   commandType == CMD_INSERT ?
+							   WCO_RLS_INSERT_CHECK : WCO_RLS_UPDATE_CHECK,
+							   permissive_policies,
+							   restrictive_policies,
+							   withCheckOptions,
+							   hasSubLinks);
 
 		/*
-		 * Handle any restrictive policies first.
-		 *
-		 * They can simply be added.
+		 * For INSERT ... ON CONFLICT DO UPDATE we need additional policy
+		 * checks for the UPDATE which may be applied to the same RTE.
 		 */
-		if (hook_with_check_expr_restrictive)
+		if (commandType == CMD_INSERT &&
+			root->onConflict && root->onConflict->action == ONCONFLICT_UPDATE)
 		{
-			WithCheckOption *wco;
+			List	   *conflict_permissive_policies;
+			List	   *conflict_restrictive_policies;
+
+			/* Get the policies that apply to the auxiliary UPDATE */
+			get_policies_for_relation(rel, CMD_UPDATE, user_id,
+									  &conflict_permissive_policies,
+									  &conflict_restrictive_policies);
 
-			wco = (WithCheckOption *) makeNode(WithCheckOption);
-			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
-				WCO_RLS_UPDATE_CHECK;
-			wco->relname = pstrdup(RelationGetRelationName(rel));
-			wco->qual = (Node *) hook_with_check_expr_restrictive;
-			wco->cascaded = false;
-			*withCheckOptions = lappend(*withCheckOptions, wco);
+			/*
+			 * Enforce the USING clauses of the UPDATE policies using WCOs
+			 * rather than security quals.  This ensures that an error is
+			 * raised if the conflicting row cannot be updated due to RLS,
+			 * rather than the change being silently dropped.
+			 */
+			add_with_check_options(rel, rt_index,
+								   WCO_RLS_CONFLICT_CHECK,
+								   conflict_permissive_policies,
+								   conflict_restrictive_policies,
+								   withCheckOptions,
+								   hasSubLinks);
+
+			/* Enforce the WITH CHECK clauses of the UPDATE policies */
+			add_with_check_options(rel, rt_index,
+								   WCO_RLS_UPDATE_CHECK,
+								   conflict_permissive_policies,
+								   conflict_restrictive_policies,
+								   withCheckOptions,
+								   hasSubLinks);
 		}
+	}
 
-		/*
-		 * Handle built-in policies, if there are no permissive policies from
-		 * the hook.
-		 */
-		if (rowsec_with_check_expr && !hook_with_check_expr_permissive)
-		{
-			WithCheckOption *wco;
+	heap_close(rel, NoLock);
 
-			wco = (WithCheckOption *) makeNode(WithCheckOption);
-			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
-				WCO_RLS_UPDATE_CHECK;
-			wco->relname = pstrdup(RelationGetRelationName(rel));
-			wco->qual = (Node *) rowsec_with_check_expr;
-			wco->cascaded = false;
-			*withCheckOptions = lappend(*withCheckOptions, wco);
-		}
-		/* Handle the hook policies, if there are no built-in ones. */
-		else if (!rowsec_with_check_expr && hook_with_check_expr_permissive)
-		{
-			WithCheckOption *wco;
+	/*
+	 * Mark this query as having row security, so plancache can invalidate it
+	 * when necessary (eg: role changes)
+	 */
+	*hasRowSecurity = true;
 
-			wco = (WithCheckOption *) makeNode(WithCheckOption);
-			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
-				WCO_RLS_UPDATE_CHECK;
-			wco->relname = pstrdup(RelationGetRelationName(rel));
-			wco->qual = (Node *) hook_with_check_expr_permissive;
-			wco->cascaded = false;
-			*withCheckOptions = lappend(*withCheckOptions, wco);
-		}
-		/* Handle the case where there are both. */
-		else if (rowsec_with_check_expr && hook_with_check_expr_permissive)
-		{
-			WithCheckOption *wco;
-			List	   *combined_quals = NIL;
-			Expr	   *combined_qual_eval;
+	return;
+}
 
-			combined_quals = lcons(copyObject(rowsec_with_check_expr),
-								   combined_quals);
+/*
+ * get_policies_for_relation
+ *
+ * Returns lists of permissive and restrictive policies to be applied to the
+ * specified relation, based on the command type and role.
+ *
+ * This includes any policies added by extensions.
+ */
+static void
+get_policies_for_relation(Relation relation, CmdType cmd, Oid user_id,
+						  List **permissive_policies,
+						  List **restrictive_policies)
+{
+	ListCell   *item;
 
-			combined_quals = lcons(copyObject(hook_with_check_expr_permissive),
-								   combined_quals);
+	*permissive_policies = NIL;
+	*restrictive_policies = NIL;
 
-			combined_qual_eval = makeBoolExpr(OR_EXPR, combined_quals, -1);
+	/*
+	 * First find all internal policies for the relation.  CREATE POLICY does
+	 * not currently support defining restrictive policies, so for now all
+	 * internal policies are permissive.
+	 */
+	foreach(item, relation->rd_rsdesc->policies)
+	{
+		bool				cmd_matches = false;
+		RowSecurityPolicy  *policy = (RowSecurityPolicy *) lfirst(item);
 
-			wco = (WithCheckOption *) makeNode(WithCheckOption);
-			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
-				WCO_RLS_UPDATE_CHECK;
-			wco->relname = pstrdup(RelationGetRelationName(rel));
-			wco->qual = (Node *) combined_qual_eval;
-			wco->cascaded = false;
-			*withCheckOptions = lappend(*withCheckOptions, wco);
+		/* Always add ALL policies, if they exist. */
+		if (policy->polcmd == '*')
+			cmd_matches = true;
+		else
+		{
+			/* Check whether the policy applies to the specified command type */
+			switch (cmd)
+			{
+				case CMD_SELECT:
+					if (policy->polcmd == ACL_SELECT_CHR)
+						cmd_matches = true;
+					break;
+				case CMD_INSERT:
+					if (policy->polcmd == ACL_INSERT_CHR)
+						cmd_matches = true;
+					break;
+				case CMD_UPDATE:
+					if (policy->polcmd == ACL_UPDATE_CHR)
+						cmd_matches = true;
+					break;
+				case CMD_DELETE:
+					if (policy->polcmd == ACL_DELETE_CHR)
+						cmd_matches = true;
+					break;
+				default:
+					elog(ERROR, "unrecognized policy command type %d",
+						 (int) cmd);
+					break;
+			}
 		}
 
 		/*
-		 * ON CONFLICT DO UPDATE has an RTE that is subject to both INSERT and
-		 * UPDATE RLS enforcement.  Those are enforced (as a special, distinct
-		 * kind of WCO) on the target tuple.
-		 *
-		 * Make a second, recursive pass over the RTE for this, gathering
-		 * UPDATE-applicable RLS checks/WCOs, and gathering and converting
-		 * UPDATE-applicable security quals into WCO_RLS_CONFLICT_CHECK RLS
-		 * checks/WCOs.  Finally, these distinct kinds of RLS checks/WCOs are
-		 * concatenated with our own INSERT-applicable list.
+		 * Add this policy to the list of permissive policies if it
+		 * applies to the specified role.
 		 */
-		if (root->onConflict && root->onConflict->action == ONCONFLICT_UPDATE &&
-			commandType == CMD_INSERT)
-		{
-			List	   *conflictSecurityQuals = NIL;
-			List	   *conflictWCOs = NIL;
-			ListCell   *item;
-			bool		conflictHasRowSecurity = false;
-			bool		conflictHasSublinks = false;
-
-			/* Assume that RTE is target resultRelation */
-			get_row_security_policies(root, CMD_UPDATE, rte, rt_index,
-									  &conflictSecurityQuals, &conflictWCOs,
-									  &conflictHasRowSecurity,
-									  &conflictHasSublinks);
-
-			if (conflictHasRowSecurity)
-				*hasRowSecurity = true;
-			if (conflictHasSublinks)
-				*hasSubLinks = true;
+		if (cmd_matches && check_role_for_policy(policy->roles, user_id))
+			*permissive_policies = lappend(*permissive_policies, policy);
+	}
 
-			/*
-			 * Append WITH CHECK OPTIONs/RLS checks, which should not conflict
-			 * between this INSERT and the auxiliary UPDATE
-			 */
-			*withCheckOptions = list_concat(*withCheckOptions,
-											conflictWCOs);
+	/*
+	 * Then add any permissive or restrictive policies defined by extensions.
+	 * These are simply appended to the lists of internal policies, if they
+	 * apply to the specified role.
+	 */
+	if (row_security_policy_hook_restrictive)
+	{
+		List	   *hook_policies =
+			(*row_security_policy_hook_restrictive) (cmd, relation);
 
-			foreach(item, conflictSecurityQuals)
-			{
-				Expr	   *conflict_rowsec_expr = (Expr *) lfirst(item);
-				WithCheckOption *wco;
+		/*
+		 * We sort restrictive policies by name so that any WCOs they generate
+		 * are checked in a well-defined order.
+		 */
+		hook_policies = sort_policies_by_name(hook_policies);
 
-				wco = (WithCheckOption *) makeNode(WithCheckOption);
+		foreach(item, hook_policies)
+		{
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
 
-				wco->kind = WCO_RLS_CONFLICT_CHECK;
-				wco->relname = pstrdup(RelationGetRelationName(rel));
-				wco->qual = (Node *) copyObject(conflict_rowsec_expr);
-				wco->cascaded = false;
-				*withCheckOptions = lappend(*withCheckOptions, wco);
-			}
+			if (check_role_for_policy(policy->roles, user_id))
+				*restrictive_policies = lappend(*restrictive_policies, policy);
 		}
 	}
 
-	/* For SELECT, UPDATE, and DELETE, set the security quals */
-	if (commandType == CMD_SELECT
-		|| commandType == CMD_UPDATE
-		|| commandType == CMD_DELETE)
+	if (row_security_policy_hook_permissive)
 	{
-		/* restrictive policies can simply be added to the list first */
-		if (hook_expr_restrictive)
-			*securityQuals = lappend(*securityQuals, hook_expr_restrictive);
-
-		/* If we only have internal permissive, then just add those */
-		if (rowsec_expr && !hook_expr_permissive)
-			*securityQuals = lappend(*securityQuals, rowsec_expr);
-		/* .. and if we have only permissive policies from the hook */
-		else if (!rowsec_expr && hook_expr_permissive)
-			*securityQuals = lappend(*securityQuals, hook_expr_permissive);
-		/* if we have both, we have to combine them with an OR */
-		else if (rowsec_expr && hook_expr_permissive)
+		List	   *hook_policies =
+			(*row_security_policy_hook_permissive) (cmd, relation);
+
+		foreach(item, hook_policies)
 		{
-			List	   *combined_quals = NIL;
-			Expr	   *combined_qual_eval;
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
 
-			combined_quals = lcons(copyObject(rowsec_expr), combined_quals);
-			combined_quals = lcons(copyObject(hook_expr_permissive),
-								   combined_quals);
+			if (check_role_for_policy(policy->roles, user_id))
+				*permissive_policies = lappend(*permissive_policies, policy);
+		}
+	}
+}
 
-			combined_qual_eval = makeBoolExpr(OR_EXPR, combined_quals, -1);
+/*
+ * sort_policies_by_name
+ *
+ * This is only used for restrictive policies, ensuring that any
+ * WithCheckOptions they generate are applied in a well-defined order.
+ * This is not necessary for permissive policies, since they are all "OR"d
+ * together into a single WithCheckOption check.
+ */
+static List *
+sort_policies_by_name(List *policies)
+{
+	int			npol = list_length(policies);
+	RowSecurityPolicy *pols;
+	ListCell   *item;
+	int			ii = 0;
 
-			*securityQuals = lappend(*securityQuals, combined_qual_eval);
-		}
+	if (npol <= 1)
+		return policies;
+
+	pols = (RowSecurityPolicy *) palloc(sizeof(RowSecurityPolicy) * npol);
+
+	foreach(item, policies)
+	{
+		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+		pols[ii++] = *policy;
 	}
 
-	heap_close(rel, NoLock);
+	qsort(pols, npol, sizeof(RowSecurityPolicy), row_security_policy_cmp);
 
-	/*
-	 * Mark this query as having row security, so plancache can invalidate it
-	 * when necessary (eg: role changes)
-	 */
-	*hasRowSecurity = true;
+	policies = NIL;
+	for (ii = 0; ii < npol; ii++)
+		policies = lappend(policies, &pols[ii]);
 
-	return;
+	return policies;
+}
+
+/*
+ * qsort comparator to sort RowSecurityPolicy entries by name
+ */
+static int
+row_security_policy_cmp(const void *a, const void *b)
+{
+	const RowSecurityPolicy *pa = (const RowSecurityPolicy *) a;
+	const RowSecurityPolicy *pb = (const RowSecurityPolicy *) b;
+
+	/* Guard against NULL policy names from extensions */
+	if (pa->policy_name == NULL)
+		return pb->policy_name == NULL ? 0 : 1;
+	if (pb->policy_name == NULL)
+		return -1;
+
+	return strcmp(pa->policy_name, pb->policy_name);
 }
 
 /*
- * pull_row_security_policies
+ * add_security_quals
  *
- * Returns the list of policies to be added for this relation, based on the
- * type of command and the roles to which it applies, from the relation cache.
+ * Add security quals to enforce the specified RLS policies, restricting
+ * access to existing data in a table.  If there are no policies controlling
+ * access to the table, then all access is prohibited --- i.e., an implicit
+ * default-deny policy is used.
  *
+ * New security quals are added to securityQuals, and hasSubLinks is set to
+ * true if any of the quals added contain sublink subqueries.
  */
-static List *
-pull_row_security_policies(CmdType cmd, Relation relation, Oid user_id)
+static void
+add_security_quals(int rt_index,
+				   List *permissive_policies,
+				   List *restrictive_policies,
+				   List **securityQuals,
+				   bool *hasSubLinks)
 {
-	List	   *policies = NIL;
 	ListCell   *item;
+	List	   *permissive_quals = NIL;
+	Expr	   *rowsec_expr;
 
 	/*
-	 * Row security is enabled for the relation and the row security GUC is
-	 * either 'on' or 'force' here, so find the policies to apply to the
-	 * table. There must always be at least one policy defined (may be the
-	 * simple 'default-deny' policy, if none are explicitly defined on the
-	 * table).
+	 * First collect up the permissive quals.  If we do not find any permissive
+	 * policies then no rows are visible (this is handled below).
 	 */
-	foreach(item, relation->rd_rsdesc->policies)
+	foreach(item, permissive_policies)
 	{
 		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
 
-		/* Always add ALL policies, if they exist. */
-		if (policy->polcmd == '*' &&
-			check_role_for_policy(policy->roles, user_id))
-			policies = lcons(policy, policies);
-
-		/* Add relevant command-specific policies to the list. */
-		switch (cmd)
+		if (policy->qual != NULL)
 		{
-			case CMD_SELECT:
-				if (policy->polcmd == ACL_SELECT_CHR
-					&& check_role_for_policy(policy->roles, user_id))
-					policies = lcons(policy, policies);
-				break;
-			case CMD_INSERT:
-				/* If INSERT then only need to add the WITH CHECK qual */
-				if (policy->polcmd == ACL_INSERT_CHR
-					&& check_role_for_policy(policy->roles, user_id))
-					policies = lcons(policy, policies);
-				break;
-			case CMD_UPDATE:
-				if (policy->polcmd == ACL_UPDATE_CHR
-					&& check_role_for_policy(policy->roles, user_id))
-					policies = lcons(policy, policies);
-				break;
-			case CMD_DELETE:
-				if (policy->polcmd == ACL_DELETE_CHR
-					&& check_role_for_policy(policy->roles, user_id))
-					policies = lcons(policy, policies);
-				break;
-			default:
-				elog(ERROR, "unrecognized policy command type %d", (int) cmd);
-				break;
+			permissive_quals = lappend(permissive_quals,
+									   copyObject(policy->qual));
+			*hasSubLinks |= policy->hassublinks;
 		}
 	}
 
 	/*
-	 * There should always be a policy applied.  If there are none found then
-	 * create a simply defauly-deny policy (might be that policies exist but
-	 * that none of them apply to the role which is querying the table).
+	 * We must have permissive quals, always, or no rows are visible.
+	 *
+	 * If we do not, then we simply return a single 'false' qual which results
+	 * in no rows being visible.
 	 */
-	if (policies == NIL)
+	if (permissive_quals != NIL)
 	{
-		RowSecurityPolicy *policy = NULL;
-		Datum		role;
-
-		role = ObjectIdGetDatum(ACL_ID_PUBLIC);
-
-		policy = palloc0(sizeof(RowSecurityPolicy));
-		policy->policy_name = pstrdup("default-deny policy");
-		policy->policy_id = InvalidOid;
-		policy->polcmd = '*';
-		policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true,
-										'i');
-		policy->qual = (Expr *) makeConst(BOOLOID, -1, InvalidOid,
-										  sizeof(bool), BoolGetDatum(false),
-										  false, true);
-		policy->with_check_qual = copyObject(policy->qual);
-		policy->hassublinks = false;
-
-		policies = list_make1(policy);
-	}
+		/*
+		 * We now know that permissive policies exist, so we can now add
+		 * security quals based on the USING clauses from the restrictive
+		 * policies.  Since these need to be "AND"d together, we can
+		 * just add them one at a time.
+		 */
+		foreach(item, restrictive_policies)
+		{
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+			Expr	   *qual;
 
-	Assert(policies != NIL);
+			if (policy->qual != NULL)
+			{
+				qual = copyObject(policy->qual);
+				ChangeVarNodes((Node *) qual, 1, rt_index, 0);
 
-	return policies;
+				*securityQuals = lappend(*securityQuals, qual);
+				*hasSubLinks |= policy->hassublinks;
+			}
+		}
+
+		/*
+		 * Then add a single security qual "OR"ing together the USING clauses
+		 * from all the permissive policies.
+		 */
+		if (list_length(permissive_quals) == 1)
+			rowsec_expr = (Expr *) linitial(permissive_quals);
+		else
+			rowsec_expr = makeBoolExpr(OR_EXPR, permissive_quals, -1);
+
+		ChangeVarNodes((Node *) rowsec_expr, 1, rt_index, 0);
+		*securityQuals = lappend(*securityQuals, rowsec_expr);
+	}
+	else
+		/*
+		 * A permissive policy must exist for rows to be visible at all.
+		 * Therefore, if there were no permissive policies found, return a
+		 * single always-false clause.
+		 */
+		*securityQuals = lappend(*securityQuals,
+								 makeConst(BOOLOID, -1, InvalidOid,
+										   sizeof(bool), BoolGetDatum(false),
+										   false, true));
 }
 
 /*
- * process_policies
+ * add_with_check_options
  *
- * This will step through the policies which are passed in (which would come
- * from either the built-in ones created on a table, or from policies provided
- * by an extension through the hook provided), work out how to combine them,
- * rewrite them as necessary, and produce an Expr for the normal security
- * quals and an Expr for the with check quals.
+ * Add WithCheckOptions of the specified kind to check that new records
+ * added by an INSERT or UPDATE are consistent with the specified RLS
+ * policies.  Normally new data must satisfy the WITH CHECK clauses from the
+ * policies.  If a policy has no explicit WITH CHECK clause, its USING clause
+ * is used instead.  In the special case of an UPDATE arising from an
+ * INSERT ... ON CONFLICT DO UPDATE, existing records are first checked using
+ * a WCO_RLS_CONFLICT_CHECK WithCheckOption, which always uses the USING
+ * clauses from RLS policies.
  *
- * qual_eval, with_check_eval, and hassublinks are output variables
+ * New WCOs are added to withCheckOptions, and hasSubLinks is set to true if
+ * any of the check clauses added contain sublink subqueries.
  */
 static void
-process_policies(Query *root, List *policies, int rt_index, Expr **qual_eval,
-				 Expr **with_check_eval, bool *hassublinks,
-				 BoolExprType boolop)
+add_with_check_options(Relation rel,
+					   int rt_index,
+					   WCOKind kind,
+					   List *permissive_policies,
+					   List *restrictive_policies,
+					   List **withCheckOptions,
+					   bool *hasSubLinks)
 {
 	ListCell   *item;
-	List	   *quals = NIL;
-	List	   *with_check_quals = NIL;
+	List	   *permissive_quals = NIL;
+
+#define QUAL_FOR_WCO(policy) \
+	( kind != WCO_RLS_CONFLICT_CHECK &&	\
+	  (policy)->with_check_qual != NULL ? \
+	  (policy)->with_check_qual : (policy)->qual )
 
 	/*
-	 * Extract the USING and WITH CHECK quals from each of the policies and
-	 * add them to our lists.  We only want WITH CHECK quals if this RTE is
-	 * the query's result relation.
+	 * First collect up the permissive policy clauses, similar to
+	 * add_security_quals.
 	 */
-	foreach(item, policies)
+	foreach(item, permissive_policies)
 	{
 		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+		Expr	   *qual = QUAL_FOR_WCO(policy);
 
-		if (policy->qual != NULL)
-			quals = lcons(copyObject(policy->qual), quals);
-
-		if (policy->with_check_qual != NULL &&
-			rt_index == root->resultRelation)
-			with_check_quals = lcons(copyObject(policy->with_check_qual),
-									 with_check_quals);
+		if (qual != NULL)
+		{
+			permissive_quals = lappend(permissive_quals, copyObject(qual));
+			*hasSubLinks |= policy->hassublinks;
+		}
+	}
 
+	/*
+	 * There must be at least one permissive qual found or no rows are
+	 * allowed to be added.  This is the same as in add_security_quals.
+	 *
+	 * If there are no permissive_quals then we fall through and return a single
+	 * 'false' WCO, preventing all new rows.
+	 */
+	if (permissive_quals != NIL)
+	{
 		/*
-		 * For each policy, if there is only a USING clause then copy/use it
-		 * for the WITH CHECK policy also, if this RTE is the query's result
-		 * relation.
+		 * Add a single WithCheckOption for all the permissive policy clauses
+		 * "OR"d together.  This check has no policy name, since if the check
+		 * fails it means that no policy granted permission to perform the
+		 * update, rather than any particular policy being violated.
 		 */
-		if (policy->qual != NULL && policy->with_check_qual == NULL &&
-			rt_index == root->resultRelation)
-			with_check_quals = lcons(copyObject(policy->qual),
-									 with_check_quals);
+		WithCheckOption *wco;
 
+		wco = (WithCheckOption *) makeNode(WithCheckOption);
+		wco->kind = kind;
+		wco->relname = pstrdup(RelationGetRelationName(rel));
+		wco->polname = NULL;
+		wco->cascaded = false;
 
-		if (policy->hassublinks)
-			*hassublinks = true;
-	}
+		if (list_length(permissive_quals) == 1)
+			wco->qual = (Node *) linitial(permissive_quals);
+		else
+			wco->qual = (Node *) makeBoolExpr(OR_EXPR, permissive_quals, -1);
 
-	/*
-	 * If we end up without any normal quals (perhaps the only policy matched
-	 * was for INSERT), then create a single all-false one.
-	 */
-	if (quals == NIL)
-		quals = lcons(makeConst(BOOLOID, -1, InvalidOid, sizeof(bool),
-								BoolGetDatum(false), false, true), quals);
+		ChangeVarNodes(wco->qual, 1, rt_index, 0);
 
-	/*
-	 * Row security quals always have the target table as varno 1, as no joins
-	 * are permitted in row security expressions. We must walk the expression,
-	 * updating any references to varno 1 to the varno the table has in the
-	 * outer query.
-	 *
-	 * We rewrite the expression in-place.
-	 *
-	 * We must have some quals at this point; the default-deny policy, if
-	 * nothing else.  Note that we might not have any WITH CHECK quals- that's
-	 * fine, as this might not be the resultRelation.
-	 */
-	Assert(quals != NIL);
+		*withCheckOptions = lappend(*withCheckOptions, wco);
 
-	ChangeVarNodes((Node *) quals, 1, rt_index, 0);
+		/*
+		 * Now add WithCheckOptions for each of the restrictive policy clauses
+		 * (which will be "AND"d together).  We use a separate WithCheckOption
+		 * for each restrictive policy to allow the policy name to be included
+		 * in error reports if the policy is violated.
+		 */
+		foreach(item, restrictive_policies)
+		{
+			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+			Expr	   *qual = QUAL_FOR_WCO(policy);
+			WithCheckOption *wco;
 
-	if (with_check_quals != NIL)
-		ChangeVarNodes((Node *) with_check_quals, 1, rt_index, 0);
+			if (qual != NULL)
+			{
+				qual = copyObject(qual);
+				ChangeVarNodes((Node *) qual, 1, rt_index, 0);
 
-	/*
-	 * If more than one security qual is returned, then they need to be
-	 * combined together.
-	 */
-	if (list_length(quals) > 1)
-		*qual_eval = makeBoolExpr(boolop, quals, -1);
-	else
-		*qual_eval = (Expr *) linitial(quals);
+				wco = (WithCheckOption *) makeNode(WithCheckOption);
+				wco->kind = kind;
+				wco->relname = pstrdup(RelationGetRelationName(rel));
+				wco->polname = pstrdup(policy->policy_name);
+				wco->qual = (Node *) qual;
+				wco->cascaded = false;
 
-	/*
-	 * Similarly, if more than one WITH CHECK qual is returned, then they need
-	 * to be combined together.
-	 *
-	 * with_check_quals is allowed to be NIL here since this might not be the
-	 * resultRelation (see above).
-	 */
-	if (list_length(with_check_quals) > 1)
-		*with_check_eval = makeBoolExpr(boolop, with_check_quals, -1);
-	else if (with_check_quals != NIL)
-		*with_check_eval = (Expr *) linitial(with_check_quals);
+				*withCheckOptions = lappend(*withCheckOptions, wco);
+				*hasSubLinks |= policy->hassublinks;
+			}
+		}
+	}
 	else
-		*with_check_eval = NULL;
-
-	return;
+	{
+		/*
+		 * If there were no policy clauses to check new data, add a single
+		 * always-false WCO (a default-deny policy).
+		 */
+		WithCheckOption *wco;
+
+		wco = (WithCheckOption *) makeNode(WithCheckOption);
+		wco->kind = kind;
+		wco->relname = pstrdup(RelationGetRelationName(rel));
+		wco->polname = NULL;
+		wco->qual = (Node *) makeConst(BOOLOID, -1, InvalidOid,
+									   sizeof(bool), BoolGetDatum(false),
+									   false, true);
+		wco->cascaded = false;
+
+		*withCheckOptions = lappend(*withCheckOptions, wco);
+	}
 }
 
 /*
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 420ef3d..9c3d096 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -859,8 +859,6 @@ equalPolicy(RowSecurityPolicy *policy1, RowSecurityPolicy *policy2)
 		if (policy2 == NULL)
 			return false;
 
-		if (policy1->policy_id != policy2->policy_id)
-			return false;
 		if (policy1->polcmd != policy2->polcmd)
 			return false;
 		if (policy1->hassublinks != policy2->hassublinks)
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index f0dcd2f..940cc32 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -928,6 +928,7 @@ typedef struct WithCheckOption
 	NodeTag		type;
 	WCOKind		kind;			/* kind of WCO */
 	char	   *relname;		/* name of relation that specified the WCO */
+	char	   *polname;		/* name of RLS policy being checked */
 	Node	   *qual;			/* constraint qual to check */
 	bool		cascaded;		/* true for a cascaded WCO on a view */
 } WithCheckOption;
diff --git a/src/include/rewrite/rowsecurity.h b/src/include/rewrite/rowsecurity.h
index 523c56e..4af244d 100644
--- a/src/include/rewrite/rowsecurity.h
+++ b/src/include/rewrite/rowsecurity.h
@@ -19,7 +19,6 @@
 
 typedef struct RowSecurityPolicy
 {
-	Oid			policy_id;		/* OID of the policy */
 	char	   *policy_name;	/* Name of the policy */
 	char		polcmd;			/* Type of command policy is for */
 	ArrayType  *roles;			/* Array of roles policy is for */
@@ -41,7 +40,7 @@ extern PGDLLIMPORT row_security_policy_hook_type row_security_policy_hook_permis
 
 extern PGDLLIMPORT row_security_policy_hook_type row_security_policy_hook_restrictive;
 
-extern void get_row_security_policies(Query *root, CmdType commandType,
+extern void get_row_security_policies(Query *root,
 						  RangeTblEntry *rte, int rt_index,
 						  List **securityQuals, List **withCheckOptions,
 						  bool *hasRowSecurity, bool *hasSubLinks);
diff --git a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
index 4587eb0..8885464 100644
--- a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
+++ b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
@@ -83,7 +83,7 @@ SELECT * FROM rls_test_restrictive;
 INSERT INTO rls_test_restrictive VALUES ('r1','s1',10);
 -- failure
 INSERT INTO rls_test_restrictive VALUES ('r4','s4',10);
-ERROR:  new row violates row level security policy for "rls_test_restrictive"
+ERROR:  new row violates row level security policy "extension policy" for "rls_test_restrictive"
 SET ROLE s1;
 -- With only the hook's policies, both
 -- permissive hook's policy is current_user = username
@@ -124,7 +124,7 @@ EXPLAIN (costs off) SELECT * FROM rls_test_permissive;
                           QUERY PLAN                           
 ---------------------------------------------------------------
  Seq Scan on rls_test_permissive
-   Filter: (("current_user"() = username) OR ((data % 2) = 0))
+   Filter: (((data % 2) = 0) OR ("current_user"() = username))
 (2 rows)
 
 SELECT * FROM rls_test_permissive;
@@ -163,7 +163,7 @@ SELECT * FROM rls_test_restrictive;
 INSERT INTO rls_test_restrictive VALUES ('r1','s1',8);
 -- failure
 INSERT INTO rls_test_restrictive VALUES ('r3','s3',10);
-ERROR:  new row violates row level security policy for "rls_test_restrictive"
+ERROR:  new row violates row level security policy "extension policy" for "rls_test_restrictive"
 -- failure
 INSERT INTO rls_test_restrictive VALUES ('r1','s1',7);
 ERROR:  new row violates row level security policy for "rls_test_restrictive"
@@ -176,7 +176,7 @@ EXPLAIN (costs off) SELECT * FROM rls_test_both;
                                         QUERY PLAN                                         
 -------------------------------------------------------------------------------------------
  Subquery Scan on rls_test_both
-   Filter: (("current_user"() = rls_test_both.username) OR ((rls_test_both.data % 2) = 0))
+   Filter: (((rls_test_both.data % 2) = 0) OR ("current_user"() = rls_test_both.username))
    ->  Seq Scan on rls_test_both rls_test_both_1
          Filter: ("current_user"() = supervisor)
 (4 rows)
@@ -190,7 +190,7 @@ SELECT * FROM rls_test_both;
 INSERT INTO rls_test_both VALUES ('r1','s1',8);
 -- failure
 INSERT INTO rls_test_both VALUES ('r3','s3',10);
-ERROR:  new row violates row level security policy for "rls_test_both"
+ERROR:  new row violates row level security policy "extension policy" for "rls_test_both"
 -- failure
 INSERT INTO rls_test_both VALUES ('r1','s1',7);
 ERROR:  new row violates row level security policy for "rls_test_both"
diff --git a/src/test/modules/test_rls_hooks/test_rls_hooks.c b/src/test/modules/test_rls_hooks/test_rls_hooks.c
index b96dbff..cc865cd 100644
--- a/src/test/modules/test_rls_hooks/test_rls_hooks.c
+++ b/src/test/modules/test_rls_hooks/test_rls_hooks.c
@@ -87,7 +87,6 @@ test_rls_hooks_permissive(CmdType cmdtype, Relation relation)
 	role = ObjectIdGetDatum(ACL_ID_PUBLIC);
 
 	policy->policy_name = pstrdup("extension policy");
-	policy->policy_id = InvalidOid;
 	policy->polcmd = '*';
 	policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true, 'i');
 
@@ -151,7 +150,6 @@ test_rls_hooks_restrictive(CmdType cmdtype, Relation relation)
 	role = ObjectIdGetDatum(ACL_ID_PUBLIC);
 
 	policy->policy_name = pstrdup("extension policy");
-	policy->policy_id = InvalidOid;
 	policy->polcmd = '*';
 	policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true, 'i');
 
-- 
1.9.1


From 4f8c4b615743a725c534c96b713c65bff4444ea4 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Mon, 14 Sep 2015 08:57:47 -0400
Subject: [PATCH 2/2] Enforce ALL/SELECT policies in RETURNING for RLS

For the UPDATE/DELETE RETURNING case, filter the records which are not
visible to the user through ALL or SELECT policies from those considered
for the UPDATE or DELETE.  This is similar to how the GRANT system
works, which prevents RETURNING unless the caller has SELECT rights on
the relation.

Per discussion with Robert, Dean, Tom, and Kevin.

Back-patch to 9.5 where RLS was introduced.
---
 src/backend/rewrite/rowsecurity.c         | 47 +++++++++++++++++++++++++++++
 src/test/regress/expected/rowsecurity.out | 50 +++++++++++++++++--------------
 2 files changed, 74 insertions(+), 23 deletions(-)

diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index b96c29d..c20a178 100644
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -187,6 +187,33 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
 						   hasSubLinks);
 
 	/*
+	 * For the target relation, when there is a returning list, we need to
+	 * collect up CMD_SELECT policies and add them via add_security_quals.
+	 * This is because, for the RETURNING case, we have to filter any records
+	 * which are not visible through an ALL or SELECT USING policy.
+	 *
+	 * We don't need to worry about the non-target relation case because we are
+	 * checking the ALL and SELECT policies for those relations anyway (see
+	 * above).
+	 */
+	if (root->returningList != NIL &&
+		(commandType == CMD_UPDATE || commandType == CMD_DELETE))
+	{
+		List	   *returning_permissive_policies;
+		List	   *returning_restrictive_policies;
+
+		get_policies_for_relation(rel, CMD_SELECT, user_id,
+								  &returning_permissive_policies,
+								  &returning_restrictive_policies);
+
+		add_security_quals(rt_index,
+						   returning_permissive_policies,
+						   returning_restrictive_policies,
+						   securityQuals,
+						   hasSubLinks);
+	}
+
+	/*
 	 * For INSERT and UPDATE, add withCheckOptions to verify that any new
 	 * records added are consistent with the security policies.  This will use
 	 * each policy's WITH CHECK clause, or its USING clause if no explicit
@@ -233,6 +260,26 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
 								   withCheckOptions,
 								   hasSubLinks);
 
+			/*
+			 * Get and add ALL/SELECT policies, if there is a RETURNING clause,
+			 * also as WCO policies, again, to avoid silently dropping data.
+			 */
+			if (root->returningList != NIL)
+			{
+				List	   *conflict_returning_permissive_policies = NIL;
+				List	   *conflict_returning_restrictive_policies = NIL;
+
+				get_policies_for_relation(rel, CMD_SELECT, user_id,
+									  &conflict_returning_permissive_policies,
+									  &conflict_returning_restrictive_policies);
+				add_with_check_options(rel, rt_index,
+									   WCO_RLS_CONFLICT_CHECK,
+									   conflict_returning_permissive_policies,
+									   conflict_returning_restrictive_policies,
+									   withCheckOptions,
+									   hasSubLinks);
+			}
+
 			/* Enforce the WITH CHECK clauses of the UPDATE policies */
 			add_with_check_options(rel, rt_index,
 								   WCO_RLS_UPDATE_CHECK,
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 6fc80af..7628e52 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -1246,21 +1246,23 @@ NOTICE:  f_leak => cde
 EXPLAIN (COSTS OFF) UPDATE t2 t2_1 SET b = t2_2.b FROM t2 t2_2
 WHERE t2_1.a = 3 AND t2_2.a = t2_1.a AND t2_2.b = t2_1.b
 AND f_leak(t2_1.b) AND f_leak(t2_2.b) RETURNING *, t2_1, t2_2;
-                          QUERY PLAN                           
----------------------------------------------------------------
+                             QUERY PLAN                              
+---------------------------------------------------------------------
  Update on t2 t2_1_1
    ->  Nested Loop
          Join Filter: (t2_1.b = t2_2.b)
          ->  Subquery Scan on t2_1
                Filter: f_leak(t2_1.b)
-               ->  LockRows
-                     ->  Seq Scan on t2 t2_1_2
-                           Filter: ((a = 3) AND ((a % 2) = 1))
+               ->  Subquery Scan on t2_1_2
+                     Filter: ((t2_1_2.a % 2) = 1)
+                     ->  LockRows
+                           ->  Seq Scan on t2 t2_1_3
+                                 Filter: ((a = 3) AND ((a % 2) = 1))
          ->  Subquery Scan on t2_2
                Filter: f_leak(t2_2.b)
                ->  Seq Scan on t2 t2_2_1
                      Filter: ((a = 3) AND ((a % 2) = 1))
-(12 rows)
+(14 rows)
 
 UPDATE t2 t2_1 SET b = t2_2.b FROM t2 t2_2
 WHERE t2_1.a = 3 AND t2_2.a = t2_1.a AND t2_2.b = t2_1.b
@@ -1275,8 +1277,8 @@ NOTICE:  f_leak => cde
 EXPLAIN (COSTS OFF) UPDATE t1 t1_1 SET b = t1_2.b FROM t1 t1_2
 WHERE t1_1.a = 4 AND t1_2.a = t1_1.a AND t1_2.b = t1_1.b
 AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
-                          QUERY PLAN                           
----------------------------------------------------------------
+                             QUERY PLAN                              
+---------------------------------------------------------------------
  Update on t1 t1_1_3
    Update on t1 t1_1_3
    Update on t2 t1_1
@@ -1285,9 +1287,11 @@ AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
          Join Filter: (t1_1.b = t1_2.b)
          ->  Subquery Scan on t1_1
                Filter: f_leak(t1_1.b)
-               ->  LockRows
-                     ->  Seq Scan on t1 t1_1_4
-                           Filter: ((a = 4) AND ((a % 2) = 0))
+               ->  Subquery Scan on t1_1_4
+                     Filter: ((t1_1_4.a % 2) = 0)
+                     ->  LockRows
+                           ->  Seq Scan on t1 t1_1_5
+                                 Filter: ((a = 4) AND ((a % 2) = 0))
          ->  Subquery Scan on t1_2
                Filter: f_leak(t1_2.b)
                ->  Append
@@ -1301,9 +1305,11 @@ AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
          Join Filter: (t1_1_1.b = t1_2_1.b)
          ->  Subquery Scan on t1_1_1
                Filter: f_leak(t1_1_1.b)
-               ->  LockRows
-                     ->  Seq Scan on t2 t1_1_5
-                           Filter: ((a = 4) AND ((a % 2) = 0))
+               ->  Subquery Scan on t1_1_6
+                     Filter: ((t1_1_6.a % 2) = 0)
+                     ->  LockRows
+                           ->  Seq Scan on t2 t1_1_7
+                                 Filter: ((a = 4) AND ((a % 2) = 0))
          ->  Subquery Scan on t1_2_1
                Filter: f_leak(t1_2_1.b)
                ->  Append
@@ -1317,9 +1323,11 @@ AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
          Join Filter: (t1_1_2.b = t1_2_2.b)
          ->  Subquery Scan on t1_1_2
                Filter: f_leak(t1_1_2.b)
-               ->  LockRows
-                     ->  Seq Scan on t3 t1_1_6
-                           Filter: ((a = 4) AND ((a % 2) = 0))
+               ->  Subquery Scan on t1_1_8
+                     Filter: ((t1_1_8.a % 2) = 0)
+                     ->  LockRows
+                           ->  Seq Scan on t3 t1_1_9
+                                 Filter: ((a = 4) AND ((a % 2) = 0))
          ->  Subquery Scan on t1_2_2
                Filter: f_leak(t1_2_2.b)
                ->  Append
@@ -1329,7 +1337,7 @@ AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
                            Filter: ((a = 4) AND ((a % 2) = 0))
                      ->  Seq Scan on t3 t1_2_11
                            Filter: ((a = 4) AND ((a % 2) = 0))
-(52 rows)
+(58 rows)
 
 UPDATE t1 t1_1 SET b = t1_2.b FROM t1 t1_2
 WHERE t1_1.a = 4 AND t1_2.a = t1_1.a AND t1_2.b = t1_1.b
@@ -1960,8 +1968,6 @@ NOTICE:  f_leak => fgh_updt
 (6 rows)
 
 DELETE FROM x1 WHERE f_leak(b) RETURNING *;
-NOTICE:  f_leak => abc_updt
-NOTICE:  f_leak => efg_updt
 NOTICE:  f_leak => cde_updt
 NOTICE:  f_leak => fgh_updt
 NOTICE:  f_leak => bcd_updt_updt
@@ -1970,15 +1976,13 @@ NOTICE:  f_leak => fgh_updt_updt
 NOTICE:  f_leak => fgh_updt_updt
  a |       b       |         c         
 ---+---------------+-------------------
- 1 | abc_updt      | rls_regress_user1
- 5 | efg_updt      | rls_regress_user1
  3 | cde_updt      | rls_regress_user2
  7 | fgh_updt      | rls_regress_user2
  2 | bcd_updt_updt | rls_regress_user1
  4 | def_updt_updt | rls_regress_user2
  6 | fgh_updt_updt | rls_regress_user1
  8 | fgh_updt_updt | rls_regress_user2
-(8 rows)
+(6 rows)
 
 --
 -- Duplicate Policy Names
-- 
1.9.1

From b8ad8ca34ad473813b072cc871ad33c48ba68b42 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Mon, 28 Sep 2015 13:25:28 -0400
Subject: [PATCH] Include policies based on ACLs needed

When considering which policies should be included, rather than look at
individual bits of the query (eg: if a RETURNING clause exists, or if a
WHERE clause exists which is referencing the table, or if it's a
FOR SHARE/UPDATE query), consider any case where we've determined
the user needs SELECT rights on the relation to be a case where we apply
SELECT policies, and any case where we've deteremind that the user needs
UPDATE rights on the relation to be a case where we apply UPDATE
policies.

This simplifies the logic and addresses concerns that a user could use
UPDATE or DELETE with a WHERE clauses to determine if rows exist, or
they could lock rows which they are not actually allowed to modify
through UPDATE policies.

Back-patch to 9.5 where RLS was added.
---
 src/backend/rewrite/rowsecurity.c         |  76 +++++++++-----
 src/test/regress/expected/rowsecurity.out | 166 ++++++++++++++++++------------
 2 files changed, 150 insertions(+), 92 deletions(-)

diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index c20a178..1a51c20 100644
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -187,28 +187,55 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
 						   hasSubLinks);
 
 	/*
-	 * For the target relation, when there is a returning list, we need to
-	 * collect up CMD_SELECT policies and add them via add_security_quals.
-	 * This is because, for the RETURNING case, we have to filter any records
-	 * which are not visible through an ALL or SELECT USING policy.
+	 * For a SELECT, if UPDATE privileges are required (eg: the user has
+	 * specified FOR [KEY] UPDATE/SHARE), then also add the UPDATE USING quals.
 	 *
-	 * We don't need to worry about the non-target relation case because we are
-	 * checking the ALL and SELECT policies for those relations anyway (see
-	 * above).
+	 * This way, we filter out any records from the SELECT FOR SHARE/UPDATE
+	 * which the user does not have access to via the UPDATE USING policies,
+	 * similar to how we require normal UPDATE rights for these queries.
 	 */
-	if (root->returningList != NIL &&
-		(commandType == CMD_UPDATE || commandType == CMD_DELETE))
+	if (commandType == CMD_SELECT && rte->requiredPerms & ACL_UPDATE)
 	{
-		List	   *returning_permissive_policies;
-		List	   *returning_restrictive_policies;
+		List	   *update_permissive_policies;
+		List	   *update_restrictive_policies;
+
+		get_policies_for_relation(rel, CMD_UPDATE, user_id,
+								  &update_permissive_policies,
+								  &update_restrictive_policies);
+
+		add_security_quals(rt_index,
+						   update_permissive_policies,
+						   update_restrictive_policies,
+						   securityQuals,
+						   hasSubLinks);
+	}
+
+	/*
+	 * Similar to above, during an UPDATE or DELETE, if SELECT rights are also
+	 * required (eg: when a RETURNING clause exists, or the user has provided
+	 * a WHERE clause which involves columns from the relation), we collect up
+	 * CMD_SELECT policies and add them via add_security_quals.
+	 *
+	 * This way, we filter out any records which are not visible through an ALL
+	 * or SELECT USING policy.
+	 *
+	 * We don't need to worry about the non-target relation case here because
+	 * we are checking the ALL and SELECT policies for those relations anyway
+	 * (see above).
+	 */
+	if ((commandType == CMD_UPDATE || commandType == CMD_DELETE) &&
+		rte->requiredPerms & ACL_SELECT)
+	{
+		List	   *select_permissive_policies;
+		List	   *select_restrictive_policies;
 
 		get_policies_for_relation(rel, CMD_SELECT, user_id,
-								  &returning_permissive_policies,
-								  &returning_restrictive_policies);
+								  &select_permissive_policies,
+								  &select_restrictive_policies);
 
 		add_security_quals(rt_index,
-						   returning_permissive_policies,
-						   returning_restrictive_policies,
+						   select_permissive_policies,
+						   select_restrictive_policies,
 						   securityQuals,
 						   hasSubLinks);
 	}
@@ -261,21 +288,22 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
 								   hasSubLinks);
 
 			/*
-			 * Get and add ALL/SELECT policies, if there is a RETURNING clause,
-			 * also as WCO policies, again, to avoid silently dropping data.
+			 * Get and add ALL/SELECT policies, if SELECT rights are required
+			 * for this relation, also as WCO policies, again, to avoid
+			 * silently dropping data.  See above.
 			 */
-			if (root->returningList != NIL)
+			if (rte->requiredPerms & ACL_SELECT)
 			{
-				List	   *conflict_returning_permissive_policies = NIL;
-				List	   *conflict_returning_restrictive_policies = NIL;
+				List	   *conflict_select_permissive_policies = NIL;
+				List	   *conflict_select_restrictive_policies = NIL;
 
 				get_policies_for_relation(rel, CMD_SELECT, user_id,
-									  &conflict_returning_permissive_policies,
-									  &conflict_returning_restrictive_policies);
+									  &conflict_select_permissive_policies,
+									  &conflict_select_restrictive_policies);
 				add_with_check_options(rel, rt_index,
 									   WCO_RLS_CONFLICT_CHECK,
-									   conflict_returning_permissive_policies,
-									   conflict_returning_restrictive_policies,
+									   conflict_select_permissive_policies,
+									   conflict_select_restrictive_policies,
 									   withCheckOptions,
 									   hasSubLinks);
 			}
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 9d3540f..b4e64ab 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -1043,28 +1043,34 @@ EXPLAIN (COSTS OFF) EXECUTE p2(2);
 --
 SET SESSION AUTHORIZATION rls_regress_user1;
 EXPLAIN (COSTS OFF) UPDATE t1 SET b = b || b WHERE f_leak(b);
-                QUERY PLAN                 
--------------------------------------------
+                   QUERY PLAN                    
+-------------------------------------------------
  Update on t1 t1_3
    Update on t1 t1_3
    Update on t2 t1
    Update on t3 t1
    ->  Subquery Scan on t1
          Filter: f_leak(t1.b)
-         ->  LockRows
-               ->  Seq Scan on t1 t1_4
-                     Filter: ((a % 2) = 0)
+         ->  Subquery Scan on t1_4
+               Filter: ((t1_4.a % 2) = 0)
+               ->  LockRows
+                     ->  Seq Scan on t1 t1_5
+                           Filter: ((a % 2) = 0)
    ->  Subquery Scan on t1_1
          Filter: f_leak(t1_1.b)
-         ->  LockRows
-               ->  Seq Scan on t2
-                     Filter: ((a % 2) = 0)
+         ->  Subquery Scan on t1_6
+               Filter: ((t1_6.a % 2) = 0)
+               ->  LockRows
+                     ->  Seq Scan on t2
+                           Filter: ((a % 2) = 0)
    ->  Subquery Scan on t1_2
          Filter: f_leak(t1_2.b)
-         ->  LockRows
-               ->  Seq Scan on t3
-                     Filter: ((a % 2) = 0)
-(19 rows)
+         ->  Subquery Scan on t1_7
+               Filter: ((t1_7.a % 2) = 0)
+               ->  LockRows
+                     ->  Seq Scan on t3
+                           Filter: ((a % 2) = 0)
+(25 rows)
 
 UPDATE t1 SET b = b || b WHERE f_leak(b);
 NOTICE:  f_leak => bbb
@@ -1073,15 +1079,17 @@ NOTICE:  f_leak => bcd
 NOTICE:  f_leak => def
 NOTICE:  f_leak => yyy
 EXPLAIN (COSTS OFF) UPDATE only t1 SET b = b || '_updt' WHERE f_leak(b);
-                QUERY PLAN                 
--------------------------------------------
+                   QUERY PLAN                    
+-------------------------------------------------
  Update on t1 t1_1
    ->  Subquery Scan on t1
          Filter: f_leak(t1.b)
-         ->  LockRows
-               ->  Seq Scan on t1 t1_2
-                     Filter: ((a % 2) = 0)
-(6 rows)
+         ->  Subquery Scan on t1_2
+               Filter: ((t1_2.a % 2) = 0)
+               ->  LockRows
+                     ->  Seq Scan on t1 t1_3
+                           Filter: ((a % 2) = 0)
+(8 rows)
 
 UPDATE only t1 SET b = b || '_updt' WHERE f_leak(b);
 NOTICE:  f_leak => bbbbbb
@@ -1129,18 +1137,20 @@ NOTICE:  f_leak => yyyyyy
 -- updates with from clause
 EXPLAIN (COSTS OFF) UPDATE t2 SET b=t2.b FROM t3
 WHERE t2.a = 3 and t3.a = 2 AND f_leak(t2.b) AND f_leak(t3.b);
-                          QUERY PLAN                           
----------------------------------------------------------------
+                             QUERY PLAN                              
+---------------------------------------------------------------------
  Update on t2 t2_1
    ->  Nested Loop
          ->  Subquery Scan on t2
                Filter: f_leak(t2.b)
-               ->  LockRows
-                     ->  Seq Scan on t2 t2_2
-                           Filter: ((a = 3) AND ((a % 2) = 1))
+               ->  Subquery Scan on t2_2
+                     Filter: ((t2_2.a % 2) = 1)
+                     ->  LockRows
+                           ->  Seq Scan on t2 t2_3
+                                 Filter: ((a = 3) AND ((a % 2) = 1))
          ->  Seq Scan on t3
                Filter: (f_leak(b) AND (a = 2))
-(9 rows)
+(11 rows)
 
 UPDATE t2 SET b=t2.b FROM t3
 WHERE t2.a = 3 and t3.a = 2 AND f_leak(t2.b) AND f_leak(t3.b);
@@ -1150,8 +1160,8 @@ NOTICE:  f_leak => zzz
 NOTICE:  f_leak => yyyyyy
 EXPLAIN (COSTS OFF) UPDATE t1 SET b=t1.b FROM t2
 WHERE t1.a = 3 and t2.a = 3 AND f_leak(t1.b) AND f_leak(t2.b);
-                          QUERY PLAN                           
----------------------------------------------------------------
+                             QUERY PLAN                              
+---------------------------------------------------------------------
  Update on t1 t1_3
    Update on t1 t1_3
    Update on t2 t1
@@ -1159,9 +1169,11 @@ WHERE t1.a = 3 and t2.a = 3 AND f_leak(t1.b) AND f_leak(t2.b);
    ->  Nested Loop
          ->  Subquery Scan on t1
                Filter: f_leak(t1.b)
-               ->  LockRows
-                     ->  Seq Scan on t1 t1_4
-                           Filter: ((a = 3) AND ((a % 2) = 0))
+               ->  Subquery Scan on t1_4
+                     Filter: ((t1_4.a % 2) = 0)
+                     ->  LockRows
+                           ->  Seq Scan on t1 t1_5
+                                 Filter: ((a = 3) AND ((a % 2) = 0))
          ->  Subquery Scan on t2
                Filter: f_leak(t2.b)
                ->  Seq Scan on t2 t2_3
@@ -1169,9 +1181,11 @@ WHERE t1.a = 3 and t2.a = 3 AND f_leak(t1.b) AND f_leak(t2.b);
    ->  Nested Loop
          ->  Subquery Scan on t1_1
                Filter: f_leak(t1_1.b)
-               ->  LockRows
-                     ->  Seq Scan on t2 t2_4
-                           Filter: ((a = 3) AND ((a % 2) = 0))
+               ->  Subquery Scan on t1_6
+                     Filter: ((t1_6.a % 2) = 0)
+                     ->  LockRows
+                           ->  Seq Scan on t2 t2_4
+                                 Filter: ((a = 3) AND ((a % 2) = 0))
          ->  Subquery Scan on t2_1
                Filter: f_leak(t2_1.b)
                ->  Seq Scan on t2 t2_5
@@ -1179,14 +1193,16 @@ WHERE t1.a = 3 and t2.a = 3 AND f_leak(t1.b) AND f_leak(t2.b);
    ->  Nested Loop
          ->  Subquery Scan on t1_2
                Filter: f_leak(t1_2.b)
-               ->  LockRows
-                     ->  Seq Scan on t3
-                           Filter: ((a = 3) AND ((a % 2) = 0))
+               ->  Subquery Scan on t1_7
+                     Filter: ((t1_7.a % 2) = 0)
+                     ->  LockRows
+                           ->  Seq Scan on t3
+                                 Filter: ((a = 3) AND ((a % 2) = 0))
          ->  Subquery Scan on t2_2
                Filter: f_leak(t2_2.b)
                ->  Seq Scan on t2 t2_6
                      Filter: ((a = 3) AND ((a % 2) = 1))
-(34 rows)
+(40 rows)
 
 UPDATE t1 SET b=t1.b FROM t2
 WHERE t1.a = 3 and t2.a = 3 AND f_leak(t1.b) AND f_leak(t2.b);
@@ -1198,20 +1214,22 @@ WHERE t1.a = 3 and t2.a = 3 AND f_leak(t1.b) AND f_leak(t2.b);
    ->  Nested Loop
          ->  Subquery Scan on t2
                Filter: f_leak(t2.b)
-               ->  LockRows
-                     ->  Seq Scan on t2 t2_2
-                           Filter: ((a = 3) AND ((a % 2) = 1))
+               ->  Subquery Scan on t2_2
+                     Filter: ((t2_2.a % 2) = 1)
+                     ->  LockRows
+                           ->  Seq Scan on t2 t2_3
+                                 Filter: ((a = 3) AND ((a % 2) = 1))
          ->  Subquery Scan on t1
                Filter: f_leak(t1.b)
                ->  Result
                      ->  Append
                            ->  Seq Scan on t1 t1_1
                                  Filter: ((a = 3) AND ((a % 2) = 0))
-                           ->  Seq Scan on t2 t2_3
+                           ->  Seq Scan on t2 t2_4
                                  Filter: ((a = 3) AND ((a % 2) = 0))
                            ->  Seq Scan on t3
                                  Filter: ((a = 3) AND ((a % 2) = 0))
-(17 rows)
+(19 rows)
 
 UPDATE t2 SET b=t2.b FROM t1
 WHERE t1.a = 3 and t2.a = 3 AND f_leak(t1.b) AND f_leak(t2.b);
@@ -1349,39 +1367,47 @@ SELECT * FROM t1 ORDER BY a,b;
 SET SESSION AUTHORIZATION rls_regress_user1;
 SET row_security TO ON;
 EXPLAIN (COSTS OFF) DELETE FROM only t1 WHERE f_leak(b);
-                QUERY PLAN                 
--------------------------------------------
+                   QUERY PLAN                    
+-------------------------------------------------
  Delete on t1 t1_1
    ->  Subquery Scan on t1
          Filter: f_leak(t1.b)
-         ->  LockRows
-               ->  Seq Scan on t1 t1_2
-                     Filter: ((a % 2) = 0)
-(6 rows)
+         ->  Subquery Scan on t1_2
+               Filter: ((t1_2.a % 2) = 0)
+               ->  LockRows
+                     ->  Seq Scan on t1 t1_3
+                           Filter: ((a % 2) = 0)
+(8 rows)
 
 EXPLAIN (COSTS OFF) DELETE FROM t1 WHERE f_leak(b);
-                QUERY PLAN                 
--------------------------------------------
+                   QUERY PLAN                    
+-------------------------------------------------
  Delete on t1 t1_3
    Delete on t1 t1_3
    Delete on t2 t1
    Delete on t3 t1
    ->  Subquery Scan on t1
          Filter: f_leak(t1.b)
-         ->  LockRows
-               ->  Seq Scan on t1 t1_4
-                     Filter: ((a % 2) = 0)
+         ->  Subquery Scan on t1_4
+               Filter: ((t1_4.a % 2) = 0)
+               ->  LockRows
+                     ->  Seq Scan on t1 t1_5
+                           Filter: ((a % 2) = 0)
    ->  Subquery Scan on t1_1
          Filter: f_leak(t1_1.b)
-         ->  LockRows
-               ->  Seq Scan on t2
-                     Filter: ((a % 2) = 0)
+         ->  Subquery Scan on t1_6
+               Filter: ((t1_6.a % 2) = 0)
+               ->  LockRows
+                     ->  Seq Scan on t2
+                           Filter: ((a % 2) = 0)
    ->  Subquery Scan on t1_2
          Filter: f_leak(t1_2.b)
-         ->  LockRows
-               ->  Seq Scan on t3
-                     Filter: ((a % 2) = 0)
-(19 rows)
+         ->  Subquery Scan on t1_7
+               Filter: ((t1_7.a % 2) = 0)
+               ->  LockRows
+                     ->  Seq Scan on t3
+                           Filter: ((a % 2) = 0)
+(25 rows)
 
 DELETE FROM only t1 WHERE f_leak(b) RETURNING oid, *, t1;
 NOTICE:  f_leak => bbbbbb_updt
@@ -1452,10 +1478,11 @@ EXPLAIN (COSTS OFF) UPDATE bv1 SET b = 'yyy' WHERE a = 4 AND f_leak(b);
    ->  Subquery Scan on b1
          Filter: f_leak(b1.b)
          ->  Subquery Scan on b1_2
+               Filter: ((b1_2.a % 2) = 0)
                ->  LockRows
                      ->  Seq Scan on b1 b1_3
                            Filter: ((a > 0) AND (a = 4) AND ((a % 2) = 0))
-(7 rows)
+(8 rows)
 
 UPDATE bv1 SET b = 'yyy' WHERE a = 4 AND f_leak(b);
 NOTICE:  f_leak => a87ff679a2f3e71d9181a67b7542122c
@@ -1466,10 +1493,11 @@ EXPLAIN (COSTS OFF) DELETE FROM bv1 WHERE a = 6 AND f_leak(b);
    ->  Subquery Scan on b1
          Filter: f_leak(b1.b)
          ->  Subquery Scan on b1_2
+               Filter: ((b1_2.a % 2) = 0)
                ->  LockRows
                      ->  Seq Scan on b1 b1_3
                            Filter: ((a > 0) AND (a = 6) AND ((a % 2) = 0))
-(7 rows)
+(8 rows)
 
 DELETE FROM bv1 WHERE a = 6 AND f_leak(b);
 NOTICE:  f_leak => 1679091c5a880faf6fb5e6087eb1b2dc
@@ -2743,15 +2771,17 @@ SELECT * FROM current_check;
 
 -- Plan should be a subquery TID scan
 EXPLAIN (COSTS OFF) UPDATE current_check SET payload = payload WHERE CURRENT OF current_check_cursor;
-                          QUERY PLAN                           
----------------------------------------------------------------
+                             QUERY PLAN                              
+---------------------------------------------------------------------
  Update on current_check current_check_1
    ->  Subquery Scan on current_check
-         ->  LockRows
-               ->  Tid Scan on current_check current_check_2
-                     TID Cond: CURRENT OF current_check_cursor
-                     Filter: (currentid = 4)
-(6 rows)
+         ->  Subquery Scan on current_check_2
+               Filter: ((current_check_2.currentid % 2) = 0)
+               ->  LockRows
+                     ->  Tid Scan on current_check current_check_3
+                           TID Cond: CURRENT OF current_check_cursor
+                           Filter: (currentid = 4)
+(8 rows)
 
 -- Similarly can only delete row 4
 FETCH ABSOLUTE 1 FROM current_check_cursor;
-- 
1.9.1