-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 07/27/2015 01:13 PM, Alvaro Herrera wrote:
> Hmm, these are not ACL objects, so conceptually it seems cleaner to
> use a different symbol for this. I think the catalog state and the
> error messages would be a bit confusing otherwise.
Ok -- done
> Isn't this leaking the previously allocated array? Not sure it's
> all that critical, but still. (I don't think you really need to
> call palloc at all here.)
Agreed -- I think the attached is a bit cleaner.
Any other comments or concerns?
- --
Joe Conway
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
iQIcBAEBAgAGBQJVts3+AAoJEDfy90M199hlzTIQAJ6LEOrEEhHEjsoVvCT6TUAu
pMQl/LWmb0s3/vF9o5VFTpVd21k0LxcggD+DdbxSagiw1WpLK5x67C9Lj5uuFn/d
3a95nFnQje3ciQJaAKS4XcGyx8+6rHPZqfBRC1rARbLuDHrwpKqmbKwvpQCud4xN
kD7OolYk5Gd3cId0xH0XBHuwLVJg4Bt/MAexrcHn+kXuQoF8V6iOjnmBI/BHvTQy
w4j4ov7DpWSAR1ZiCTCkF2ZuNd9TJ8FmAEtSDVrlWMxB9J+9PU5oTfD50jp62BTz
wycANVYmbpCyZ7DAAiqopt3IQFIiF/1bYwzWH3/M8RRMKJF8HNyE8KBPDyC/doO5
0c0poCugJI2wOhumLGJShizgSAS85vqijh2Uxpp2yQxdStMnADykzT4Nb5EnEJVE
i7OKy6w+2xyilfFGEWhHfS7uo5Y0zBorhpjgT9fRaqPBGoK4jYwZoO8w97SUd9aS
kNXOCfmFxvcDFSZIYZP77pOeJZR2dLCbr+X9bF1Fz5S4FVkgCXVOp9rmsqzgxoDh
lcpkDh9zPPezdczRkfq/qCf0lmzGg8apdqdr7+g8DxU+01OvPspEdpovQQN0HXlM
m5Kbny4KCWhAgBTyAsOFTEy6lK7u4KsHV1cee3bG+ev65czbQ14gGRMJc/Lhm6Gg
Apcn/UcF14vLWxYVo6lS
=pS6L
-----END PGP SIGNATURE-----
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 9096ee5..7781c56 100644
*** a/doc/src/sgml/catalogs.sgml
--- b/doc/src/sgml/catalogs.sgml
***************
*** 5793,5798 ****
--- 5793,5808 ----
</varlistentry>
<varlistentry>
+ <term><symbol>SHARED_DEPENDENCY_POLICY</> (<literal>r</>)</term>
+ <listitem>
+ <para>
+ The referenced object (which must be a role) is mentioned as the
+ target of a dependent policy object.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><symbol>SHARED_DEPENDENCY_PIN</> (<literal>p</>)</term>
<listitem>
<para>
diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c
index 34fe4e2..43076c9 100644
*** a/src/backend/catalog/pg_shdepend.c
--- b/src/backend/catalog/pg_shdepend.c
*************** storeObjectDescription(StringInfo descs,
*** 1083,1088 ****
--- 1083,1090 ----
appendStringInfo(descs, _("owner of %s"), objdesc);
else if (deptype == SHARED_DEPENDENCY_ACL)
appendStringInfo(descs, _("privileges for %s"), objdesc);
+ else if (deptype == SHARED_DEPENDENCY_POLICY)
+ appendStringInfo(descs, _("target of %s"), objdesc);
else
elog(ERROR, "unrecognized dependency type: %d",
(int) deptype);
diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 17b48d4..9544f75 100644
*** a/src/backend/commands/policy.c
--- b/src/backend/commands/policy.c
***************
*** 22,27 ****
--- 22,28 ----
#include "catalog/indexing.h"
#include "catalog/namespace.h"
#include "catalog/objectaccess.h"
+ #include "catalog/pg_authid.h"
#include "catalog/pg_policy.h"
#include "catalog/pg_type.h"
#include "commands/policy.h"
***************
*** 48,54 ****
static void RangeVarCallbackForPolicy(const RangeVar *rv,
Oid relid, Oid oldrelid, void *arg);
static char parse_policy_command(const char *cmd_name);
! static ArrayType *policy_role_list_to_array(List *roles);
/*
* Callback to RangeVarGetRelidExtended().
--- 49,55 ----
static void RangeVarCallbackForPolicy(const RangeVar *rv,
Oid relid, Oid oldrelid, void *arg);
static char parse_policy_command(const char *cmd_name);
! static Datum *policy_role_list_to_array(List *roles, int *num_roles);
/*
* Callback to RangeVarGetRelidExtended().
*************** parse_policy_command(const char *cmd_nam
*** 130,159 ****
/*
* policy_role_list_to_array
! * helper function to convert a list of RoleSpecs to an array of role ids.
*/
! static ArrayType *
! policy_role_list_to_array(List *roles)
{
! ArrayType *role_ids;
! Datum *temp_array;
ListCell *cell;
- int num_roles;
int i = 0;
/* Handle no roles being passed in as being for public */
if (roles == NIL)
{
! temp_array = (Datum *) palloc(sizeof(Datum));
! temp_array[0] = ObjectIdGetDatum(ACL_ID_PUBLIC);
! role_ids = construct_array(temp_array, 1, OIDOID, sizeof(Oid), true,
! 'i');
! return role_ids;
}
! num_roles = list_length(roles);
! temp_array = (Datum *) palloc(num_roles * sizeof(Datum));
foreach(cell, roles)
{
--- 131,158 ----
/*
* policy_role_list_to_array
! * helper function to convert a list of RoleSpecs to an array of
! * role id Datums.
*/
! static Datum *
! policy_role_list_to_array(List *roles, int *num_roles)
{
! Datum *role_oids;
ListCell *cell;
int i = 0;
/* Handle no roles being passed in as being for public */
if (roles == NIL)
{
! *num_roles = 1;
! role_oids = (Datum *) palloc(*num_roles * sizeof(Datum));
! role_oids[0] = ObjectIdGetDatum(ACL_ID_PUBLIC);
! return role_oids;
}
! *num_roles = list_length(roles);
! role_oids = (Datum *) palloc(*num_roles * sizeof(Datum));
foreach(cell, roles)
{
*************** policy_role_list_to_array(List *roles)
*** 164,187 ****
*/
if (spec->roletype == ROLESPEC_PUBLIC)
{
! if (num_roles != 1)
ereport(WARNING,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("ignoring roles specified other than public"),
errhint("All roles are members of the public role.")));
! temp_array[0] = ObjectIdGetDatum(ACL_ID_PUBLIC);
! num_roles = 1;
! break;
}
else
! temp_array[i++] =
ObjectIdGetDatum(get_rolespec_oid((Node *) spec, false));
}
! role_ids = construct_array(temp_array, num_roles, OIDOID, sizeof(Oid), true,
! 'i');
!
! return role_ids;
}
/*
--- 163,186 ----
*/
if (spec->roletype == ROLESPEC_PUBLIC)
{
! if (*num_roles != 1)
! {
ereport(WARNING,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("ignoring roles specified other than public"),
errhint("All roles are members of the public role.")));
! *num_roles = 1;
! }
! role_oids[0] = ObjectIdGetDatum(ACL_ID_PUBLIC);
!
! return role_oids;
}
else
! role_oids[i++] =
ObjectIdGetDatum(get_rolespec_oid((Node *) spec, false));
}
! return role_oids;
}
/*
*************** CreatePolicy(CreatePolicyStmt *stmt)
*** 463,468 ****
--- 462,469 ----
Relation target_table;
Oid table_id;
char polcmd;
+ Datum *role_oids;
+ int nitems = 0;
ArrayType *role_ids;
ParseState *qual_pstate;
ParseState *with_check_pstate;
*************** CreatePolicy(CreatePolicyStmt *stmt)
*** 476,481 ****
--- 477,483 ----
bool isnull[Natts_pg_policy];
ObjectAddress target;
ObjectAddress myself;
+ int i;
/* Parse command */
polcmd = parse_policy_command(stmt->cmd);
*************** CreatePolicy(CreatePolicyStmt *stmt)
*** 498,506 ****
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("only WITH CHECK expression allowed for INSERT")));
-
/* Collect role ids */
! role_ids = policy_role_list_to_array(stmt->roles);
/* Parse the supplied clause */
qual_pstate = make_parsestate(NULL);
--- 500,509 ----
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("only WITH CHECK expression allowed for INSERT")));
/* Collect role ids */
! role_oids = policy_role_list_to_array(stmt->roles, &nitems);
! role_ids = construct_array(role_oids, nitems, OIDOID,
! sizeof(Oid), true, 'i');
/* Parse the supplied clause */
qual_pstate = make_parsestate(NULL);
*************** CreatePolicy(CreatePolicyStmt *stmt)
*** 614,619 ****
--- 617,634 ----
recordDependencyOnExpr(&myself, with_check_qual,
with_check_pstate->p_rtable, DEPENDENCY_NORMAL);
+ /* Register role dependencies */
+ target.classId = AuthIdRelationId;
+ target.objectSubId = 0;
+ for (i = 0; i < nitems; i++)
+ {
+ target.objectId = DatumGetObjectId(role_oids[i]);
+ /* no dependency if public */
+ if (target.objectId != ACL_ID_PUBLIC)
+ recordSharedDependencyOn(&myself, &target,
+ SHARED_DEPENDENCY_POLICY);
+ }
+
/* Invalidate Relation Cache */
CacheInvalidateRelcache(target_table);
*************** AlterPolicy(AlterPolicyStmt *stmt)
*** 641,646 ****
--- 656,663 ----
Oid policy_id;
Relation target_table;
Oid table_id;
+ Datum *role_oids;
+ int nitems = 0;
ArrayType *role_ids = NULL;
List *qual_parse_rtable = NIL;
List *with_check_parse_rtable = NIL;
*************** AlterPolicy(AlterPolicyStmt *stmt)
*** 658,667 ****
Datum cmd_datum;
char polcmd;
bool polcmd_isnull;
/* Parse role_ids */
if (stmt->roles != NULL)
! role_ids = policy_role_list_to_array(stmt->roles);
/* Get id of table. Also handles permissions checks. */
table_id = RangeVarGetRelidExtended(stmt->table, AccessExclusiveLock,
--- 675,689 ----
Datum cmd_datum;
char polcmd;
bool polcmd_isnull;
+ int i;
/* Parse role_ids */
if (stmt->roles != NULL)
! {
! role_oids = policy_role_list_to_array(stmt->roles, &nitems);
! role_ids = construct_array(role_oids, nitems, OIDOID,
! sizeof(Oid), true, 'i');
! }
/* Get id of table. Also handles permissions checks. */
table_id = RangeVarGetRelidExtended(stmt->table, AccessExclusiveLock,
*************** AlterPolicy(AlterPolicyStmt *stmt)
*** 825,830 ****
--- 847,865 ----
recordDependencyOnExpr(&myself, with_check_qual, with_check_parse_rtable,
DEPENDENCY_NORMAL);
+ /* Register role dependencies */
+ deleteSharedDependencyRecordsFor(PolicyRelationId, policy_id, 0);
+ target.classId = AuthIdRelationId;
+ target.objectSubId = 0;
+ for (i = 0; i < nitems; i++)
+ {
+ target.objectId = DatumGetObjectId(role_oids[i]);
+ /* no dependency if public */
+ if (target.objectId != ACL_ID_PUBLIC)
+ recordSharedDependencyOn(&myself, &target,
+ SHARED_DEPENDENCY_POLICY);
+ }
+
heap_freetuple(new_tuple);
/* Invalidate Relation Cache */
diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index aa3f3d9..fbcf904 100644
*** a/src/include/catalog/dependency.h
--- b/src/include/catalog/dependency.h
*************** typedef enum DependencyType
*** 96,101 ****
--- 96,105 ----
* created for the owner of an object; hence two objects may be linked by
* one or the other, but not both, of these dependency types.)
*
+ * (d) a SHARED_DEPENDENCY_POLICY entry means that the referenced object is
+ * a role mentioned in a policy object. The referenced object must be a
+ * pg_authid entry.
+ *
* SHARED_DEPENDENCY_INVALID is a value used as a parameter in internal
* routines, and is not valid in the catalog itself.
*/
*************** typedef enum SharedDependencyType
*** 104,109 ****
--- 108,114 ----
SHARED_DEPENDENCY_PIN = 'p',
SHARED_DEPENDENCY_OWNER = 'o',
SHARED_DEPENDENCY_ACL = 'a',
+ SHARED_DEPENDENCY_POLICY = 'r',
SHARED_DEPENDENCY_INVALID = 0
} SharedDependencyType;
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index e7c242c..0983d5b 100644
*** a/src/test/regress/expected/rowsecurity.out
--- b/src/test/regress/expected/rowsecurity.out
*************** SELECT * FROM coll_t;
*** 2860,2865 ****
--- 2860,2920 ----
ROLLBACK;
--
+ -- Shared Object Dependencies
+ --
+ RESET SESSION AUTHORIZATION;
+ BEGIN;
+ CREATE ROLE alice;
+ CREATE ROLE bob;
+ CREATE TABLE tbl1 (c) AS VALUES ('bar'::text);
+ GRANT SELECT ON TABLE tbl1 TO alice;
+ CREATE POLICY P ON tbl1 TO alice, bob USING (true);
+ SELECT refclassid::regclass, deptype
+ FROM pg_depend
+ WHERE classid = 'pg_policy'::regclass
+ AND refobjid = 'tbl1'::regclass;
+ refclassid | deptype
+ ------------+---------
+ pg_class | a
+ (1 row)
+
+ SELECT refclassid::regclass, deptype
+ FROM pg_shdepend
+ WHERE classid = 'pg_policy'::regclass
+ AND refobjid IN ('alice'::regrole, 'bob'::regrole);
+ refclassid | deptype
+ ------------+---------
+ pg_authid | r
+ pg_authid | r
+ (2 rows)
+
+ SAVEPOINT q;
+ DROP ROLE alice; --fails due to dependency on POLICY p
+ ERROR: role "alice" cannot be dropped because some objects depend on it
+ DETAIL: target of policy p on table tbl1
+ privileges for table tbl1
+ ROLLBACK TO q;
+ ALTER POLICY p ON tbl1 TO bob USING (true);
+ SAVEPOINT q;
+ DROP ROLE alice; --fails due to dependency on GRANT SELECT
+ ERROR: role "alice" cannot be dropped because some objects depend on it
+ DETAIL: privileges for table tbl1
+ ROLLBACK TO q;
+ REVOKE ALL ON TABLE tbl1 FROM alice;
+ SAVEPOINT q;
+ DROP ROLE alice; --succeeds
+ ROLLBACK TO q;
+ SAVEPOINT q;
+ DROP ROLE bob; --fails due to dependency on POLICY p
+ ERROR: role "bob" cannot be dropped because some objects depend on it
+ DETAIL: target of policy p on table tbl1
+ ROLLBACK TO q;
+ DROP POLICY p ON tbl1;
+ SAVEPOINT q;
+ DROP ROLE bob; -- succeeds
+ ROLLBACK TO q;
+ ROLLBACK; -- cleanup
+ --
-- Clean up objects
--
RESET SESSION AUTHORIZATION;
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index e86f814..a074edd 100644
*** a/src/test/regress/sql/rowsecurity.sql
--- b/src/test/regress/sql/rowsecurity.sql
*************** SELECT * FROM coll_t;
*** 1153,1158 ****
--- 1153,1202 ----
ROLLBACK;
--
+ -- Shared Object Dependencies
+ --
+ RESET SESSION AUTHORIZATION;
+ BEGIN;
+ CREATE ROLE alice;
+ CREATE ROLE bob;
+ CREATE TABLE tbl1 (c) AS VALUES ('bar'::text);
+ GRANT SELECT ON TABLE tbl1 TO alice;
+ CREATE POLICY P ON tbl1 TO alice, bob USING (true);
+ SELECT refclassid::regclass, deptype
+ FROM pg_depend
+ WHERE classid = 'pg_policy'::regclass
+ AND refobjid = 'tbl1'::regclass;
+ SELECT refclassid::regclass, deptype
+ FROM pg_shdepend
+ WHERE classid = 'pg_policy'::regclass
+ AND refobjid IN ('alice'::regrole, 'bob'::regrole);
+
+ SAVEPOINT q;
+ DROP ROLE alice; --fails due to dependency on POLICY p
+ ROLLBACK TO q;
+
+ ALTER POLICY p ON tbl1 TO bob USING (true);
+ SAVEPOINT q;
+ DROP ROLE alice; --fails due to dependency on GRANT SELECT
+ ROLLBACK TO q;
+
+ REVOKE ALL ON TABLE tbl1 FROM alice;
+ SAVEPOINT q;
+ DROP ROLE alice; --succeeds
+ ROLLBACK TO q;
+
+ SAVEPOINT q;
+ DROP ROLE bob; --fails due to dependency on POLICY p
+ ROLLBACK TO q;
+
+ DROP POLICY p ON tbl1;
+ SAVEPOINT q;
+ DROP ROLE bob; -- succeeds
+ ROLLBACK TO q;
+
+ ROLLBACK; -- cleanup
+
+ --
-- Clean up objects
--
RESET SESSION AUTHORIZATION;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
