diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c index 5bd5f8c9968f..b4a16d0ff2d4 100644 --- a/src/backend/commands/policy.c +++ b/src/backend/commands/policy.c @@ -26,6 +26,7 @@ #include "catalog/pg_authid.h" #include "catalog/pg_policy.h" #include "catalog/pg_type.h" +#include "commands/comment.h" #include "commands/policy.h" #include "miscadmin.h" #include "nodes/pg_list.h" @@ -33,6 +34,7 @@ #include "parser/parse_collate.h" #include "parser/parse_node.h" #include "parser/parse_relation.h" +#include "parser/parse_utilcmd.h" #include "rewrite/rewriteManip.h" #include "rewrite/rowsecurity.h" #include "utils/acl.h" @@ -564,9 +566,10 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id) * handles the execution of the CREATE POLICY command. * * stmt - the CreatePolicyStmt that describes the policy to create. + * queryString - the source text of the CREATE POLICY command. */ ObjectAddress -CreatePolicy(CreatePolicyStmt *stmt) +CreatePolicy(CreatePolicyStmt *stmt, const char *queryString) { Relation pg_policy_rel; Oid policy_id; @@ -576,9 +579,7 @@ CreatePolicy(CreatePolicyStmt *stmt) Datum *role_oids; int nitems = 0; ArrayType *role_ids; - ParseState *qual_pstate; - ParseState *with_check_pstate; - ParseNamespaceItem *nsitem; + List *rtable; Node *qual; Node *with_check_qual; ScanKeyData skey[2]; @@ -615,48 +616,37 @@ CreatePolicy(CreatePolicyStmt *stmt) role_oids = policy_role_list_to_array(stmt->roles, &nitems); role_ids = construct_array_builtin(role_oids, nitems, OIDOID); - /* Parse the supplied clause */ - qual_pstate = make_parsestate(NULL); - with_check_pstate = make_parsestate(NULL); - /* zero-clear */ memset(values, 0, sizeof(values)); memset(isnull, 0, sizeof(isnull)); - /* Get id of table. Also handles permissions checks. */ - table_id = RangeVarGetRelidExtended(stmt->table, AccessExclusiveLock, - 0, - RangeVarCallbackForPolicy, - stmt); - - /* Open target_table to build quals. No additional lock is necessary. */ - target_table = relation_open(table_id, NoLock); - - /* Add for the regular security quals */ - nsitem = addRangeTableEntryForRelation(qual_pstate, target_table, - AccessShareLock, - NULL, false, false); - addNSItemToQuery(qual_pstate, nsitem, false, true, true); + if (stmt->rte != NULL) + table_id = stmt->rte->relid; + else + { + /* Get id of table. Also handles permissions checks. */ + table_id = RangeVarGetRelidExtended(stmt->table, AccessExclusiveLock, + 0, + RangeVarCallbackForPolicy, + stmt); + + /* Create a range table entry. */ + stmt->rte = makeNode(RangeTblEntry); + stmt->rte->rtekind = RTE_RELATION; + stmt->rte->relid = table_id; + stmt->rte->rellockmode = AccessExclusiveLock; + } - /* Add for the with-check quals */ - nsitem = addRangeTableEntryForRelation(with_check_pstate, target_table, - AccessShareLock, - NULL, false, false); - addNSItemToQuery(with_check_pstate, nsitem, false, true, true); + rtable = list_make1(stmt->rte); - qual = transformWhereClause(qual_pstate, - stmt->qual, - EXPR_KIND_POLICY, - "POLICY"); + /* do parse analysis for qual and check qual */ + transformPolicyStmt(stmt, NULL); - with_check_qual = transformWhereClause(with_check_pstate, - stmt->with_check, - EXPR_KIND_POLICY, - "POLICY"); + qual = stmt->qual; + with_check_qual = stmt->with_check; - /* Fix up collation information */ - assign_expr_collations(qual_pstate, qual); - assign_expr_collations(with_check_pstate, with_check_qual); + /* Open target_table, No additional lock is necessary. */ + target_table = relation_open(table_id, NoLock); /* Open pg_policy catalog */ pg_policy_rel = table_open(PolicyRelationId, RowExclusiveLock); @@ -724,11 +714,11 @@ CreatePolicy(CreatePolicyStmt *stmt) recordDependencyOn(&myself, &target, DEPENDENCY_AUTO); - recordDependencyOnExpr(&myself, qual, qual_pstate->p_rtable, + recordDependencyOnExpr(&myself, qual, rtable, DEPENDENCY_NORMAL); recordDependencyOnExpr(&myself, with_check_qual, - with_check_pstate->p_rtable, DEPENDENCY_NORMAL); + rtable, DEPENDENCY_NORMAL); /* Register role dependencies */ target.classId = AuthIdRelationId; @@ -749,12 +739,15 @@ CreatePolicy(CreatePolicyStmt *stmt) /* Clean up. */ heap_freetuple(policy_tuple); - free_parsestate(qual_pstate); - free_parsestate(with_check_pstate); systable_endscan(sscan); relation_close(target_table, NoLock); table_close(pg_policy_rel, RowExclusiveLock); + /* Add any requested comment */ + if (stmt->polcomment != NULL) + CreateComments(policy_id, PolicyRelationId, 0, + stmt->polcomment); + return myself; } @@ -1277,3 +1270,65 @@ relation_has_policies(Relation rel) return ret; } + +/* + * PolicyGetRelation: given a policy object's OID, get the OID of + * the relation it is defined on. + */ +Oid +PolicyGetRelation(Oid policyId) +{ + Relation pg_policy_rel; + ScanKeyData skey[1]; + SysScanDesc sscan; + HeapTuple tuple; + Oid relid; + + /* Fetch the policy's table OID the hard way */ + pg_policy_rel = table_open(PolicyRelationId, AccessShareLock); + ScanKeyInit(&skey[0], + Anum_pg_policy_oid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(policyId)); + sscan = systable_beginscan(pg_policy_rel, PolicyOidIndexId, true, + NULL, 1, skey); + tuple = systable_getnext(sscan); + + if (HeapTupleIsValid(tuple)) + relid = ((Form_pg_policy) GETSTRUCT(tuple))->polrelid; + else + { + relid = InvalidOid; /* shouldn't happen */ + elog(ERROR, "could not find tuple for policy %u", policyId); + } + systable_endscan(sscan); + + table_close(pg_policy_rel, AccessShareLock); + + return relid; +} + +/* + * get_policy_applied_command + * + * Convert pg_policy.polcmd char representation to their full command strings. + */ +char * +get_policy_applied_command(char polcmd) +{ + if (polcmd == '*') + return pstrdup("all"); + else if (polcmd == ACL_SELECT_CHR) + return pstrdup("select"); + else if (polcmd == ACL_INSERT_CHR) + return pstrdup("insert"); + else if (polcmd == ACL_UPDATE_CHR) + return pstrdup("update"); + else if (polcmd == ACL_DELETE_CHR) + return pstrdup("delete"); + else + { + elog(ERROR, "unrecognized policy command"); + return NULL; + } +} diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 1d9565b09fcd..c141ba0ed752 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -60,6 +60,7 @@ #include "commands/comment.h" #include "commands/defrem.h" #include "commands/event_trigger.h" +#include "commands/policy.h" #include "commands/sequence.h" #include "commands/tablecmds.h" #include "commands/tablespace.h" @@ -208,6 +209,8 @@ typedef struct AlteredTableInfo char *clusterOnIndex; /* index to use for CLUSTER */ List *changedStatisticsOids; /* OIDs of statistics to rebuild */ List *changedStatisticsDefs; /* string definitions of same */ + List *changedPolicyOids; /* OIDs of policy to rebuild */ + List *changedPolicyDefs; /* string definitions of same */ } AlteredTableInfo; /* Struct describing one new constraint to check in Phase 3 scan */ @@ -546,6 +549,8 @@ static ObjectAddress ATExecAddIndex(AlteredTableInfo *tab, Relation rel, IndexStmt *stmt, bool is_rebuild, LOCKMODE lockmode); static ObjectAddress ATExecAddStatistics(AlteredTableInfo *tab, Relation rel, CreateStatsStmt *stmt, bool is_rebuild, LOCKMODE lockmode); +static ObjectAddress ATExecAddPolicies(AlteredTableInfo *tab, Relation rel, + CreatePolicyStmt *stmt, bool is_rebuild, LOCKMODE lockmode); static ObjectAddress ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, Constraint *newConstraint, bool recurse, bool is_readd, @@ -648,9 +653,12 @@ static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, AlterTableCmd *cmd, LOCKMODE lockmode); static void RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, Relation rel, AttrNumber attnum, const char *colName); +static void CheckDepentPolicies(AlteredTableInfo *tab, AlterTableType subtype, + Relation rel, AttrNumber attnum, const char *colName); static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab); static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab); static void RememberStatisticsForRebuilding(Oid stxoid, AlteredTableInfo *tab); +static void RememberPolicyForRebuilding(Oid policyId, AlteredTableInfo *tab); static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode); static void ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, @@ -5464,6 +5472,10 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, address = ATExecAddStatistics(tab, rel, (CreateStatsStmt *) cmd->def, true, lockmode); break; + case AT_ReAddPolicies: /* ADD POLICIES */ + address = ATExecAddPolicies(tab, rel, castNode(CreatePolicyStmt, cmd->def), + true, lockmode); + break; case AT_AddConstraint: /* ADD CONSTRAINT */ /* Transform the command only during initial examination */ if (cur_pass == AT_PASS_ADD_CONSTR) @@ -6751,6 +6763,8 @@ alter_table_type_to_string(AlterTableType cmdtype) return "ALTER COLUMN ... DROP IDENTITY"; case AT_ReAddStatistics: return NULL; /* not real grammar */ + case AT_ReAddPolicies: + return NULL; /* not real grammar */ } return NULL; @@ -9723,6 +9737,29 @@ ATExecAddStatistics(AlteredTableInfo *tab, Relation rel, return address; } +/* + * ALTER TABLE ADD POLICIES + * + * This is no such command in the grammar, but we use this internally to add + * AT_ReAddPolicies subcommands to rebuild policy after a table + * column type change. + */ +static ObjectAddress +ATExecAddPolicies(AlteredTableInfo *tab, Relation rel, + CreatePolicyStmt *stmt, bool is_rebuild, LOCKMODE lockmode) +{ + ObjectAddress address; + + Assert(IsA(stmt, CreatePolicyStmt)); + + /* The CreatePolicyStmt has already been through transformPolicyStmt */ + Assert(stmt->transformed); + + address = CreatePolicy(stmt, NULL); + + return address; +} + /* * ALTER TABLE ADD CONSTRAINT USING INDEX * @@ -14868,6 +14905,13 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, */ RememberAllDependentForRebuilding(tab, AT_AlterColumnType, rel, attnum, colName); + /* + * Check that all policies does not contain subquery before adding its + * definition to tab->changedPolicyDefs, as + * RememberAllDependentForRebuilding only collects the policy OID. + */ + CheckDepentPolicies(tab, AT_AlterColumnType, rel, attnum, colName); + /* * Now scan for dependencies of this column on other things. The only * things we should find are the dependency on the column datatype and @@ -15062,6 +15106,109 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, return address; } +/* + * Subroutine for ATExecAlterColumnType. + * + * ALTER COLUMN SET DATA TYPE cannot cope with policies that contain subqueries + * in the USING or WITH CHECK qual; otherwise, repeated name lookup issues may + * occur. + */ +static void +CheckDepentPolicies(AlteredTableInfo *tab, AlterTableType subtype, + Relation rel, AttrNumber attnum, const char *colName) +{ + Relation pg_policy; + ScanKeyData skey[1]; + SysScanDesc sscan; + HeapTuple tuple; + Form_pg_policy form_policy; + Node *qual; + Node *with_check_qual; + bool hassublinks = false; + + pg_policy = table_open(PolicyRelationId, AccessShareLock); + + foreach_oid(poloid, tab->changedPolicyOids) + { + Datum datum; + bool isnull; + char *str_value; + + /* Fetch the policy's table OID the hard way */ + ScanKeyInit(&skey[0], + Anum_pg_policy_oid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(poloid)); + sscan = systable_beginscan(pg_policy, PolicyOidIndexId, true, + NULL, 1, skey); + tuple = systable_getnext(sscan); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "could not find tuple for pg_policy %u", poloid); + + form_policy = (Form_pg_policy) GETSTRUCT(tuple); + + /* Get policy qual */ + datum = heap_getattr(tuple, Anum_pg_policy_polqual, + RelationGetDescr(pg_policy), &isnull); + if (!isnull) + { + str_value = TextDatumGetCString(datum); + qual = stringToNode(str_value); + + if (checkExprHasSubLink(qual)) + hassublinks = true; + + pfree(str_value); + } + + if (hassublinks) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot alter type of a column used by a policy definition when the policy contains subquery"), + errdetail("policy %s on table %s depends on column \"%s\"", + NameStr(form_policy->polname), + RelationGetRelationName(rel), + colName)); + + /* Get WITH CHECK qual */ + datum = heap_getattr(tuple, Anum_pg_policy_polwithcheck, + RelationGetDescr(pg_policy), &isnull); + if (!isnull) + { + str_value = TextDatumGetCString(datum); + with_check_qual = stringToNode(str_value); + + if (checkExprHasSubLink(with_check_qual)) + hassublinks = true; + + pfree(str_value); + } + + if (hassublinks) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot alter type of a column used by a policy definition when the policy contains subquery"), + errdetail("policy %s on table %s depends on column \"%s\"", + NameStr(form_policy->polname), + RelationGetRelationName(rel), + colName)); + + Assert(form_policy->polrelid == RelationGetRelid(rel)); + + systable_endscan(sscan); + } + + table_close(pg_policy, AccessShareLock); + + foreach_oid(poloid, tab->changedPolicyOids) + { + /* OK, capture the policies's existing definition string */ + char *defstring = pg_get_policy_def_command(poloid); + + tab->changedPolicyDefs = lappend(tab->changedPolicyDefs, defstring); + } +} + /* * Subroutine for ATExecAlterColumnType and ATExecSetExpression: Find everything * that depends on the column (constraints, indexes, etc), and record enough @@ -15196,22 +15343,8 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, break; case PolicyRelationId: - - /* - * A policy can depend on a column because the column is - * specified in the policy's USING or WITH CHECK qual - * expressions. It might be possible to rewrite and recheck - * the policy expression, but punt for now. It's certainly - * easy enough to remove and recreate the policy; still, FIXME - * someday. - */ if (subtype == AT_AlterColumnType) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot alter type of a column used in a policy definition"), - errdetail("%s depends on column \"%s\"", - getObjectDescription(&foundObject, false), - colName))); + RememberPolicyForRebuilding(foundObject.objectId, tab); break; case AttrDefaultRelationId: @@ -15453,6 +15586,27 @@ RememberStatisticsForRebuilding(Oid stxoid, AlteredTableInfo *tab) } } +/* + * Subroutine for ATExecAlterColumnType: remember that a policy object needs to + * be rebuilt (which we might already know). + */ +static void +RememberPolicyForRebuilding(Oid policyId, AlteredTableInfo *tab) +{ + /* + * This de-duplication check is critical for two independent reasons: we + * mustn't try to recreate the same policy twice, and if a policy depends + * on more than one column whose type is to be altered, we must capture + * its definition string before applying any of the column type changes. + * ruleutils.c will get confused if we ask again later. + * + * changedPolicyDefs will be collected later on CheckDepentPolicies. + */ + if (!list_member_oid(tab->changedPolicyOids, policyId)) + tab->changedPolicyOids = lappend_oid(tab->changedPolicyOids, + policyId); +} + /* * Cleanup after we've finished all the ALTER TYPE or SET EXPRESSION * operations for a particular relation. We have to drop and recreate all the @@ -15597,6 +15751,35 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) add_exact_object_address(&obj, objects); } + /* add dependencies for new policies */ + forboth(oid_item, tab->changedPolicyOids, + def_item, tab->changedPolicyDefs) + { + Oid oldId = lfirst_oid(oid_item); + Oid relid; + + relid = PolicyGetRelation(oldId); + + /* + * As above, make sure we have lock on the relations if it's not the + * same table. However, we take AccessExclusiveLock here, aligning + * with the lock level used in CreatePolicy and RemovePolicyById. + * + * CAUTION: this should be done after all cases that grab + * AccessExclusiveLock, else we risk causing deadlock due to needing + * to promote our table lock. + */ + if (relid != tab->relid) + LockRelationOid(relid, ShareUpdateExclusiveLock); + + ATPostAlterTypeParse(oldId, tab->relid, InvalidOid, + (char *) lfirst(def_item), + wqueue, lockmode, tab->rewrite); + + ObjectAddressSet(obj, PolicyRelationId, oldId); + add_exact_object_address(&obj, objects); + } + /* * Queue up command to restore replica identity index marking */ @@ -15645,8 +15828,8 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) } /* - * Parse the previously-saved definition string for a constraint, index or - * statistics object against the newly-established column data type(s), and + * Parse the previously-saved definition string for a constraint, index, + * statistics or policy object against the newly-established column data type(s), and * queue up the resulting command parsetrees for execution. * * This might fail if, for example, you have a WHERE clause that uses an @@ -15698,6 +15881,21 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, transformStatsStmt(oldRelId, (CreateStatsStmt *) stmt, cmd)); + else if (IsA(stmt, CreatePolicyStmt)) + { + RangeTblEntry *rte; + CreatePolicyStmt *polstmt = castNode(CreatePolicyStmt, stmt); + + rte = makeNode(RangeTblEntry); + rte->rtekind = RTE_RELATION; + rte->relid = oldRelId; + rte->rellockmode = AccessExclusiveLock; + polstmt->rte = rte; + + querytree_list = lappend(querytree_list, + transformPolicyStmt(polstmt, + cmd)); + } else querytree_list = lappend(querytree_list, stmt); } @@ -15848,6 +16046,20 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, tab->subcmds[AT_PASS_MISC] = lappend(tab->subcmds[AT_PASS_MISC], newcmd); } + else if (IsA(stm, CreatePolicyStmt)) + { + CreatePolicyStmt *stmt = (CreatePolicyStmt *) stm; + AlterTableCmd *newcmd; + + /* keep the policy's object's comment */ + stmt->polcomment = GetComment(oldId, PolicyRelationId, 0); + + newcmd = makeNode(AlterTableCmd); + newcmd->subtype = AT_ReAddPolicies; + newcmd->def = (Node *) stmt; + tab->subcmds[AT_PASS_MISC] = + lappend(tab->subcmds[AT_PASS_MISC], newcmd); + } else elog(ERROR, "unexpected statement type: %d", (int) nodeTag(stm)); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 28f4e11e30ff..0ec86872f167 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -6023,11 +6023,14 @@ CreatePolicyStmt: n->policy_name = $3; n->table = $5; + n->rte = NULL; n->permissive = $6; n->cmd_name = $7; n->roles = $8; n->qual = $9; n->with_check = $10; + n->transformed = false; + n->polcomment = NULL; $$ = (Node *) n; } ; diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 2b7b084f2162..37f140d691de 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -3208,6 +3208,74 @@ transformStatsStmt(Oid relid, CreateStatsStmt *stmt, const char *queryString) return stmt; } +/* + * transformPolicyStmt - parse analysis for CREATE POLICY + * + * To avoid race conditions, it's important that this function relies only on + * the relid on stmt->rte (and not on stmt->table) to determine the target + * relation. + */ +CreatePolicyStmt * +transformPolicyStmt(CreatePolicyStmt *stmt, const char *queryString) +{ + Relation target_table; + ParseNamespaceItem *nsitem; + ParseState *qual_pstate; + ParseState *with_check_pstate; + + /* Nothing to do if statement already transformed. */ + if (stmt->transformed) + { + Assert(stmt->rte != NULL); + return stmt; + } + + target_table = relation_open(stmt->rte->relid, NoLock); + + /* Parse the supplied clause */ + qual_pstate = make_parsestate(NULL); + with_check_pstate = make_parsestate(NULL); + + qual_pstate->p_sourcetext = queryString; + with_check_pstate->p_sourcetext = queryString; + + /* Add for the regular security quals */ + nsitem = addRangeTableEntryForRelation(qual_pstate, target_table, + AccessShareLock, + NULL, false, false); + addNSItemToQuery(qual_pstate, nsitem, false, true, true); + + /* Add for the with-check quals */ + nsitem = addRangeTableEntryForRelation(with_check_pstate, target_table, + AccessShareLock, + NULL, false, false); + addNSItemToQuery(with_check_pstate, nsitem, false, true, true); + + stmt->qual = transformWhereClause(qual_pstate, + stmt->qual, + EXPR_KIND_POLICY, + "POLICY"); + + stmt->with_check = transformWhereClause(with_check_pstate, + stmt->with_check, + EXPR_KIND_POLICY, + "POLICY"); + + /* Fix up collation information */ + assign_expr_collations(qual_pstate, stmt->qual); + assign_expr_collations(with_check_pstate, stmt->with_check); + + free_parsestate(qual_pstate); + free_parsestate(with_check_pstate); + + relation_close(target_table, NoLock); + + /* Mark statement as successfully transformed */ + stmt->transformed = true; + + return stmt; +} + /* * transformRuleStmt - diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index d18a3a60a467..de9dc267ef24 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -1826,7 +1826,7 @@ ProcessUtilitySlow(ParseState *pstate, break; case T_CreatePolicyStmt: /* CREATE POLICY */ - address = CreatePolicy((CreatePolicyStmt *) parsetree); + address = CreatePolicy(castNode(CreatePolicyStmt, parsetree), queryString); break; case T_AlterPolicyStmt: /* ALTER POLICY */ diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 9f85eb86da1c..a30ccf40407a 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -33,11 +33,13 @@ #include "catalog/pg_opclass.h" #include "catalog/pg_operator.h" #include "catalog/pg_partitioned_table.h" +#include "catalog/pg_policy.h" #include "catalog/pg_proc.h" #include "catalog/pg_statistic_ext.h" #include "catalog/pg_trigger.h" #include "catalog/pg_type.h" #include "commands/defrem.h" +#include "commands/policy.h" #include "commands/tablespace.h" #include "common/keywords.h" #include "executor/spi.h" @@ -367,6 +369,7 @@ static char *pg_get_partkeydef_worker(Oid relid, int prettyFlags, bool attrsOnly, bool missing_ok); static char *pg_get_constraintdef_worker(Oid constraintId, bool fullCommand, int prettyFlags, bool missing_ok); +static char *pg_get_policydef_worker(Oid policyId, int prettyFlags, bool missing_ok); static text *pg_get_expr_worker(text *expr, Oid relid, int prettyFlags); static int print_function_arguments(StringInfo buf, HeapTuple proctup, bool print_table_args, bool print_defaults); @@ -2610,6 +2613,166 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand, return buf.data; } +/* + * Internal version that returns a full CREATE POLICY command + */ +char * +pg_get_policy_def_command(Oid policyId) +{ + int prettyFlags; + + prettyFlags = PRETTYFLAG_INDENT; + + return pg_get_policydef_worker(policyId, prettyFlags, false); +} + +static char * +pg_get_policydef_worker(Oid policyId, int prettyFlags, bool missing_ok) +{ + HeapTuple tup; + Form_pg_policy policy_form; + StringInfoData buf; + SysScanDesc scandesc; + ScanKeyData scankey[1]; + Snapshot snapshot = RegisterSnapshot(GetTransactionSnapshot()); + Relation relation = table_open(PolicyRelationId, AccessShareLock); + ArrayType *policy_roles; + Datum roles_datum; + Datum datum; + bool isnull; + Oid *roles; + int num_roles; + List *context = NIL; + char *str_value; + char *exprsrc; + char *rolString; + char *policy_command; + Node *expr; + + ScanKeyInit(&scankey[0], + Anum_pg_policy_oid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(policyId)); + + scandesc = systable_beginscan(relation, + PolicyOidIndexId, + true, + snapshot, + 1, + scankey); + tup = systable_getnext(scandesc); + + UnregisterSnapshot(snapshot); + + if (!HeapTupleIsValid(tup)) + { + if (missing_ok) + { + systable_endscan(scandesc); + table_close(relation, AccessShareLock); + return NULL; + } + elog(ERROR, "could not find tuple for policy %u", policyId); + } + + policy_form = (Form_pg_policy) GETSTRUCT(tup); + context = deparse_context_for(get_relation_name(policy_form->polrelid), + policy_form->polrelid); + + initStringInfo(&buf); + if (OidIsValid(policy_form->oid)) + appendStringInfo(&buf, "CREATE POLICY %s ON %s ", + quote_identifier(NameStr(policy_form->polname)), + generate_qualified_relation_name(policy_form->polrelid)); + else + elog(ERROR, "invalid policy: %u", policyId); + + /* Get policy type, permissive or restrictive */ + if (policy_form->polpermissive) + appendStringInfoString(&buf, "AS PERMISSIVE "); + else + appendStringInfoString(&buf, "AS RESTRICTIVE "); + + appendStringInfoString(&buf, "FOR "); + + /* Get policy applied command type */ + policy_command = get_policy_applied_command(policy_form->polcmd); + if (strcmp(policy_command, "all") == 0) + appendStringInfoString(&buf, "ALL "); + else if (strcmp(policy_command, "select") == 0) + appendStringInfoString(&buf, "SELECT "); + else if (strcmp(policy_command, "insert") == 0) + appendStringInfoString(&buf, "INSERT "); + else if (strcmp(policy_command, "update") == 0) + appendStringInfoString(&buf, "UPDATE "); + else if (strcmp(policy_command, "delete") == 0) + appendStringInfoString(&buf, "DELETE "); + else + elog(ERROR, "invalid command type %c", policy_form->polcmd); + + appendStringInfoString(&buf, "TO "); + + /* Get the current set of roles */ + datum = heap_getattr(tup, + Anum_pg_policy_polroles, + RelationGetDescr(relation), + &isnull); + Assert(!isnull); + + policy_roles = DatumGetArrayTypePCopy(datum); + roles = (Oid *) ARR_DATA_PTR(policy_roles); + num_roles = ARR_DIMS(policy_roles)[0]; + + for (int i = 0; i < num_roles; i++) + { + if (i > 0) + appendStringInfoString(&buf, ", "); + + if (OidIsValid(roles[i])) + { + datum = ObjectIdGetDatum(roles[i]); + roles_datum = DirectFunctionCall1(pg_get_userbyid, datum); + rolString = DatumGetCString(DirectFunctionCall1(nameout, roles_datum)); + appendStringInfo(&buf, "%s", rolString); + } + else + appendStringInfoString(&buf, "PUBLIC"); + } + + /* Get policy qual */ + datum = heap_getattr(tup, Anum_pg_policy_polqual, + RelationGetDescr(relation), &isnull); + if (!isnull) + { + str_value = TextDatumGetCString(datum); + expr = stringToNode(str_value); + exprsrc = deparse_expression_pretty(expr, context, false, false, + prettyFlags, 0); + appendStringInfo(&buf, " USING (%s) ", exprsrc); + pfree(str_value); + } + + /* Get WITH CHECK qual */ + datum = heap_getattr(tup, Anum_pg_policy_polwithcheck, + RelationGetDescr(relation), &isnull); + if (!isnull) + { + str_value = TextDatumGetCString(datum); + expr = stringToNode(str_value); + pfree(str_value); + + exprsrc = deparse_expression_pretty(expr, context, false, false, + prettyFlags, 0); + appendStringInfo(&buf, "WITH CHECK (%s)", exprsrc); + } + + /* Cleanup */ + systable_endscan(scandesc); + table_close(relation, AccessShareLock); + + return buf.data; +} + /* * Convert an int16[] Datum into a comma-separated list of column names diff --git a/src/include/commands/policy.h b/src/include/commands/policy.h index f06aa1df4394..a9f6d38a0d32 100644 --- a/src/include/commands/policy.h +++ b/src/include/commands/policy.h @@ -25,7 +25,7 @@ extern void RemovePolicyById(Oid policy_id); extern bool RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id); -extern ObjectAddress CreatePolicy(CreatePolicyStmt *stmt); +extern ObjectAddress CreatePolicy(CreatePolicyStmt *stmt, const char *queryString); extern ObjectAddress AlterPolicy(AlterPolicyStmt *stmt); extern Oid get_relation_policy_oid(Oid relid, const char *policy_name, @@ -34,5 +34,7 @@ extern Oid get_relation_policy_oid(Oid relid, const char *policy_name, extern ObjectAddress rename_policy(RenameStmt *stmt); extern bool relation_has_policies(Relation rel); +extern Oid PolicyGetRelation(Oid policyId); +extern char *get_policy_applied_command(char polcmd); #endif /* POLICY_H */ diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index bc7adba4a0fc..06d9d639edf3 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2508,6 +2508,7 @@ typedef enum AlterTableType AT_SetIdentity, /* SET identity column options */ AT_DropIdentity, /* DROP IDENTITY */ AT_ReAddStatistics, /* internal to commands/tablecmds.c */ + AT_ReAddPolicies, /* internal to commands/tablecmds.c */ } AlterTableType; typedef struct AlterTableCmd /* one subcommand of an ALTER TABLE */ @@ -3094,11 +3095,23 @@ typedef struct CreatePolicyStmt NodeTag type; char *policy_name; /* Policy's name */ RangeVar *table; /* the table name the policy applies to */ + + /* + * RangeTblEntry for the table. This is useful for avoid repeated name + * lookups issue. If CreatePolicyStmt.table has been looked up, we should + * not rely on it to resolve the relation again, use this rte field + * instead. This is useful when calling CreatePolicy not directly from + * parser. + */ + RangeTblEntry *rte; + char *cmd_name; /* the command name the policy applies to */ + char *polcomment; /* comment to apply to policies, or NULL */ bool permissive; /* restrictive or permissive policy */ List *roles; /* the roles associated with the policy */ Node *qual; /* the policy's condition */ Node *with_check; /* the policy's WITH CHECK condition. */ + bool transformed; /* true when transformPolicyStmt is finished */ } CreatePolicyStmt; /*---------------------- diff --git a/src/include/parser/parse_utilcmd.h b/src/include/parser/parse_utilcmd.h index 4965fac4495e..916df6eabbef 100644 --- a/src/include/parser/parse_utilcmd.h +++ b/src/include/parser/parse_utilcmd.h @@ -28,6 +28,7 @@ extern IndexStmt *transformIndexStmt(Oid relid, IndexStmt *stmt, const char *queryString); extern CreateStatsStmt *transformStatsStmt(Oid relid, CreateStatsStmt *stmt, const char *queryString); +extern CreatePolicyStmt *transformPolicyStmt(CreatePolicyStmt *stmt, const char *queryString); extern void transformRuleStmt(RuleStmt *stmt, const char *queryString, List **actions, Node **whereClause); extern List *transformCreateSchemaStmtElements(List *schemaElts, diff --git a/src/include/utils/ruleutils.h b/src/include/utils/ruleutils.h index 7ba7d8879149..b21ec468bde6 100644 --- a/src/include/utils/ruleutils.h +++ b/src/include/utils/ruleutils.h @@ -34,6 +34,7 @@ extern char *pg_get_partkeydef_columns(Oid relid, bool pretty); extern char *pg_get_partconstrdef_string(Oid partitionId, char *aliasname); extern char *pg_get_constraintdef_command(Oid constraintId); +extern char *pg_get_policy_def_command(Oid policyId); extern char *deparse_expression(Node *expr, List *dpcontext, bool forceprefix, bool showimplicit); extern List *deparse_context_for(const char *aliasname, Oid relid); diff --git a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c index 17d72e412ff8..9726fc764eee 100644 --- a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c +++ b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c @@ -314,6 +314,9 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS) case AT_ReAddStatistics: strtype = "(re) ADD STATS"; break; + case AT_ReAddPolicies: + strtype = "(re) ADD POLICIES"; + break; } if (subcmd->recurse) diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index c958ef4d70a1..c748f118bda4 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -26,6 +26,27 @@ GRANT regress_rls_group2 TO regress_rls_carol; CREATE SCHEMA regress_rls_schema; GRANT ALL ON SCHEMA regress_rls_schema to public; SET search_path = regress_rls_schema; +--check alter column set data type will recreate security policy +CREATE TABLE rls_tbl (a int, b int, c int); +CREATE POLICY p1 ON rls_tbl + FOR UPDATE + TO regress_rls_alice, regress_rls_bob, regress_rls_carol + USING (a < 20) WITH CHECK (c <> 0 and a is not null); +CREATE POLICY p2 ON rls_tbl AS RESTRICTIVE + FOR ALL + TO PUBLIC + USING (a < 40) WITH CHECK (c > 0 and a is not null); +ALTER TABLE rls_tbl ALTER COLUMN a SET DATA TYPE INT8, ALTER COLUMN c SET DATA TYPE INT8; +SELECT * FROM pg_policies +WHERE schemaname = 'regress_rls_schema' AND tablename = 'rls_tbl' +ORDER BY policyname; + schemaname | tablename | policyname | permissive | roles | cmd | qual | with_check +--------------------+-----------+------------+-------------+-------------------------------------------------------+--------+----------+-------------------------------- + regress_rls_schema | rls_tbl | p1 | PERMISSIVE | {regress_rls_alice,regress_rls_bob,regress_rls_carol} | UPDATE | (a < 20) | ((c <> 0) AND (a IS NOT NULL)) + regress_rls_schema | rls_tbl | p2 | RESTRICTIVE | {public} | ALL | (a < 40) | ((c > 0) AND (a IS NOT NULL)) +(2 rows) + +DROP TABLE rls_tbl; -- setup of malicious function CREATE OR REPLACE FUNCTION f_leak(text) RETURNS bool COST 0.0000001 LANGUAGE plpgsql @@ -2637,6 +2658,54 @@ SELECT * FROM document; 14 | 11 | 1 | regress_rls_bob | new novel | (16 rows) +CREATE TABLE document_c(LIKE document INCLUDING ALL); +CREATE POLICY p5 ON document_c AS RESTRICTIVE FOR ALL TO regress_rls_bob, regress_rls_alice, regress_rls_carol +USING (CID IS NOT NULL AND + (WITH CTE AS (SELECT uaccount IS NOT NULL FROM uaccount) SELECT true FROM CTE)); +ALTER TABLE document_c ALTER COLUMN cid SET DATA TYPE INT8; --error +ERROR: cannot alter type of a column used by a policy definition when the policy contains subquery +DETAIL: policy p5 on table document_c depends on column "cid" +ALTER TABLE uaccount ALTER COLUMN seclv SET DATA TYPE INT8; --error +ERROR: cannot alter type of a column used by a policy definition when the policy contains subquery +DETAIL: policy pp3 on table uaccount depends on column "seclv" +DROP POLICY p5 ON document_c CASCADE; +CREATE POLICY p5 ON document_c AS RESTRICTIVE + FOR ALL TO regress_rls_bob, regress_rls_alice, regress_rls_carol + USING (cid IS NOT NULL AND dlevel > 0); +COMMENT ON POLICY p5 ON document_c IS 'policy p5'; +SELECT * FROM pg_policies +WHERE schemaname = 'regress_rls_schema' AND tablename = 'document_c'; + schemaname | tablename | policyname | permissive | roles | cmd | qual | with_check +--------------------+------------+------------+-------------+-------------------------------------------------------+-----+--------------------------------------+------------ + regress_rls_schema | document_c | p5 | RESTRICTIVE | {regress_rls_alice,regress_rls_bob,regress_rls_carol} | ALL | ((cid IS NOT NULL) AND (dlevel > 0)) | +(1 row) + +ALTER TABLE document_c ALTER COLUMN dlevel SET DATA TYPE text; --error +ERROR: operator does not exist: text > integer +DETAIL: No operator of that name accepts the given argument types. +HINT: You might need to add explicit type casts. +ALTER TABLE document_c ALTER COLUMN cid SET DATA TYPE bool USING (cid::bool); +ALTER TABLE document_c + ALTER COLUMN cid SET DATA TYPE INT8 USING cid::INT4::INT8, + ALTER COLUMN dlevel SET DATA TYPE INT8; +SELECT * FROM pg_policies +WHERE schemaname = 'regress_rls_schema' AND tablename = 'document_c'; + schemaname | tablename | policyname | permissive | roles | cmd | qual | with_check +--------------------+------------+------------+-------------+-------------------------------------------------------+-----+--------------------------------------+------------ + regress_rls_schema | document_c | p5 | RESTRICTIVE | {regress_rls_alice,regress_rls_bob,regress_rls_carol} | ALL | ((cid IS NOT NULL) AND (dlevel > 0)) | +(1 row) + +--comments should not change +SELECT polname, description +FROM pg_description, pg_policy c +WHERE classoid = 'pg_policy'::regclass AND objoid = c.oid AND c.polrelid = 'document_c'::regclass +ORDER BY polname; + polname | description +---------+------------- + p5 | policy p5 +(1 row) + +DROP TABLE document_c; -- -- ROLE/GROUP -- diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql index 5d923c5ca3bc..a3a0f9e56f0f 100644 --- a/src/test/regress/sql/rowsecurity.sql +++ b/src/test/regress/sql/rowsecurity.sql @@ -35,6 +35,25 @@ CREATE SCHEMA regress_rls_schema; GRANT ALL ON SCHEMA regress_rls_schema to public; SET search_path = regress_rls_schema; +--check alter column set data type will recreate security policy +CREATE TABLE rls_tbl (a int, b int, c int); +CREATE POLICY p1 ON rls_tbl + FOR UPDATE + TO regress_rls_alice, regress_rls_bob, regress_rls_carol + USING (a < 20) WITH CHECK (c <> 0 and a is not null); +CREATE POLICY p2 ON rls_tbl AS RESTRICTIVE + FOR ALL + TO PUBLIC + USING (a < 40) WITH CHECK (c > 0 and a is not null); + +ALTER TABLE rls_tbl ALTER COLUMN a SET DATA TYPE INT8, ALTER COLUMN c SET DATA TYPE INT8; + +SELECT * FROM pg_policies +WHERE schemaname = 'regress_rls_schema' AND tablename = 'rls_tbl' +ORDER BY policyname; + +DROP TABLE rls_tbl; + -- setup of malicious function CREATE OR REPLACE FUNCTION f_leak(text) RETURNS bool COST 0.0000001 LANGUAGE plpgsql @@ -1163,6 +1182,41 @@ DROP POLICY p1 ON document; -- Just check everything went per plan SELECT * FROM document; +CREATE TABLE document_c(LIKE document INCLUDING ALL); +CREATE POLICY p5 ON document_c AS RESTRICTIVE FOR ALL TO regress_rls_bob, regress_rls_alice, regress_rls_carol +USING (CID IS NOT NULL AND + (WITH CTE AS (SELECT uaccount IS NOT NULL FROM uaccount) SELECT true FROM CTE)); + +ALTER TABLE document_c ALTER COLUMN cid SET DATA TYPE INT8; --error +ALTER TABLE uaccount ALTER COLUMN seclv SET DATA TYPE INT8; --error + +DROP POLICY p5 ON document_c CASCADE; +CREATE POLICY p5 ON document_c AS RESTRICTIVE + FOR ALL TO regress_rls_bob, regress_rls_alice, regress_rls_carol + USING (cid IS NOT NULL AND dlevel > 0); + +COMMENT ON POLICY p5 ON document_c IS 'policy p5'; + +SELECT * FROM pg_policies +WHERE schemaname = 'regress_rls_schema' AND tablename = 'document_c'; + +ALTER TABLE document_c ALTER COLUMN dlevel SET DATA TYPE text; --error +ALTER TABLE document_c ALTER COLUMN cid SET DATA TYPE bool USING (cid::bool); +ALTER TABLE document_c + ALTER COLUMN cid SET DATA TYPE INT8 USING cid::INT4::INT8, + ALTER COLUMN dlevel SET DATA TYPE INT8; + +SELECT * FROM pg_policies +WHERE schemaname = 'regress_rls_schema' AND tablename = 'document_c'; + +--comments should not change +SELECT polname, description +FROM pg_description, pg_policy c +WHERE classoid = 'pg_policy'::regclass AND objoid = c.oid AND c.polrelid = 'document_c'::regclass +ORDER BY polname; + +DROP TABLE document_c; + -- -- ROLE/GROUP --