From fd496129d160950ed681c1150ea8f627b292c511 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 24 Jan 2015 16:16:22 -0500 Subject: [PATCH] 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. --- doc/src/sgml/catalogs.sgml | 234 ++++++++++++++------------- src/backend/catalog/system_views.sql | 13 +- src/backend/commands/policy.c | 155 +++++++++--------- src/backend/rewrite/rowsecurity.c | 19 ++- src/backend/utils/cache/relcache.c | 2 +- src/bin/pg_dump/pg_dump.c | 28 ++-- src/bin/psql/describe.c | 7 +- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_policy.h | 4 +- src/include/rewrite/rowsecurity.h | 4 +- src/test/regress/expected/rules.out | 17 +- 11 files changed, 245 insertions(+), 240 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 9ceb96b54c..62305d2bb3 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -218,6 +218,11 @@ template data for procedural languages + + pg_policy + row-security policies + + pg_proc functions and procedures @@ -238,11 +243,6 @@ replication slot information - - pg_policy - table policies - - pg_seclabel security labels on database objects @@ -1939,16 +1939,6 @@ - - relrowsecurity - bool - - - True if table has row level security enabled; see - pg_policy catalog - - - relhassubclass bool @@ -1956,6 +1946,16 @@ True if table has (or once had) any inheritance children + + relrowsecurity + bool + + + True if table has row-level security enabled; see + pg_policy catalog + + + relispopulated bool @@ -4711,6 +4711,98 @@ + + <structname>pg_policy</structname> + + + pg_policy + + + + The catalog pg_policy 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 WITH CHECK option for queries that attempt to + add new records to the table. + + + + + <structname>pg_policy</structname> Columns + + + + + Name + Type + References + Description + + + + + + polname + name + + The name of the policy + + + + polrelid + oid + pg_class.oid + The table to which the policy applies + + + + polcmd + char + + The command type to which the policy is applied: + r for SELECT, + a for INSERT, + w for UPDATE, + d for DELETE, + or * for all + + + + polroles + oid[] + pg_authid.oid + The roles to which the policy is applied + + + + polqual + pg_node_tree + + The expression tree to be added to the security barrier qualifications for queries that use the table + + + + polwithcheck + pg_node_tree + + The expression tree to be added to the WITH CHECK qualifications for queries that attempt to add rows to the table + + + + +
+ + + + Policies stored in pg_policy are applied only when + pg_class.relrowsecurity is set for + their table. + + + + <structname>pg_proc</structname> @@ -5342,94 +5434,6 @@ - - <structname>pg_policy</structname> - - - pg_policy - - - - The catalog pg_policy 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. - - - - - <structname>pg_policy</structname> Columns - - - - - Name - Type - References - Description - - - - - - polname - name - - The name of the policy - - - - polrelid - oid - pg_class.oid - The table to which the policy belongs - - - - polcmd - char - - The command type to which the policy is applied. - - - - polroles - char - - The roles to which the policy is applied. - - - - polqual - pg_node_tree - - The expression tree to be added to the security barrier qualifications for queries which use the table. - - - - polwithcheck - pg_node_tree - - The expression tree to be added to the with check qualifications for queries which attempt to add rows to the table. - - - - -
- - - - pg_class.relrowsecurity - True if the table has row security enabled. Policies will not be applied - unless row security is enabled on the table. - - - - - <structname>pg_seclabel</structname> @@ -8166,7 +8170,7 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx The view pg_policies provides access to - useful information about each policy in the database. + useful information about each row-level security policy in the database. @@ -8197,34 +8201,34 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx policyname name - pg_class.relname + pg_policy.polname Name of policy - - cmd - text - - The command type to which the policy is applied. - roles name[] - The roles to which this policy applies. + The roles to which this policy applies + + + cmd + text + + The command type to which the policy is applied qual text The expression added to the security barrier qualifications for - queries which this policy applies to. + queries that this policy applies to with_check text - The expression added to the with check qualifications for - queries which attempt to add rows to this table. + The expression added to the WITH CHECK qualifications for + queries that attempt to add rows to this table diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 4bc874f5c2..6df6bf27d1 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -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 diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c index 9b79d88633..d98da0dd50 100644 --- a/src/backend/commands/policy.c +++ b/src/backend/commands/policy.c @@ -28,10 +28,10 @@ #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; + MemoryContext rscxt; + MemoryContext oldcxt = CurrentMemoryContext; + RowSecurityDesc * volatile rsdesc = NULL; - catalog = heap_open(PolicyRelationId, AccessShareLock); + /* + * 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); - ScanKeyInit(&skey, - Anum_pg_policy_polrelid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(RelationGetRelid(relation))); - - 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. diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c index 35790a948f..8f8e291fb8 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, diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 24c92e791f..1db4ba8410 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -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; diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 1e330f243a..c3ebb3a9b0 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -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, diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index 4cda07d87c..c44e447d06 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -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;", diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index bad9123c95..13c4376b8c 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 201412301 +#define CATALOG_VERSION_NO 201501241 #endif diff --git a/src/include/catalog/pg_policy.h b/src/include/catalog/pg_policy.h index ed0c6113e6..ae71f3f3a2 100644 --- a/src/include/catalog/pg_policy.h +++ b/src/include/catalog/pg_policy.h @@ -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 diff --git a/src/include/rewrite/rowsecurity.h b/src/include/rewrite/rowsecurity.h index aa1b45b1c9..240f987a3a 100644 --- a/src/include/rewrite/rowsecurity.h +++ b/src/include/rewrite/rowsecurity.h @@ -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 diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 80c3351291..7df5d2dce9 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -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