Clean up includes from RLS patch

The initial patch for RLS mistakenly included headers associated with
the executor and planner bits in rewrite/rowsecurity.h.  Per policy 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 RowSecurityPolicy
struct and the RowSecurityConfigType enum, and changes Relation->rsdesc
to Relation->rd_rsdesc to follow Relation field naming convention.

Additionally, utils/rel.h was including rewrite/rowsecurity.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 RowSecurityDesc' 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
index e15784e1ba9f415139ad5553794d5ffbdf3ab00c..761688214a347076e6a7e7b8d191f686d815540c 100644 (file)
--- 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/policy.c b/src/backend/commands/policy.c
index 36275394306d7c39fa6d3b784d9cae605c84b0da..10d230ef431c5deb96161938c3be763899203b3a 100644 (file)
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -32,6 +32,7 @@
 #include "parser/parse_clause.h"
 #include "parser/parse_node.h"
 #include "parser/parse_relation.h"
+#include "rewrite/rowsecurity.h"
 #include "storage/lock.h"
 #include "utils/acl.h"
 #include "utils/array.h"
@@ -358,7 +359,7 @@ RelationBuildRowSecurity(Relation relation)
        systable_endscan(sscan);
        heap_close(catalog, AccessShareLock);
 
-       relation->rsdesc = rsdesc;
+       relation->rd_rsdesc = rsdesc;
 }
 
 /*

diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index bb95b367198273996655ebfc5bcbf5a1a0005693..66c358cdec9b841fb53d9b99885d628acf7ef384 100644 (file)
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -300,7 +300,7 @@ pull_row_security_policies(CmdType cmd, Relation relation, Oid user_id)
         * There must always be at least one policy defined (may be the simple
         * 'default-deny' policy, if none are explicitly defined on the table).
         */
-       foreach(item, relation->rsdesc->policies)
+       foreach(item, relation->rd_rsdesc->policies)
        {
                policy = (RowSecurityPolicy *) lfirst(item);
 

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index e8ed9995ff6a7430721d2d1d265b0643394fdb6b..4b4528f79175945c3762a9bc4cd76bbc84f67d87 100644 (file)
--- 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/rowsecurity.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->relrowsecurity)
                RelationBuildRowSecurity(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(RowSecurityDesc *, rsdesc);
+                       SWAPFIELD(RowSecurityDesc *, rd_rsdesc);
                /* toast OID override must be preserved */
                SWAPFIELD(Oid, rd_toastoid);
                /* pgstat_info must be preserved */
@@ -3435,11 +3436,11 @@ RelationCacheInitializePhase3(void)
                 * RelationBuildRowSecurity will create a single default-deny policy
                 * if there is no policy defined in pg_rowsecurity.
                 */
-               if (relation->rd_rel->relrowsecurity && relation->rsdesc == NULL)
+               if (relation->rd_rel->relrowsecurity && relation->rd_rsdesc == NULL)
                {
                        RelationBuildRowSecurity(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/rowsecurity.h b/src/include/rewrite/rowsecurity.h
index 245005cae2ebdc0061041dec7839a71d130fc11a..8b4d5c0918faacb236a1cc6b1f3d70edebf14f60 100644 (file)
--- a/src/include/rewrite/rowsecurity.h
+++ b/src/include/rewrite/rowsecurity.h
@@ -1,7 +1,9 @@
 /* -------------------------------------------------------------------------
  *
  * rowsecurity.h
- *    prototypes for optimizer/rowsecurity.c
+ *
+ *    prototypes for rewrite/rowsecurity.c and the structures for managing
+ *    the row security 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 RowSecurityPolicy
 {
-       Oid                                     rsecid;
-       char                       *policy_name;
-       char                            cmd;
-       ArrayType                  *roles;
-       Expr                       *qual;
-       Expr                       *with_check_qual;
-       bool                            hassublinks;
+       Oid                                     rsecid;                 /* OID of the policy */
+       char                       *policy_name;        /* Name of the policy */
+       char                            cmd;                    /* Type of command policy is for */
+       ArrayType                  *roles;                      /* Array of roles policy is for */
+       Expr                       *qual;                       /* Expression to filter rows */
+       Expr                       *with_check_qual; /* Expression to limit rows allowed */
+       bool                            hassublinks;    /* If expression has sublinks */
 } RowSecurityPolicy;
 
 typedef struct RowSecurityDesc
@@ -39,9 +40,9 @@ extern int row_security;
 /* Possible values for row_security GUC */
 typedef enum RowSecurityConfigType
 {
-       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 */
 } RowSecurityConfigType;
 
 /*

diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 198b98f2f8a8080c7c5815056064347c629f0632..01a9ef32ebf9158815a67fae3b4f599654c9137b 100644 (file)
--- 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/rowsecurity.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 */
-       RowSecurityDesc *rsdesc;        /* Row-security policy, or NULL */
+       /* use "struct" here to avoid needing to include rowsecurity.h: */
+       struct RowSecurityDesc *rd_rsdesc;      /* Row-security policies, or NULL */
 
        /* data managed by RelationGetIndexList: */
        List       *rd_indexlist;       /* list of OIDs of indexes on relation */