From: Tom Lane Date: Sun, 7 Jan 2001 00:05:22 +0000 (+0000) Subject: Clean up checking of relkind for ALTER TABLE and LOCK TABLE commands. X-Git-Tag: REL7_1_BETA2~3 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1402201463d98f0ef8ab07ac7aeb06d60cf337dc;p=postgresql Clean up checking of relkind for ALTER TABLE and LOCK TABLE commands. Disallow cases like adding constraints to sequences :-(, and eliminate now-unnecessary search of pg_rewrite to decide if a relation is a view. --- diff --git a/src/backend/commands/command.c b/src/backend/commands/command.c index 035967f920..d3a00753e5 100644 --- a/src/backend/commands/command.c +++ b/src/backend/commands/command.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/commands/Attic/command.c,v 1.114 2000/12/22 23:12:05 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/commands/Attic/command.c,v 1.115 2001/01/07 00:05:22 tgl Exp $ * * NOTES * The PerformAddAttribute() code, like most of the relation @@ -57,8 +57,7 @@ static bool needs_toast_table(Relation rel); -static bool is_viewr(char *relname); -static bool is_view(Relation rel); +static bool is_relation(char *name); /* -------------------------------- @@ -302,6 +301,11 @@ AlterTableAddColumn(const char *relationName, * release until end of transaction. */ rel = heap_openr(relationName, AccessExclusiveLock); + + if (rel->rd_rel->relkind != RELKIND_RELATION) + elog(ERROR, "ALTER TABLE: relation \"%s\" is not a table", + relationName); + myrelid = RelationGetRelid(rel); heap_close(rel, NoLock); /* close rel but keep lock! */ @@ -367,13 +371,6 @@ AlterTableAddColumn(const char *relationName, elog(ERROR, "ALTER TABLE: relation \"%s\" not found", relationName); - /* - * XXX is the following check sufficient? - */ - if (((Form_pg_class) GETSTRUCT(reltup))->relkind != RELKIND_RELATION) - elog(ERROR, "ALTER TABLE: relation \"%s\" is not a table", - relationName); - minattnum = ((Form_pg_class) GETSTRUCT(reltup))->relnatts; maxatts = minattnum + 1; if (maxatts > MaxHeapAttributeNumber) @@ -517,8 +514,9 @@ AlterTableAlterColumn(const char *relationName, #endif rel = heap_openr(relationName, AccessExclusiveLock); - if ( rel->rd_rel->relkind == RELKIND_VIEW ) - elog(ERROR, "ALTER TABLE: %s is a view", relationName); + if (rel->rd_rel->relkind != RELKIND_RELATION) + elog(ERROR, "ALTER TABLE: relation \"%s\" is not a table", + relationName); myrelid = RelationGetRelid(rel); heap_close(rel, NoLock); @@ -936,6 +934,11 @@ AlterTableDropColumn(const char *relationName, * release until end of transaction. */ rel = heap_openr(relationName, AccessExclusiveLock); + + if (rel->rd_rel->relkind != RELKIND_RELATION) + elog(ERROR, "ALTER TABLE: relation \"%s\" is not a table", + relationName); + myrelid = RelationGetRelid(rel); heap_close(rel, NoLock); /* close rel but keep lock! */ @@ -969,15 +972,6 @@ AlterTableDropColumn(const char *relationName, reltup = heap_copytuple(&classtuple); ReleaseBuffer(buffer); - /* - * XXX is the following check sufficient? - */ - if (((Form_pg_class) GETSTRUCT(reltup))->relkind != RELKIND_RELATION) - { - elog(ERROR, "ALTER TABLE: relation \"%s\" is not a table", - relationName); - } - attrdesc = heap_openr(AttributeRelationName, RowExclusiveLock); /* @@ -1090,9 +1084,10 @@ AlterTableAddConstraint(char *relationName, elog(ERROR, "ALTER TABLE: permission denied"); #endif - /* check to see if the table to be constrained is a view. */ - if (is_viewr(relationName)) - elog(ERROR, "ALTER TABLE: Cannot add constraints to views."); + /* Disallow ADD CONSTRAINT on views, indexes, sequences, etc */ + if (! is_relation(relationName)) + elog(ERROR, "ALTER TABLE ADD CONSTRAINT: %s is not a table", + relationName); switch (nodeTag(newConstraint)) { @@ -1482,9 +1477,18 @@ AlterTableOwner(const char *relationName, const char *newOwnerName) elog(ERROR, "ALTER TABLE: relation \"%s\" not found", relationName); - if (((Form_pg_class) GETSTRUCT(tuple))->relkind != RELKIND_RELATION) - elog(ERROR, "ALTER TABLE: relation \"%s\" is not a table", - relationName); + switch (((Form_pg_class) GETSTRUCT(tuple))->relkind) + { + case RELKIND_RELATION: + case RELKIND_INDEX: + case RELKIND_VIEW: + case RELKIND_SEQUENCE: + /* ok to change owner */ + break; + default: + elog(ERROR, "ALTER TABLE: relation \"%s\" is not a table, index, view, or sequence", + relationName); + } /* * modify the table's entry and write to the heap @@ -1541,6 +1545,11 @@ AlterTableCreateToastTable(const char *relationName, bool silent) * release until end of transaction. */ rel = heap_openr(relationName, AccessExclusiveLock); + + if (rel->rd_rel->relkind != RELKIND_RELATION) + elog(ERROR, "ALTER TABLE: relation \"%s\" is not a table", + relationName); + myrelid = RelationGetRelid(rel); /* @@ -1569,19 +1578,8 @@ AlterTableCreateToastTable(const char *relationName, bool silent) ReleaseBuffer(buffer); /* - * XXX is the following check sufficient? At least it would - * allow to create TOAST tables for views. But why not - someone - * can insert into a view, so it shouldn't be impossible to hide - * huge data there :-) - * - * Not any more. + * Is it already toasted? */ - if (((Form_pg_class) GETSTRUCT(reltup))->relkind != RELKIND_RELATION) - { - elog(ERROR, "ALTER TABLE: relation \"%s\" is not a table", - relationName); - } - if (((Form_pg_class) GETSTRUCT(reltup))->reltoastrelid != InvalidOid) { if (silent) @@ -1781,9 +1779,6 @@ LockTableCommand(LockStmt *lockstmt) if (rel->rd_rel->relkind != RELKIND_RELATION) elog(ERROR, "LOCK TABLE: %s is not a table", lockstmt->relname); - if (is_view(rel)) - elog(ERROR, "LOCK TABLE: cannot lock a view"); - if (lockstmt->mode == AccessShareLock) aclresult = pg_aclcheck(lockstmt->relname, GetUserId(), ACL_RD); else @@ -1799,60 +1794,13 @@ LockTableCommand(LockStmt *lockstmt) static bool -is_viewr(char *name) +is_relation(char *name) { Relation rel = heap_openr(name, NoLock); - bool retval = is_view(rel); + bool retval = (rel->rd_rel->relkind == RELKIND_RELATION); heap_close(rel, NoLock); return retval; } - -static bool -is_view(Relation rel) -{ - Relation RewriteRelation; - HeapScanDesc scanDesc; - ScanKeyData scanKeyData; - HeapTuple tuple; - Form_pg_rewrite data; - bool retval = false; - - /* - * Open the pg_rewrite relation. - */ - RewriteRelation = heap_openr(RewriteRelationName, RowExclusiveLock); - - /* - * Scan the RuleRelation ('pg_rewrite') for all the tuples that has - * the same ev_class as the relation being checked. - */ - ScanKeyEntryInitialize(&scanKeyData, - 0, - Anum_pg_rewrite_ev_class, - F_OIDEQ, - ObjectIdGetDatum(rel->rd_id)); - scanDesc = heap_beginscan(RewriteRelation, - 0, SnapshotNow, 1, &scanKeyData); - - while (HeapTupleIsValid(tuple = heap_getnext(scanDesc, 0))) - { - if (tuple->t_data != NULL) - { - data = (Form_pg_rewrite) GETSTRUCT(tuple); - if (data->ev_type == '1') - { - retval = true; - break; - } - } - - } - - heap_endscan(scanDesc); - heap_close(RewriteRelation, RowExclusiveLock); - - return retval; -}