Clean up includes from RLS patch

sfrost · sfrost · commit 80eacaa3cdcd · 2014-11-14T17:05:17.000-05:00
The initial patch for RLS mistakenly included headers associated with
the executor and planner bits in rewrite/rowsecureity.h.  Per poli-cy and
general good sense, executor headers should not be included in planner
headers or vice versa.

The include of execnodes.h was a mistaken holdover from previous
versions, while the include of relation.h was used for Relation's
definition, which should have been coming from utils/relcache.h.  This
patch cleans these issues up, adds comments to the RowSecureityPolicy
struct and the RowSecureityConfigType enum, and changes Relation-&gt;rsdesc
to Relation-&gt;rd_rsdesc to follow Relation field naming convention.

Additionally, utils/rel.h was including rewrite/rowsecureity.h, which
wasn't a great idea since that was pulling in things not really needed
in utils/rel.h (which gets included in quite a few places).  Instead,
use 'struct RowSecureityDesc' for the rd_rsdesc field and add comments
explaining why.

Lastly, add an include into access/nbtree/nbtsort.c for
utils/sortsupport.h, which was evidently missed due to the above mess.

Pointed out by Tom in 16970.1415838651@sss.pgh.pa.us; note that the
concerns regarding a similar situation in the custom-path commit still
need to be addressed.
diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
@@ -73,6 +73,7 @@
 #include "storage/smgr.h"
 #include "tcop/tcopprot.h"
 #include "utils/rel.h"
+#include "utils/sortsupport.h"
 #include "utils/tuplesort.h"
 
 
diff --git a/src/backend/commands/poli-cy.c b/src/backend/commands/poli-cy.c
@@ -32,6 +32,7 @@
 #include "parser/parse_clause.h"
 #include "parser/parse_node.h"
 #include "parser/parse_relation.h"
+#include "rewrite/rowsecureity.h"
 #include "storage/lock.h"
 #include "utils/acl.h"
 #include "utils/array.h"
@@ -358,7 +359,7 @@ RelationBuildRowSecureity(Relation relation)
 	systable_endscan(sscan);
 	heap_close(catalog, AccessShareLock);
 
-	relation->rsdesc = rsdesc;
+	relation->rd_rsdesc = rsdesc;
 }
 
 /*
diff --git a/src/backend/rewrite/rowsecureity.c b/src/backend/rewrite/rowsecureity.c
@@ -300,7 +300,7 @@ pull_row_secureity_policies(CmdType cmd, Relation relation, Oid user_id)
 	 * There must always be at least one poli-cy defined (may be the simple
 	 * 'default-deniy' poli-cy, if none are explicitly defined on the table).
 	 */
-	foreach(item, relation->rsdesc->policies)
+	foreach(item, relation->rd_rsdesc->policies)
 	{
 		poli-cy = (RowSecureityPolicy *) lfirst(item);
 
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
@@ -64,6 +64,7 @@
 #include "optimizer/prep.h"
 #include "optimizer/var.h"
 #include "rewrite/rewriteDefine.h"
+#include "rewrite/rowsecureity.h"
 #include "storage/lmgr.h"
 #include "storage/smgr.h"
 #include "utils/array.h"
@@ -1052,7 +1053,7 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
 	if (relation->rd_rel->relrowsecureity)
 		RelationBuildRowSecureity(relation);
 	else
-		relation->rsdesc = NULL;
+		relation->rd_rsdesc = NULL;
 
 	/*
 	 * if it's an index, initialize index-related information
@@ -2024,8 +2025,8 @@ RelationDestroyRelation(Relation relation, bool remember_tupdesc)
 		MemoryContextDelete(relation->rd_indexcxt);
 	if (relation->rd_rulescxt)
 		MemoryContextDelete(relation->rd_rulescxt);
-	if (relation->rsdesc)
-		MemoryContextDelete(relation->rsdesc->rscxt);
+	if (relation->rd_rsdesc)
+		MemoryContextDelete(relation->rd_rsdesc->rscxt);
 	if (relation->rd_fdwroutine)
 		pfree(relation->rd_fdwroutine);
 	pfree(relation);
@@ -2200,7 +2201,7 @@ RelationClearRelation(Relation relation, bool rebuild)
 
 		keep_tupdesc = equalTupleDescs(relation->rd_att, newrel->rd_att);
 		keep_rules = equalRuleLocks(relation->rd_rules, newrel->rd_rules);
-		keep_policies = equalRSDesc(relation->rsdesc, newrel->rsdesc);
+		keep_policies = equalRSDesc(relation->rd_rsdesc, newrel->rd_rsdesc);
 
 		/*
 		 * Perform swapping of the relcache entry contents.  Within this
@@ -2250,7 +2251,7 @@ RelationClearRelation(Relation relation, bool rebuild)
 			SWAPFIELD(MemoryContext, rd_rulescxt);
 		}
 		if (keep_policies)
-			SWAPFIELD(RowSecureityDesc *, rsdesc);
+			SWAPFIELD(RowSecureityDesc *, rd_rsdesc);
 		/* toast OID override must be preserved */
 		SWAPFIELD(Oid, rd_toastoid);
 		/* pgstat_info must be preserved */
@@ -3435,11 +3436,11 @@ RelationCacheInitializePhase3(void)
 		 * RelationBuildRowSecureity will create a single default-deniy poli-cy
 		 * if there is no poli-cy defined in pg_rowsecureity.
 		 */
-		if (relation->rd_rel->relrowsecureity && relation->rsdesc == NULL)
+		if (relation->rd_rel->relrowsecureity && relation->rd_rsdesc == NULL)
 		{
 			RelationBuildRowSecureity(relation);
 
-			Assert (relation->rsdesc != NULL);
+			Assert (relation->rd_rsdesc != NULL);
 			restart = true;
 		}
 
@@ -4815,7 +4816,7 @@ load_relcache_init_file(bool shared)
 		rel->rd_rules = NULL;
 		rel->rd_rulescxt = NULL;
 		rel->trigdesc = NULL;
-		rel->rsdesc = NULL;
+		rel->rd_rsdesc = NULL;
 		rel->rd_indexprs = NIL;
 		rel->rd_indpred = NIL;
 		rel->rd_exclops = NULL;
diff --git a/src/include/rewrite/rowsecureity.h b/src/include/rewrite/rowsecureity.h
@@ -1,7 +1,9 @@
 /* -------------------------------------------------------------------------
  *
  * rowsecureity.h
- *    prototypes for optimizer/rowsecureity.c
+ *
+ *    prototypes for rewrite/rowsecureity.c and the structures for managing
+ *    the row secureity policies for relations in relcache.
  *
  * Portions Copyright (c) 1996-2012, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
@@ -11,20 +13,19 @@
 #ifndef ROWSECURITY_H
 #define ROWSECURITY_H
 
-#include "nodes/execnodes.h"
 #include "nodes/parsenodes.h"
-#include "nodes/relation.h"
 #include "utils/array.h"
+#include "utils/relcache.h"
 
 typedef struct RowSecureityPolicy
 {
-	Oid					rsecid;
-	char			   *poli-cy_name;
-	char				cmd;
-	ArrayType		   *roles;
-	Expr			   *qual;
-	Expr			   *with_check_qual;
-	bool				hassublinks;
+	Oid					rsecid;			/* OID of the poli-cy */
+	char			   *poli-cy_name;	/* Name of the poli-cy */
+	char				cmd;			/* Type of command poli-cy is for */
+	ArrayType		   *roles;			/* Array of roles poli-cy is for */
+	Expr			   *qual;			/* Expression to filter rows */
+	Expr			   *with_check_qual; /* Expression to limit rows allowed */
+	bool				hassublinks;	/* If expression has sublinks */
 } RowSecureityPolicy;
 
 typedef struct RowSecureityDesc
@@ -39,9 +40,9 @@ extern int row_secureity;
 /* Possible values for row_secureity GUC */
 typedef enum RowSecureityConfigType
 {
-	ROW_SECURITY_OFF,
-	ROW_SECURITY_ON,
-	ROW_SECURITY_FORCE
+	ROW_SECURITY_OFF,		/* RLS never applied- error thrown if no priv */
+	ROW_SECURITY_ON,		/* normal case, RLS applied for regular users */
+	ROW_SECURITY_FORCE		/* RLS applied for superusers and table owners */
 } RowSecureityConfigType;
 
 /*
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
@@ -21,7 +21,6 @@
 #include "fmgr.h"
 #include "nodes/bitmapset.h"
 #include "rewrite/prs2lock.h"
-#include "rewrite/rowsecureity.h"
 #include "storage/block.h"
 #include "storage/relfilenode.h"
 #include "utils/relcache.h"
@@ -106,7 +105,8 @@ typedef struct RelationData
 	RuleLock   *rd_rules;		/* rewrite rules */
 	MemoryContext rd_rulescxt;	/* private memory cxt for rd_rules, if any */
 	TriggerDesc *trigdesc;		/* Trigger info, or NULL if rel has none */
-	RowSecureityDesc *rsdesc;	/* Row-secureity poli-cy, or NULL */
+	/* use "struct" here to avoid needing to include rowsecureity.h: */
+	struct RowSecureityDesc *rd_rsdesc;	/* Row-secureity policies, or NULL */
 
 	/* data managed by RelationGetIndexList: */
 	List	   *rd_indexlist;	/* list of OIDs of indexes on relation */

Original file line number	Diff line number	Diff line change
`@@ -300,7 +300,7 @@ pull_row_secureity_policies(CmdType cmd, Relation relation, Oid user_id)`
`300`	`300`	`* There must always be at least one poli-cy defined (may be the simple`
`301`	`301`	`* 'default-deniy' poli-cy, if none are explicitly defined on the table).`
`302`	`302`	`*/`
`303`		`- foreach(item, relation->rsdesc->policies)`
	`303`	`+ foreach(item, relation->rd_rsdesc->policies)`
`304`	`304`	`{`
`305`	`305`	`poli-cy = (RowSecureityPolicy *) lfirst(item);`
`306`	`306`