]> granicus.if.org Git - postgresql/commitdiff
Clean up some mess in row-security patches.
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 24 Jan 2015 21:16:22 +0000 (16:16 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 24 Jan 2015 21:16:22 +0000 (16:16 -0500)
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.

doc/src/sgml/catalogs.sgml
src/backend/catalog/system_views.sql
src/backend/commands/policy.c
src/backend/rewrite/rowsecurity.c
src/backend/utils/cache/relcache.c
src/bin/pg_dump/pg_dump.c
src/bin/psql/describe.c
src/include/catalog/catversion.h
src/include/catalog/pg_policy.h
src/include/rewrite/rowsecurity.h
src/test/regress/expected/rules.out

index 9ceb96b54c74bc090373050e069a25897432e458..62305d2bb3ec17b08d7794d4798d5d399279bb0f 100644 (file)
       <entry>template data for procedural languages</entry>
      </row>
 
+     <row>
+      <entry><link linkend="catalog-pg-policy"><structname>pg_policy</structname></link></entry>
+      <entry>row-security policies</entry>
+     </row>
+
      <row>
       <entry><link linkend="catalog-pg-proc"><structname>pg_proc</structname></link></entry>
       <entry>functions and procedures</entry>
       <entry>replication slot information</entry>
      </row>
 
-     <row>
-      <entry><link linkend="catalog-pg-policy"><structname>pg_policy</structname></link></entry>
-      <entry>table policies</entry>
-     </row>
-
      <row>
       <entry><link linkend="catalog-pg-seclabel"><structname>pg_seclabel</structname></link></entry>
       <entry>security labels on database objects</entry>
      </row>
 
      <row>
-      <entry><structfield>relrowsecurity</structfield></entry>
+      <entry><structfield>relhassubclass</structfield></entry>
       <entry><type>bool</type></entry>
       <entry></entry>
-      <entry>
-       True if table has row level security enabled; see
-       <link linkend="catalog-pg-policy"><structname>pg_policy</structname></link> catalog
-      </entry>
+      <entry>True if table has (or once had) any inheritance children</entry>
      </row>
 
      <row>
-      <entry><structfield>relhassubclass</structfield></entry>
+      <entry><structfield>relrowsecurity</structfield></entry>
       <entry><type>bool</type></entry>
       <entry></entry>
-      <entry>True if table has (or once had) any inheritance children</entry>
+      <entry>
+       True if table has row-level security enabled; see
+       <link linkend="catalog-pg-policy"><structname>pg_policy</structname></link> catalog
+      </entry>
      </row>
 
      <row>
 
  </sect1>
 
+ <sect1 id="catalog-pg-policy">
+  <title><structname>pg_policy</structname></title>
+
+  <indexterm zone="catalog-pg-policy">
+   <primary>pg_policy</primary>
+  </indexterm>
+
+  <para>
+   The catalog <structname>pg_policy</structname> stores row-level
+   security policies for tables.  A policy includes the kind of
+   command that it applies to (possibly all commands), the roles that it
+   applies to, the expression to be added as a security-barrier
+   qualification to queries that include the table, and the expression
+   to be added as a <literal>WITH CHECK</> option for queries that attempt to
+   add new records to the table.
+  </para>
+
+  <table>
+
+   <title><structname>pg_policy</structname> Columns</title>
+
+   <tgroup cols="4">
+    <thead>
+     <row>
+      <entry>Name</entry>
+      <entry>Type</entry>
+      <entry>References</entry>
+      <entry>Description</entry>
+     </row>
+    </thead>
+
+    <tbody>
+     <row>
+      <entry><structfield>polname</structfield></entry>
+      <entry><type>name</type></entry>
+      <entry></entry>
+      <entry>The name of the policy</entry>
+     </row>
+
+     <row>
+      <entry><structfield>polrelid</structfield></entry>
+      <entry><type>oid</type></entry>
+      <entry><literal><link linkend="catalog-pg-class"><structname>pg_class</structname></link>.oid</literal></entry>
+      <entry>The table to which the policy applies</entry>
+     </row>
+
+     <row>
+      <entry><structfield>polcmd</structfield></entry>
+      <entry><type>char</type></entry>
+      <entry></entry>
+      <entry>The command type to which the policy is applied:
+       <literal>r</> for <command>SELECT</>,
+       <literal>a</> for <command>INSERT</>,
+       <literal>w</> for <command>UPDATE</>,
+       <literal>d</> for <command>DELETE</>,
+       or <literal>*</> for all</entry>
+     </row>
+
+     <row>
+      <entry><structfield>polroles</structfield></entry>
+      <entry><type>oid[]</type></entry>
+      <entry><literal><link linkend="catalog-pg-authid"><structname>pg_authid</structname></link>.oid</literal></entry>
+      <entry>The roles to which the policy is applied</entry>
+     </row>
+
+     <row>
+      <entry><structfield>polqual</structfield></entry>
+      <entry><type>pg_node_tree</type></entry>
+      <entry></entry>
+      <entry>The expression tree to be added to the security barrier qualifications for queries that use the table</entry>
+     </row>
+
+     <row>
+      <entry><structfield>polwithcheck</structfield></entry>
+      <entry><type>pg_node_tree</type></entry>
+      <entry></entry>
+      <entry>The expression tree to be added to the WITH CHECK qualifications for queries that attempt to add rows to the table</entry>
+     </row>
+
+    </tbody>
+   </tgroup>
+  </table>
+
+  <note>
+   <para>
+    Policies stored in <structname>pg_policy</> are applied only when
+    <structname>pg_class</>.<structfield>relrowsecurity</> is set for
+    their table.
+   </para>
+  </note>
+
+ </sect1>
 
  <sect1 id="catalog-pg-proc">
   <title><structname>pg_proc</structname></title>
   </table>
  </sect1>
 
- <sect1 id="catalog-pg-policy">
-  <title><structname>pg_policy</structname></title>
-
-  <indexterm zone="catalog-pg-policy">
-   <primary>pg_policy</primary>
-  </indexterm>
-
-  <para>
-   The catalog <structname>pg_policy</structname> stores row-level
-   security policies for each table.  A policy includes the kind of
-   command which it applies to (or all commands), the roles which it
-   applies to, the expression to be added as a security-barrier
-   qualification to queries which include the table and the expression
-   to be added as a with-check option for queries which attempt to add
-   new records to the table.
-  </para>
-
-  <table>
-
-   <title><structname>pg_policy</structname> Columns</title>
-
-   <tgroup cols="4">
-    <thead>
-     <row>
-      <entry>Name</entry>
-      <entry>Type</entry>
-      <entry>References</entry>
-      <entry>Description</entry>
-     </row>
-    </thead>
-
-    <tbody>
-     <row>
-      <entry><structfield>polname</structfield></entry>
-      <entry><type>name</type></entry>
-      <entry></entry>
-      <entry>The name of the policy</entry>
-     </row>
-
-     <row>
-      <entry><structfield>polrelid</structfield></entry>
-      <entry><type>oid</type></entry>
-      <entry><literal><link linkend="catalog-pg-class"><structname>pg_class</structname></link>.oid</literal></entry>
-      <entry>The table to which the policy belongs</entry>
-     </row>
-
-     <row>
-      <entry><structfield>polcmd</structfield></entry>
-      <entry><type>char</type></entry>
-      <entry></entry>
-      <entry>The command type to which the policy is applied.</entry>
-     </row>
-
-     <row>
-      <entry><structfield>polroles</structfield></entry>
-      <entry><type>char</type></entry>
-      <entry></entry>
-      <entry>The roles to which the policy is applied.</entry>
-     </row>
-
-     <row>
-      <entry><structfield>polqual</structfield></entry>
-      <entry><type>pg_node_tree</type></entry>
-      <entry></entry>
-      <entry>The expression tree to be added to the security barrier qualifications for queries which use the table.</entry>
-     </row>
-
-     <row>
-      <entry><structfield>polwithcheck</structfield></entry>
-      <entry><type>pg_node_tree</type></entry>
-      <entry></entry>
-      <entry>The expression tree to be added to the with check qualifications for queries which attempt to add rows to the table.</entry>
-     </row>
-
-    </tbody>
-   </tgroup>
-  </table>
-
-  <note>
-   <para>
-    <literal>pg_class.relrowsecurity</literal>
-    True if the table has row security enabled.  Policies will not be applied
-    unless row security is enabled on the table.
-   </para>
-  </note>
-
- </sect1>
-
  <sect1 id="catalog-pg-seclabel">
   <title><structname>pg_seclabel</structname></title>
 
@@ -8166,7 +8170,7 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
 
   <para>
    The view <structname>pg_policies</structname> provides access to
-   useful information about each policy in the database.
+   useful information about each row-level security policy in the database.
   </para>
 
   <table>
@@ -8197,34 +8201,34 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
      <row>
       <entry><structfield>policyname</structfield></entry>
       <entry><type>name</type></entry>
-      <entry><literal><link linkend="catalog-pg-class"><structname>pg_class</structname></link>.relname</literal></entry>
+      <entry><literal><link linkend="catalog-pg-policy"><structname>pg_policy</structname></link>.polname</literal></entry>
       <entry>Name of policy</entry>
      </row>
      <row>
-      <entry><structfield>cmd</structfield></entry>
-      <entry><type>text</type></entry>
+      <entry><structfield>roles</structfield></entry>
+      <entry><type>name[]</type></entry>
       <entry></entry>
-      <entry>The command type to which the policy is applied.</entry>
+      <entry>The roles to which this policy applies</entry>
      </row>
      <row>
-      <entry><structfield>roles</structfield></entry>
-      <entry><type>name[]</type></entry>
+      <entry><structfield>cmd</structfield></entry>
+      <entry><type>text</type></entry>
       <entry></entry>
-      <entry>The roles to which this policy applies.</entry>
+      <entry>The command type to which the policy is applied</entry>
      </row>
      <row>
       <entry><structfield>qual</structfield></entry>
       <entry><type>text</type></entry>
       <entry></entry>
       <entry>The expression added to the security barrier qualifications for
-      queries which this policy applies to.</entry>
+      queries that this policy applies to</entry>
      </row>
      <row>
       <entry><structfield>with_check</structfield></entry>
       <entry><type>text</type></entry>
       <entry></entry>
-      <entry>The expression added to the with check qualifications for
-      queries which attempt to add rows to this table.</entry>
+      <entry>The expression added to the WITH CHECK qualifications for
+      queries that attempt to add rows to this table</entry>
      </row>
     </tbody>
    </tgroup>
index 4bc874f5c25fe856f8d950a7fc4d94e256f3f431..6df6bf27d191bf1aff41f99d4b3dddf436855c92 100644 (file)
@@ -79,13 +79,12 @@ CREATE VIEW pg_policies AS
                     WHERE oid = ANY (pol.polroles) ORDER BY 1
                 )
         END AS roles,
-        CASE WHEN pol.polcmd IS NULL THEN 'ALL' ELSE
-            CASE pol.polcmd
-                WHEN 'r' THEN 'SELECT'
-                WHEN 'a' THEN 'INSERT'
-                WHEN 'u' THEN 'UPDATE'
-                WHEN 'd' THEN 'DELETE'
-            END
+        CASE pol.polcmd
+            WHEN 'r' THEN 'SELECT'
+            WHEN 'a' THEN 'INSERT'
+            WHEN 'w' THEN 'UPDATE'
+            WHEN 'd' THEN 'DELETE'
+            WHEN '*' THEN 'ALL'
         END AS cmd,
         pg_catalog.pg_get_expr(pol.polqual, pol.polrelid) AS qual,
         pg_catalog.pg_get_expr(pol.polwithcheck, pol.polrelid) AS with_check
index 9b79d8863329a3c10aaee81d4789b6dcbb57ce64..d98da0dd506495708967510fd28237db4ab0b9e3 100644 (file)
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
 #include "nodes/pg_list.h"
-#include "optimizer/clauses.h"
 #include "parser/parse_clause.h"
 #include "parser/parse_node.h"
 #include "parser/parse_relation.h"
+#include "rewrite/rewriteManip.h"
 #include "rewrite/rowsecurity.h"
 #include "storage/lock.h"
 #include "utils/acl.h"
@@ -109,10 +109,10 @@ parse_policy_command(const char *cmd_name)
        char cmd;
 
        if (!cmd_name)
-               elog(ERROR, "unrecognized command");
+               elog(ERROR, "unrecognized policy command");
 
        if (strcmp(cmd_name, "all") == 0)
-               cmd = 0;
+               cmd = '*';
        else if (strcmp(cmd_name, "select") == 0)
                cmd = ACL_SELECT_CHR;
        else if (strcmp(cmd_name, "insert") == 0)
@@ -122,7 +122,7 @@ parse_policy_command(const char *cmd_name)
        else if (strcmp(cmd_name, "delete") == 0)
                cmd = ACL_DELETE_CHR;
        else
-               elog(ERROR, "unrecognized command");
+               elog(ERROR, "unrecognized policy command");
 
        return cmd;
 }
@@ -190,44 +190,54 @@ policy_role_list_to_array(List *roles)
 }
 
 /*
- * Load row security policy from the catalog, and keep it in
- * the relation cache.
+ * 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)
 {
-       Relation                        catalog;
-       ScanKeyData                     skey;
-       SysScanDesc                     sscan;
-       HeapTuple                       tuple;
-       MemoryContext           oldcxt;
-       MemoryContext           rscxt = NULL;
-       RowSecurityDesc    *rsdesc = NULL;
-
-       catalog = heap_open(PolicyRelationId, AccessShareLock);
+       MemoryContext           rscxt;
+       MemoryContext           oldcxt = CurrentMemoryContext;
+       RowSecurityDesc    * volatile rsdesc = NULL;
 
-       ScanKeyInit(&skey,
-                               Anum_pg_policy_polrelid,
-                               BTEqualStrategyNumber, F_OIDEQ,
-                               ObjectIdGetDatum(RelationGetRelid(relation)));
+       /*
+        * Create a memory context to hold everything associated with this
+        * relation's row security policy.  This makes it easy to clean up
+        * during a relcache flush.
+        */
+       rscxt = AllocSetContextCreate(CacheMemoryContext,
+                                                                 "row security descriptor",
+                                                                 ALLOCSET_SMALL_MINSIZE,
+                                                                 ALLOCSET_SMALL_INITSIZE,
+                                                                 ALLOCSET_SMALL_MAXSIZE);
 
-       sscan = systable_beginscan(catalog, PolicyPolrelidPolnameIndexId, true,
-                                                          NULL, 1, &skey);
+       /*
+        * Since rscxt lives under CacheMemoryContext, it is long-lived.  Use
+        * a PG_TRY block to ensure it'll get freed if we fail partway through.
+        */
        PG_TRY();
        {
-               /*
-                * Set up our memory context- we will always set up some kind of
-                * policy here.  If no explicit policies are found then an implicit
-                * default-deny policy is created.
-                */
-               rscxt = AllocSetContextCreate(CacheMemoryContext,
-                                                                         "row security descriptor",
-                                                                         ALLOCSET_SMALL_MINSIZE,
-                                                                         ALLOCSET_SMALL_INITSIZE,
-                                                                         ALLOCSET_SMALL_MAXSIZE);
+               Relation                        catalog;
+               ScanKeyData                     skey;
+               SysScanDesc                     sscan;
+               HeapTuple                       tuple;
+
                rsdesc = MemoryContextAllocZero(rscxt, sizeof(RowSecurityDesc));
                rsdesc->rscxt = rscxt;
 
+               catalog = heap_open(PolicyRelationId, AccessShareLock);
+
+               ScanKeyInit(&skey,
+                                       Anum_pg_policy_polrelid,
+                                       BTEqualStrategyNumber, F_OIDEQ,
+                                       ObjectIdGetDatum(RelationGetRelid(relation)));
+
+               sscan = systable_beginscan(catalog, PolicyPolrelidPolnameIndexId, true,
+                                                                  NULL, 1, &skey);
+
                /*
                 * Loop through the row level security policies for this relation, if
                 * any.
@@ -236,7 +246,7 @@ RelationBuildRowSecurity(Relation relation)
                {
                        Datum                           value_datum;
                        char                            cmd_value;
-                       ArrayType                  *roles;
+                       Datum                           roles_datum;
                        char                       *qual_value;
                        Expr                       *qual_expr;
                        char                       *with_check_value;
@@ -244,29 +254,33 @@ RelationBuildRowSecurity(Relation relation)
                        char                       *policy_name_value;
                        Oid                                     policy_id;
                        bool                            isnull;
-                       RowSecurityPolicy  *policy = NULL;
+                       RowSecurityPolicy  *policy;
 
-                       oldcxt = MemoryContextSwitchTo(rscxt);
+                       /*
+                        * Note: all the pass-by-reference data we collect here is either
+                        * still stored in the tuple, or constructed in the caller's
+                        * short-lived memory context.  We must copy it into rscxt
+                        * explicitly below.
+                        */
 
                        /* Get policy command */
                        value_datum = heap_getattr(tuple, Anum_pg_policy_polcmd,
                                                                 RelationGetDescr(catalog), &isnull);
-                       if (isnull)
-                               cmd_value = 0;
-                       else
-                               cmd_value = DatumGetChar(value_datum);
+                       Assert(!isnull);
+                       cmd_value = DatumGetChar(value_datum);
 
                        /* Get policy name */
                        value_datum = heap_getattr(tuple, Anum_pg_policy_polname,
                                                                                RelationGetDescr(catalog), &isnull);
                        Assert(!isnull);
-                       policy_name_value = DatumGetCString(value_datum);
+                       policy_name_value = NameStr(*(DatumGetName(value_datum)));
 
                        /* Get policy roles */
-                       value_datum = heap_getattr(tuple, Anum_pg_policy_polroles,
+                       roles_datum = heap_getattr(tuple, Anum_pg_policy_polroles,
                                                                                RelationGetDescr(catalog), &isnull);
-                       Assert(!isnull);
-                       roles = DatumGetArrayTypeP(value_datum);
+                       /* shouldn't be null, but initdb doesn't mark it so, so check */
+                       if (isnull)
+                               elog(ERROR, "unexpected null value in pg_policy.polroles");
 
                        /* Get policy qual */
                        value_datum = heap_getattr(tuple, Anum_pg_policy_polqual,
@@ -282,7 +296,6 @@ RelationBuildRowSecurity(Relation relation)
                        /* Get WITH CHECK qual */
                        value_datum = heap_getattr(tuple, Anum_pg_policy_polwithcheck,
                                                                                RelationGetDescr(catalog), &isnull);
-
                        if (!isnull)
                        {
                                with_check_value = TextDatumGetCString(value_datum);
@@ -293,27 +306,33 @@ RelationBuildRowSecurity(Relation relation)
 
                        policy_id = HeapTupleGetOid(tuple);
 
+                       /* Now copy everything into the cache context */
+                       MemoryContextSwitchTo(rscxt);
+
                        policy = palloc0(sizeof(RowSecurityPolicy));
-                       policy->policy_name = policy_name_value;
+                       policy->policy_name = pstrdup(policy_name_value);
                        policy->policy_id = policy_id;
-                       policy->cmd = cmd_value;
-                       policy->roles = roles;
+                       policy->polcmd = cmd_value;
+                       policy->roles = DatumGetArrayTypePCopy(roles_datum);
                        policy->qual = copyObject(qual_expr);
                        policy->with_check_qual = copyObject(with_check_qual);
-                       policy->hassublinks = contain_subplans((Node *) qual_expr) ||
-                                                                 contain_subplans((Node *) with_check_qual);
+                       policy->hassublinks = checkExprHasSubLink((Node *) qual_expr) ||
+                                                                 checkExprHasSubLink((Node *) with_check_qual);
 
                        rsdesc->policies = lcons(policy, rsdesc->policies);
 
                        MemoryContextSwitchTo(oldcxt);
 
+                       /* clean up some (not all) of the junk ... */
                        if (qual_expr != NULL)
                                pfree(qual_expr);
-
                        if (with_check_qual != NULL)
                                pfree(with_check_qual);
                }
 
+               systable_endscan(sscan);
+               heap_close(catalog, AccessShareLock);
+
                /*
                 * Check if no policies were added
                 *
@@ -324,17 +343,17 @@ RelationBuildRowSecurity(Relation relation)
                 */
                if (rsdesc->policies == NIL)
                {
-                       RowSecurityPolicy  *policy = NULL;
+                       RowSecurityPolicy  *policy;
                        Datum                           role;
 
-                       oldcxt = MemoryContextSwitchTo(rscxt);
+                       MemoryContextSwitchTo(rscxt);
 
                        role = ObjectIdGetDatum(ACL_ID_PUBLIC);
 
                        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,
@@ -350,15 +369,14 @@ RelationBuildRowSecurity(Relation relation)
        }
        PG_CATCH();
        {
-               if (rscxt != NULL)
-                       MemoryContextDelete(rscxt);
+               /* Delete rscxt, first making sure it isn't active */
+               MemoryContextSwitchTo(oldcxt);
+               MemoryContextDelete(rscxt);
                PG_RE_THROW();
        }
        PG_END_TRY();
 
-       systable_endscan(sscan);
-       heap_close(catalog, AccessShareLock);
-
+       /* Success --- attach the policy descriptor to the relcache entry */
        relation->rd_rsdesc = rsdesc;
 }
 
@@ -555,27 +573,20 @@ CreatePolicy(CreatePolicyStmt *stmt)
                                 stmt->policy_name, RelationGetRelationName(target_table))));
 
        values[Anum_pg_policy_polrelid - 1] = ObjectIdGetDatum(table_id);
-       values[Anum_pg_policy_polname - 1]
-               = DirectFunctionCall1(namein, CStringGetDatum(stmt->policy_name));
-
-       if (polcmd)
-               values[Anum_pg_policy_polcmd - 1] = CharGetDatum(polcmd);
-       else
-               isnull[Anum_pg_policy_polcmd - 1] = true;
-
+       values[Anum_pg_policy_polname - 1] = DirectFunctionCall1(namein,
+                                                                                                                        CStringGetDatum(stmt->policy_name));
+       values[Anum_pg_policy_polcmd - 1] = CharGetDatum(polcmd);
        values[Anum_pg_policy_polroles - 1] = PointerGetDatum(role_ids);
 
        /* Add qual if present. */
        if (qual)
-               values[Anum_pg_policy_polqual - 1]
-                       = CStringGetTextDatum(nodeToString(qual));
+               values[Anum_pg_policy_polqual - 1] = CStringGetTextDatum(nodeToString(qual));
        else
                isnull[Anum_pg_policy_polqual - 1] = true;
 
        /* Add WITH CHECK qual if present */
        if (with_check_qual)
-               values[Anum_pg_policy_polwithcheck - 1]
-                       = CStringGetTextDatum(nodeToString(with_check_qual));
+               values[Anum_pg_policy_polwithcheck - 1] = CStringGetTextDatum(nodeToString(with_check_qual));
        else
                isnull[Anum_pg_policy_polwithcheck - 1] = true;
 
@@ -738,10 +749,8 @@ AlterPolicy(AlterPolicyStmt *stmt)
        cmd_datum = heap_getattr(policy_tuple, Anum_pg_policy_polcmd,
                                                         RelationGetDescr(pg_policy_rel),
                                                         &polcmd_isnull);
-       if (polcmd_isnull)
-               polcmd = 0;
-       else
-               polcmd = DatumGetChar(cmd_datum);
+       Assert(!polcmd_isnull);
+       polcmd = DatumGetChar(cmd_datum);
 
        /*
         * If the command is SELECT or DELETE then WITH CHECK should be NULL.
index 35790a948f5f96871a93f7a85443a20579868f82..8f8e291fb887bf80162a1c2fed5a86628227d030 100644 (file)
@@ -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,
index 24c92e791ff99b57927f918b96ddba4648ec9be7..1db4ba8410060053b8f5139d4265c41f7276cb93 100644 (file)
@@ -868,7 +868,7 @@ equalPolicy(RowSecurityPolicy *policy1, RowSecurityPolicy *policy2)
 
                if (policy1->policy_id != policy2->policy_id)
                        return false;
-               if (policy1->cmd != policy2->cmd)
+               if (policy1->polcmd != policy2->polcmd)
                        return false;
                if (policy1->hassublinks != policy2->hassublinks)
                        return false;
index 1e330f243abd38088f2a55ace4acde92072af88e..c3ebb3a9b0b55868a7303c85f9c262565d75affb 100644 (file)
@@ -2851,9 +2851,9 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int numTables)
                appendPQExpBuffer(query,
                                                  "SELECT oid, tableoid, pol.polname, pol.polcmd, "
                                                  "CASE WHEN pol.polroles = '{0}' THEN 'PUBLIC' ELSE "
-                                                 "   array_to_string(ARRAY(SELECT rolname from pg_roles WHERE oid = ANY(pol.polroles)), ', ') END AS polroles, "
-                                                 "pg_get_expr(pol.polqual, pol.polrelid) AS polqual, "
-                               "pg_get_expr(pol.polwithcheck, pol.polrelid) AS polwithcheck "
+                                                 "   pg_catalog.array_to_string(ARRAY(SELECT pg_catalog.quote_ident(rolname) from pg_catalog.pg_roles WHERE oid = ANY(pol.polroles)), ', ') END AS polroles, "
+                                                 "pg_catalog.pg_get_expr(pol.polqual, pol.polrelid) AS polqual, "
+                               "pg_catalog.pg_get_expr(pol.polwithcheck, pol.polrelid) AS polwithcheck "
                                                  "FROM pg_catalog.pg_policy pol "
                                                  "WHERE polrelid = '%u'",
                                                  tbinfo->dobj.catId.oid);
@@ -2865,7 +2865,7 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int numTables)
                {
                        /*
                         * No explicit policies to handle (only the default-deny policy,
-                        * which is handled as part of the table definition.  Clean up and
+                        * which is handled as part of the table definition).  Clean up and
                         * return.
                         */
                        PQclear(res);
@@ -2892,14 +2892,9 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int numTables)
                        polinfo[j].dobj.namespace = tbinfo->dobj.namespace;
                        polinfo[j].poltable = tbinfo;
                        polinfo[j].polname = pg_strdup(PQgetvalue(res, j, i_polname));
-
                        polinfo[j].dobj.name = pg_strdup(polinfo[j].polname);
 
-                       if (PQgetisnull(res, j, i_polcmd))
-                               polinfo[j].polcmd = NULL;
-                       else
-                               polinfo[j].polcmd = pg_strdup(PQgetvalue(res, j, i_polcmd));
-
+                       polinfo[j].polcmd = pg_strdup(PQgetvalue(res, j, i_polcmd));
                        polinfo[j].polroles = pg_strdup(PQgetvalue(res, j, i_polroles));
 
                        if (PQgetisnull(res, j, i_polqual))
@@ -2959,7 +2954,7 @@ dumpPolicy(Archive *fout, DumpOptions *dopt, PolicyInfo *polinfo)
                return;
        }
 
-       if (!polinfo->polcmd)
+       if (strcmp(polinfo->polcmd, "*") == 0)
                cmd = "ALL";
        else if (strcmp(polinfo->polcmd, "r") == 0)
                cmd = "SELECT";
@@ -2971,15 +2966,16 @@ dumpPolicy(Archive *fout, DumpOptions *dopt, PolicyInfo *polinfo)
                cmd = "DELETE";
        else
        {
-               write_msg(NULL, "unexpected command type: '%s'\n", polinfo->polcmd);
+               write_msg(NULL, "unexpected policy command type: \"%s\"\n",
+                                 polinfo->polcmd);
                exit_nicely(1);
        }
 
        query = createPQExpBuffer();
        delqry = createPQExpBuffer();
 
-       appendPQExpBuffer(query, "CREATE POLICY %s ON %s FOR %s",
-                                         polinfo->polname, fmtId(tbinfo->dobj.name), cmd);
+       appendPQExpBuffer(query, "CREATE POLICY %s", fmtId(polinfo->polname));
+       appendPQExpBuffer(query, " ON %s FOR %s", fmtId(tbinfo->dobj.name), cmd);
 
        if (polinfo->polroles != NULL)
                appendPQExpBuffer(query, " TO %s", polinfo->polroles);
@@ -2992,8 +2988,8 @@ dumpPolicy(Archive *fout, DumpOptions *dopt, PolicyInfo *polinfo)
 
        appendPQExpBuffer(query, ";\n");
 
-       appendPQExpBuffer(delqry, "DROP POLICY %s ON %s;\n",
-                                         polinfo->polname, fmtId(tbinfo->dobj.name));
+       appendPQExpBuffer(delqry, "DROP POLICY %s", fmtId(polinfo->polname));
+       appendPQExpBuffer(delqry, " ON %s;\n", fmtId(tbinfo->dobj.name));
 
        ArchiveEntry(fout, polinfo->dobj.catId, polinfo->dobj.dumpId,
                                 polinfo->dobj.name,
index 4cda07d87c424e3a6f8896863ad945b9fe3c3915..c44e447d06ff289ed29e66e1288ed84c54ac47ec 100644 (file)
@@ -784,8 +784,8 @@ permissionsList(const char *pattern)
                appendPQExpBuffer(&buf,
                                                  ",\n  pg_catalog.array_to_string(ARRAY(\n"
                                                  "    SELECT polname\n"
-                                                 "    || CASE WHEN polcmd IS NOT NULL THEN\n"
-                                                 "           E' (' || polcmd || E')'\n"
+                                                 "    || CASE WHEN polcmd != '*' THEN\n"
+                                                 "           E' (' || polcmd || E'):'\n"
                                                  "       ELSE E':' \n"
                                                  "       END\n"
                                                  "    || CASE WHEN polqual IS NOT NULL THEN\n"
@@ -2031,9 +2031,10 @@ describeOneTableDetails(const char *schemaname,
                                                   "pg_catalog.pg_get_expr(pol.polwithcheck, pol.polrelid),\n"
                                                   "CASE pol.polcmd \n"
                                                   "WHEN 'r' THEN 'SELECT'\n"
-                                                  "WHEN 'u' THEN 'UPDATE'\n"
                                                   "WHEN 'a' THEN 'INSERT'\n"
+                                                  "WHEN 'w' THEN 'UPDATE'\n"
                                                   "WHEN 'd' THEN 'DELETE'\n"
+                                                  "WHEN '*' THEN 'ALL'\n"
                                                   "END AS cmd\n"
                                                          "FROM pg_catalog.pg_policy pol\n"
                                  "WHERE pol.polrelid = '%s' ORDER BY 1;",
index bad9123c95dfb3373ec88f208688187455fda752..13c4376b8cc6ced155bb3b14af36276faa962c57 100644 (file)
@@ -53,6 +53,6 @@
  */
 
 /*                                                     yyyymmddN */
-#define CATALOG_VERSION_NO     201412301
+#define CATALOG_VERSION_NO     201501241
 
 #endif
index ed0c6113e61282d9cd79f110fa9b2539b1436617..ae71f3f3a2f7983e90c19e2bc682dc8bf81b49d5 100644 (file)
@@ -22,10 +22,10 @@ CATALOG(pg_policy,3256)
 {
        NameData                polname;                /* Policy name. */
        Oid                             polrelid;               /* Oid of the relation with policy. */
-       char                    polcmd;                 /* One of ACL_*_CHR, or \0 for all */
+       char                    polcmd;                 /* One of ACL_*_CHR, or '*' for all */
 
 #ifdef CATALOG_VARLEN
-       Oid                             polroles[1]             /* Roles associated with policy, not-NULL */
+       Oid                             polroles[1];    /* Roles associated with policy, not-NULL */
        pg_node_tree    polqual;                /* Policy quals. */
        pg_node_tree    polwithcheck;   /* WITH CHECK quals. */
 #endif
index aa1b45b1c97cdf24fbf028a17c55fdfba514e7d5..240f987a3a7fe6636c0c993268761e07b1ff4461 100644 (file)
@@ -21,11 +21,11 @@ typedef struct RowSecurityPolicy
 {
        Oid                                     policy_id;              /* OID of the policy */
        char                       *policy_name;        /* Name of the policy */
-       char                            cmd;                    /* Type of command policy is for */
+       char                            polcmd;                 /* 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 */
+       bool                            hassublinks;    /* If either expression has sublinks */
 } RowSecurityPolicy;
 
 typedef struct RowSecurityDesc
index 80c3351291638cc8f37194f34e0eb773564616ca..7df5d2dce9a5d27ca75bb2248031c788b05e9a1a 100644 (file)
@@ -1363,16 +1363,13 @@ pg_policies| SELECT n.nspname AS schemaname,
               WHERE (pg_authid.oid = ANY (pol.polroles))
               ORDER BY pg_authid.rolname)
         END AS roles,
-        CASE
-            WHEN (pol.polcmd IS NULL) THEN 'ALL'::text
-            ELSE
-            CASE pol.polcmd
-                WHEN 'r'::"char" THEN 'SELECT'::text
-                WHEN 'a'::"char" THEN 'INSERT'::text
-                WHEN 'u'::"char" THEN 'UPDATE'::text
-                WHEN 'd'::"char" THEN 'DELETE'::text
-                ELSE NULL::text
-            END
+        CASE pol.polcmd
+            WHEN 'r'::"char" THEN 'SELECT'::text
+            WHEN 'a'::"char" THEN 'INSERT'::text
+            WHEN 'w'::"char" THEN 'UPDATE'::text
+            WHEN 'd'::"char" THEN 'DELETE'::text
+            WHEN '*'::"char" THEN 'ALL'::text
+            ELSE NULL::text
         END AS cmd,
     pg_get_expr(pol.polqual, pol.polrelid) AS qual,
     pg_get_expr(pol.polwithcheck, pol.polrelid) AS with_check