From: Tom Lane Date: Fri, 3 Mar 2006 18:25:14 +0000 (+0000) Subject: Dept. of second thoughts: rejigger the TRUNCATE ... CASCADE patch so that X-Git-Tag: REL9_0_0~8289 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=4e086f7cb516b57624eb4b135c2539b827fb191e;p=pg-rex%2Fsyncrep.git Dept. of second thoughts: rejigger the TRUNCATE ... CASCADE patch so that relations are still checked for permissions etc as soon as they are opened. The original form of the patch could hold exclusive lock for a long time on relations that the user doesn't even have permissions to access, let alone truncate. --- diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 4a3c146faa..14ec0c899a 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.178 2006/03/03 03:30:52 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.179 2006/03/03 18:25:14 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -156,6 +156,7 @@ typedef struct NewColumnValue } NewColumnValue; +static void truncate_check_rel(Relation rel); static List *MergeAttributes(List *schema, List *supers, bool istemp, List **supOids, List **supconstr, int *supOidCount); static bool change_varattnos_of_a_node(Node *node, const AttrNumber *newattno); @@ -539,33 +540,33 @@ void ExecuteTruncate(TruncateStmt *stmt) { List *rels = NIL; - List *directRelids = NIL; + List *relids = NIL; ListCell *cell; - Oid relid; - Relation rel; /* - * Open and exclusive-lock all the explicitly-specified relations + * Open, exclusive-lock, and check all the explicitly-specified relations */ foreach(cell, stmt->relations) { RangeVar *rv = lfirst(cell); + Relation rel; rel = heap_openrv(rv, AccessExclusiveLock); + truncate_check_rel(rel); rels = lappend(rels, rel); - directRelids = lappend_oid(directRelids, RelationGetRelid(rel)); + relids = lappend_oid(relids, RelationGetRelid(rel)); } /* * In CASCADE mode, suck in all referencing relations as well. This * requires multiple iterations to find indirectly-dependent relations. * At each phase, we need to exclusive-lock new rels before looking - * for their dependencies, else we might miss something. + * for their dependencies, else we might miss something. Also, we + * check each rel as soon as we open it, to avoid a faux pas such as + * holding lock for a long time on a rel we have no permissions for. */ if (stmt->behavior == DROP_CASCADE) { - List *relids = list_copy(directRelids); - for (;;) { List *newrelids; @@ -576,70 +577,20 @@ ExecuteTruncate(TruncateStmt *stmt) foreach(cell, newrelids) { - relid = lfirst_oid(cell); + Oid relid = lfirst_oid(cell); + Relation rel; + rel = heap_open(relid, AccessExclusiveLock); + ereport(NOTICE, + (errmsg("truncate cascades to table \"%s\"", + RelationGetRelationName(rel)))); + truncate_check_rel(rel); rels = lappend(rels, rel); relids = lappend_oid(relids, relid); } } } - /* now check all involved relations */ - foreach(cell, rels) - { - rel = (Relation) lfirst(cell); - relid = RelationGetRelid(rel); - - /* - * If this table was added to the command by CASCADE, report it. - * We don't do this earlier because if we error out on one of the - * tables, it'd be confusing to list subsequently-added tables. - */ - if (stmt->behavior == DROP_CASCADE && - !list_member_oid(directRelids, relid)) - ereport(NOTICE, - (errmsg("truncate cascades to table \"%s\"", - RelationGetRelationName(rel)))); - - /* Only allow truncate on regular tables */ - if (rel->rd_rel->relkind != RELKIND_RELATION) - ereport(ERROR, - (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table", - RelationGetRelationName(rel)))); - - /* Permissions checks */ - if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS, - RelationGetRelationName(rel)); - - if (!allowSystemTableMods && IsSystemRelation(rel)) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("permission denied: \"%s\" is a system catalog", - RelationGetRelationName(rel)))); - - /* - * We can never allow truncation of shared or nailed-in-cache - * relations, because we can't support changing their relfilenode - * values. - */ - if (rel->rd_rel->relisshared || rel->rd_isnailed) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot truncate system relation \"%s\"", - RelationGetRelationName(rel)))); - - /* - * Don't allow truncate on temp tables of other backends ... their - * local buffer manager is not going to cope. - */ - if (isOtherTempNamespace(RelationGetNamespace(rel))) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot truncate temporary tables of other sessions"))); - } - /* * Check foreign key references. In CASCADE mode, this should be * unnecessary since we just pulled in all the references; but as @@ -657,11 +608,10 @@ ExecuteTruncate(TruncateStmt *stmt) */ foreach(cell, rels) { + Relation rel = (Relation) lfirst(cell); Oid heap_relid; Oid toast_relid; - rel = (Relation) lfirst(cell); - /* * Create a new empty storage file for the relation, and assign it as * the relfilenode value. The old storage file is scheduled for @@ -691,6 +641,51 @@ ExecuteTruncate(TruncateStmt *stmt) } } +/* + * Check that a given rel is safe to truncate. Subroutine for ExecuteTruncate + */ +static void +truncate_check_rel(Relation rel) +{ + /* Only allow truncate on regular tables */ + if (rel->rd_rel->relkind != RELKIND_RELATION) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is not a table", + RelationGetRelationName(rel)))); + + /* Permissions checks */ + if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS, + RelationGetRelationName(rel)); + + if (!allowSystemTableMods && IsSystemRelation(rel)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied: \"%s\" is a system catalog", + RelationGetRelationName(rel)))); + + /* + * We can never allow truncation of shared or nailed-in-cache + * relations, because we can't support changing their relfilenode + * values. + */ + if (rel->rd_rel->relisshared || rel->rd_isnailed) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot truncate system relation \"%s\"", + RelationGetRelationName(rel)))); + + /* + * Don't allow truncate on temp tables of other backends ... their + * local buffer manager is not going to cope. + */ + if (isOtherTempNamespace(RelationGetNamespace(rel))) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot truncate temporary tables of other sessions"))); +} + /*---------- * MergeAttributes * Returns new schema given initial schema and superclasses.