Clean up some mess in row-security patches.

Fix unsafe coding around PG_TRY in RelationBuildRowSecurity: can't change
a variable inside PG_TRY and then use it in PG_CATCH without marking it
"volatile".  In this case though it seems saner to avoid that by doing
a single assignment before entering the TRY block.

I started out just intending to fix that, but the more I looked at the
row-security code the more distressed I got.  This patch also fixes
incorrect construction of the RowSecurityPolicy cache entries (there was
not sufficient care taken to copy pass-by-ref data into the cache memory
context) and a whole bunch of sloppiness around the definition and use of
pg_policy.polcmd.  You can't use nulls in that column because initdb will
mark it NOT NULL --- and I see no particular reason why a null entry would
be a good idea anyway, so changing initdb's behavior is not the right
answer.  The internal value of '\0' wouldn't be suitable in a "char" column
either, so after a bit of thought I settled on using '*' to represent ALL.
Chasing those changes down also revealed that somebody wasn't paying
attention to what the underlying values of ACL_UPDATE_CHR etc really were,
and there was a great deal of lackadaiscalness in the catalogs.sgml
documentation for pg_policy and pg_policies too.

This doesn't pretend to be a complete code review for the row-security
stuff, it just fixes the things that were in my face while dealing with
the bugs in RelationBuildRowSecurity.

1 files changed, 9 insertions, 10 deletions

diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index 35790a948f5..8f8e291fb88 100644
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -273,8 +273,7 @@ prepend_row_security_policies(Query* root, RangeTblEntry* rte, int rt_index)
 	 * query, then set hasSubLinks on the Query to force subLinks to be
 	 * properly expanded.
 	 */
-	if (hassublinks)
-		root->hasSubLinks = hassublinks;
+	root->hasSubLinks |= hassublinks;
 
 	/* If we got this far, we must have added quals */
 	return true;
@@ -305,36 +304,36 @@ pull_row_security_policies(CmdType cmd, Relation relation, Oid user_id)
 		policy = (RowSecurityPolicy *) lfirst(item);
 
 		/* Always add ALL policies, if they exist. */
-		if (policy->cmd == '\0' &&
+		if (policy->polcmd == '*' &&
 				check_role_for_policy(policy->roles, user_id))
 			policies = lcons(policy, policies);
 
-		/* Build the list of policies to return. */
+		/* Add relevant command-specific policies to the list. */
 		switch(cmd)
 		{
 			case CMD_SELECT:
-				if (policy->cmd == ACL_SELECT_CHR
+				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->cmd == ACL_INSERT_CHR
+				if (policy->polcmd == ACL_INSERT_CHR
 					&& check_role_for_policy(policy->roles, user_id))
 					policies = lcons(policy, policies);
 				break;
 			case CMD_UPDATE:
-				if (policy->cmd == ACL_UPDATE_CHR
+				if (policy->polcmd == ACL_UPDATE_CHR
 					&& check_role_for_policy(policy->roles, user_id))
 					policies = lcons(policy, policies);
 				break;
 			case CMD_DELETE:
-				if (policy->cmd == ACL_DELETE_CHR
+				if (policy->polcmd == ACL_DELETE_CHR
 					&& check_role_for_policy(policy->roles, user_id))
 					policies = lcons(policy, policies);
 				break;
 			default:
-				elog(ERROR, "unrecognized command type.");
+				elog(ERROR, "unrecognized policy command type %d", (int) cmd);
 				break;
 		}
 	}
@@ -354,7 +353,7 @@ pull_row_security_policies(CmdType cmd, Relation relation, Oid user_id)
 		policy = palloc0(sizeof(RowSecurityPolicy));
 		policy->policy_name = pstrdup("default-deny policy");
 		policy->policy_id = InvalidOid;
-		policy->cmd = '\0';
+		policy->polcmd = '*';
 		policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true,
 										'i');
 		policy->qual = (Expr *) makeConst(BOOLOID, -1, InvalidOid,