From a13b01853084b6c6f9c34944bc19b3dd7dc4ceb2 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 19 Sep 2003 21:04:20 +0000 Subject: [PATCH] Disallow foreign-key references from temp tables to permanent tables. Per recent discussion, this does not work because other backends can't reliably see tuples in a temp table and so cannot run the RI checks correctly. Seems better to disallow this case than go back to accessing temp tables through shared buffers. Also, disallow FK references to ON COMMIT DELETE ROWS tables. We already caught this problem for normal TRUNCATE, but the path used by ON COMMIT didn't check. --- doc/src/sgml/ref/truncate.sgml | 12 ++++- src/backend/catalog/heap.c | 71 ++++++++++++++++++++++++-- src/backend/commands/tablecmds.c | 67 +++++++++--------------- src/include/catalog/heap.h | 4 +- src/test/regress/expected/truncate.out | 2 +- 5 files changed, 107 insertions(+), 49 deletions(-) diff --git a/doc/src/sgml/ref/truncate.sgml b/doc/src/sgml/ref/truncate.sgml index fc65f65db55..aa27890f47e 100644 --- a/doc/src/sgml/ref/truncate.sgml +++ b/doc/src/sgml/ref/truncate.sgml @@ -1,5 +1,5 @@ @@ -50,6 +50,16 @@ TRUNCATE [ TABLE ] name + + Notes + + + TRUNCATE cannot be used if there are foreign-key references + to the table from other tables. Checking validity in such cases would + require table scans, and the whole point is not to do one. + + + Examples diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index c3253677f93..ce189ff903b 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.251 2003/08/04 02:39:58 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.252 2003/09/19 21:04:19 tgl Exp $ * * * INTERFACE ROUTINES @@ -1966,9 +1966,11 @@ RelationTruncateIndexes(Oid heapId) /* * heap_truncate * - * This routine is used to truncate the data from the - * storage manager of any data within the relation handed - * to this routine. This is not transaction-safe! + * This routine deletes all data within the specified relation. + * + * This is not transaction-safe! There is another, transaction-safe + * implementation in commands/cluster.c. We now use this only for + * ON COMMIT truncation of temporary tables, where it doesn't matter. */ void heap_truncate(Oid rid) @@ -1979,6 +1981,9 @@ heap_truncate(Oid rid) /* Open relation for processing, and grab exclusive access on it. */ rel = heap_open(rid, AccessExclusiveLock); + /* Don't allow truncate on tables that are referenced by foreign keys */ + heap_truncate_check_FKs(rel); + /* * Release any buffers associated with this relation. If they're * dirty, they're just dropped without bothering to flush to disk. @@ -2003,3 +2008,61 @@ heap_truncate(Oid rid) */ heap_close(rel, NoLock); } + +/* + * heap_truncate_check_FKs + * Check for foreign keys referencing a relation that's to be truncated + * + * We disallow such FKs (except self-referential ones) since the whole point + * of TRUNCATE is to not scan the individual rows to be thrown away. + * + * This is split out so it can be shared by both implementations of truncate. + * Caller should already hold a suitable lock on the relation. + */ +void +heap_truncate_check_FKs(Relation rel) +{ + Oid relid = RelationGetRelid(rel); + ScanKeyData key; + Relation fkeyRel; + SysScanDesc fkeyScan; + HeapTuple tuple; + + /* + * Fast path: if the relation has no triggers, it surely has no FKs + * either. + */ + if (rel->rd_rel->reltriggers == 0) + return; + + /* + * Otherwise, must scan pg_constraint. Right now, this is a seqscan + * because there is no available index on confrelid. + */ + fkeyRel = heap_openr(ConstraintRelationName, AccessShareLock); + + ScanKeyEntryInitialize(&key, 0, + Anum_pg_constraint_confrelid, + F_OIDEQ, + ObjectIdGetDatum(relid)); + + fkeyScan = systable_beginscan(fkeyRel, NULL, false, + SnapshotNow, 1, &key); + + while (HeapTupleIsValid(tuple = systable_getnext(fkeyScan))) + { + Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(tuple); + + if (con->contype == CONSTRAINT_FOREIGN && con->conrelid != relid) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot truncate a table referenced in a foreign key constraint"), + errdetail("Table \"%s\" references \"%s\" via foreign key constraint \"%s\".", + get_rel_name(con->conrelid), + RelationGetRelationName(rel), + NameStr(con->conname)))); + } + + systable_endscan(fkeyScan); + heap_close(fkeyRel, AccessShareLock); +} diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b99d68b3400..fc8a87123fe 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v 1.81 2003/09/15 00:26:31 petere Exp $ + * $Header: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v 1.82 2003/09/19 21:04:20 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -365,15 +365,9 @@ void TruncateRelation(const RangeVar *relation) { Relation rel; - Oid relid; - ScanKeyData key; - Relation fkeyRel; - SysScanDesc fkeyScan; - HeapTuple tuple; /* Grab exclusive lock in preparation for truncate */ rel = heap_openrv(relation, AccessExclusiveLock); - relid = RelationGetRelid(rel); /* Only allow truncate on regular tables */ if (rel->rd_rel->relkind != RELKIND_RELATION) @@ -383,7 +377,7 @@ TruncateRelation(const RangeVar *relation) RelationGetRelationName(rel)))); /* Permissions checks */ - if (!pg_class_ownercheck(relid, GetUserId())) + if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId())) aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS, RelationGetRelationName(rel)); @@ -405,35 +399,7 @@ TruncateRelation(const RangeVar *relation) /* * Don't allow truncate on tables which are referenced by foreign keys */ - fkeyRel = heap_openr(ConstraintRelationName, AccessShareLock); - - ScanKeyEntryInitialize(&key, 0, - Anum_pg_constraint_confrelid, - F_OIDEQ, - ObjectIdGetDatum(relid)); - - fkeyScan = systable_beginscan(fkeyRel, 0, false, - SnapshotNow, 1, &key); - - /* - * First foreign key found with us as the reference should throw an - * error. - */ - while (HeapTupleIsValid(tuple = systable_getnext(fkeyScan))) - { - Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(tuple); - - if (con->contype == 'f' && con->conrelid != relid) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot truncate a table referenced in a foreign key constraint"), - errdetail("Table \"%s\" references this one via foreign key constraint \"%s\".", - get_rel_name(con->conrelid), - NameStr(con->conname)))); - } - - systable_endscan(fkeyScan); - heap_close(fkeyRel, AccessShareLock); + heap_truncate_check_FKs(rel); /* * Do the real work using the same technique as cluster, but without @@ -3137,11 +3103,28 @@ AlterTableAddForeignKeyConstraint(Relation rel, FkConstraint *fkconstraint) aclcheck_error(aclresult, ACL_KIND_CLASS, RelationGetRelationName(rel)); - if (isTempNamespace(RelationGetNamespace(pkrel)) && - !isTempNamespace(RelationGetNamespace(rel))) - ereport(ERROR, - (errcode(ERRCODE_INVALID_TABLE_DEFINITION), - errmsg("cannot reference temporary table from permanent table constraint"))); + /* + * Disallow reference from permanent table to temp table or vice versa. + * (The ban on perm->temp is for fairly obvious reasons. The ban on + * temp->perm is because other backends might need to run the RI triggers + * on the perm table, but they can't reliably see tuples the owning + * backend has created in the temp table, because non-shared buffers + * are used for temp tables.) + */ + if (isTempNamespace(RelationGetNamespace(pkrel))) + { + if (!isTempNamespace(RelationGetNamespace(rel))) + ereport(ERROR, + (errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("cannot reference temporary table from permanent table constraint"))); + } + else + { + if (isTempNamespace(RelationGetNamespace(rel))) + ereport(ERROR, + (errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("cannot reference permanent table from temporary table constraint"))); + } /* * Look up the referencing attributes to make sure they exist, and diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h index 86c97ff4786..a9cd65ff068 100644 --- a/src/include/catalog/heap.h +++ b/src/include/catalog/heap.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: heap.h,v 1.61 2003/08/04 02:40:10 momjian Exp $ + * $Id: heap.h,v 1.62 2003/09/19 21:04:20 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -48,6 +48,8 @@ extern void heap_drop_with_catalog(Oid rid); extern void heap_truncate(Oid rid); +extern void heap_truncate_check_FKs(Relation rel); + extern void AddRelationRawConstraints(Relation rel, List *rawColDefaults, List *rawConstraints); diff --git a/src/test/regress/expected/truncate.out b/src/test/regress/expected/truncate.out index 91ea40b7929..2cd41f83ba0 100644 --- a/src/test/regress/expected/truncate.out +++ b/src/test/regress/expected/truncate.out @@ -42,7 +42,7 @@ SELECT * FROM truncate_a; TRUNCATE truncate_a; ERROR: cannot truncate a table referenced in a foreign key constraint -DETAIL: Table "truncate_b" references this one via foreign key constraint "$1". +DETAIL: Table "truncate_b" references "truncate_a" via foreign key constraint "$1". SELECT * FROM truncate_a; col1 ------