From b15a6da29217b14f02895af1d9271e84415a91ae Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 3 Mar 2013 19:05:47 -0500 Subject: [PATCH] Get rid of any toast table when converting a table to a view. Also make sure other fields of the view's pg_class entry are appropriate for a view; it shouldn't have relfrozenxid set for instance. This ancient omission isn't believed to have any serious consequences in versions 8.4-9.2, so no backpatch. But let's fix it before it does bite us in some serious way. It's just luck that the case doesn't cause problems for autovacuum. (It did cause problems in 8.3, but that's out of support.) Andres Freund --- src/backend/rewrite/rewriteDefine.c | 98 ++++++++++++++++++++++++---- src/backend/rewrite/rewriteSupport.c | 11 +--- src/include/rewrite/rewriteSupport.h | 3 +- src/test/regress/expected/rules.out | 7 ++ src/test/regress/sql/rules.sql | 3 + 5 files changed, 101 insertions(+), 21 deletions(-) diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c index b37f36b3e6..a1a9808e5d 100644 --- a/src/backend/rewrite/rewriteDefine.c +++ b/src/backend/rewrite/rewriteDefine.c @@ -16,6 +16,9 @@ #include "access/heapam.h" #include "access/htup_details.h" +#include "access/multixact.h" +#include "access/transam.h" +#include "access/xact.h" #include "catalog/catalog.h" #include "catalog/dependency.h" #include "catalog/heap.h" @@ -409,7 +412,7 @@ DefineQueryRewrite(char *rulename, * strict, because they will reject relations that once had such but * don't anymore. But we don't really care, because this whole * business of converting relations to views is just a kluge to allow - * loading ancient pg_dump files.) + * dump/reload of views that participate in circular dependencies.) */ if (event_relation->rd_rel->relkind != RELKIND_VIEW) { @@ -500,30 +503,103 @@ DefineQueryRewrite(char *rulename, replace); /* - * Set pg_class 'relhasrules' field TRUE for event relation. If - * appropriate, also modify the 'relkind' field to show that the - * relation is now a view. + * Set pg_class 'relhasrules' field TRUE for event relation. * * Important side effect: an SI notice is broadcast to force all * backends (including me!) to update relcache entries with the new * rule. */ - SetRelationRuleStatus(event_relid, true, RelisBecomingView); + SetRelationRuleStatus(event_relid, true); } - /* - * If the relation is becoming a view, delete the storage files associated - * with it. Also, get rid of any system attribute entries in pg_attribute, - * because a view shouldn't have any of those. + /* --------------------------------------------------------------------- + * If the relation is becoming a view: + * - delete the associated storage files + * - get rid of any system attributes in pg_attribute; a view shouldn't + * have any of those + * - remove the toast table; there is no need for it anymore, and its + * presence would make vacuum slightly more complicated + * - set relkind to RELKIND_VIEW, and adjust other pg_class fields + * to be appropriate for a view * * NB: we had better have AccessExclusiveLock to do this ... - * - * XXX what about getting rid of its TOAST table? For now, we don't. + * --------------------------------------------------------------------- */ if (RelisBecomingView) { + Relation relationRelation; + Oid toastrelid; + HeapTuple classTup; + Form_pg_class classForm; + + relationRelation = heap_open(RelationRelationId, RowExclusiveLock); + toastrelid = event_relation->rd_rel->reltoastrelid; + + /* drop storage while table still looks like a table */ RelationDropStorage(event_relation); DeleteSystemAttributeTuples(event_relid); + + /* + * Drop the toast table if any. (This won't take care of updating + * the toast fields in the relation's own pg_class entry; we handle + * that below.) + */ + if (OidIsValid(toastrelid)) + { + ObjectAddress toastobject; + + /* + * Delete the dependency of the toast relation on the main + * relation so we can drop the former without dropping the latter. + */ + deleteDependencyRecordsFor(RelationRelationId, toastrelid, + false); + + /* Make deletion of dependency record visible */ + CommandCounterIncrement(); + + /* Now drop toast table, including its index */ + toastobject.classId = RelationRelationId; + toastobject.objectId = toastrelid; + toastobject.objectSubId = 0; + performDeletion(&toastobject, DROP_RESTRICT, + PERFORM_DELETION_INTERNAL); + } + + /* + * SetRelationRuleStatus may have updated the pg_class row, so we must + * advance the command counter before trying to update it again. + */ + CommandCounterIncrement(); + + /* + * Fix pg_class entry to look like a normal view's, including setting + * the correct relkind and removal of reltoastrelid/reltoastidxid of + * the toast table we potentially removed above. + */ + classTup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(event_relid)); + if (!HeapTupleIsValid(classTup)) + elog(ERROR, "cache lookup failed for relation %u", event_relid); + classForm = (Form_pg_class) GETSTRUCT(classTup); + + classForm->reltablespace = InvalidOid; + classForm->relpages = 0; + classForm->reltuples = 0; + classForm->relallvisible = 0; + classForm->reltoastrelid = InvalidOid; + classForm->reltoastidxid = InvalidOid; + classForm->relhasindex = false; + classForm->relkind = RELKIND_VIEW; + classForm->relhasoids = false; + classForm->relhaspkey = false; + classForm->relfrozenxid = InvalidTransactionId; + classForm->relminmxid = InvalidMultiXactId; + + simple_heap_update(relationRelation, &classTup->t_self, classTup); + CatalogUpdateIndexes(relationRelation, classTup); + + heap_freetuple(classTup); + heap_close(relationRelation, RowExclusiveLock); } /* Close rel, but keep lock till commit... */ diff --git a/src/backend/rewrite/rewriteSupport.c b/src/backend/rewrite/rewriteSupport.c index 47295600ad..f481c531ac 100644 --- a/src/backend/rewrite/rewriteSupport.c +++ b/src/backend/rewrite/rewriteSupport.c @@ -41,8 +41,7 @@ IsDefinedRewriteRule(Oid owningRel, const char *ruleName) /* * SetRelationRuleStatus - * Set the value of the relation's relhasrules field in pg_class; - * if the relation is becoming a view, also adjust its relkind. + * Set the value of the relation's relhasrules field in pg_class. * * NOTE: caller must be holding an appropriate lock on the relation. * @@ -53,8 +52,7 @@ IsDefinedRewriteRule(Oid owningRel, const char *ruleName) * row. */ void -SetRelationRuleStatus(Oid relationId, bool relHasRules, - bool relIsBecomingView) +SetRelationRuleStatus(Oid relationId, bool relHasRules) { Relation relationRelation; HeapTuple tuple; @@ -69,13 +67,10 @@ SetRelationRuleStatus(Oid relationId, bool relHasRules, elog(ERROR, "cache lookup failed for relation %u", relationId); classForm = (Form_pg_class) GETSTRUCT(tuple); - if (classForm->relhasrules != relHasRules || - (relIsBecomingView && classForm->relkind != RELKIND_VIEW)) + if (classForm->relhasrules != relHasRules) { /* Do the update */ classForm->relhasrules = relHasRules; - if (relIsBecomingView) - classForm->relkind = RELKIND_VIEW; simple_heap_update(relationRelation, &tuple->t_self, tuple); diff --git a/src/include/rewrite/rewriteSupport.h b/src/include/rewrite/rewriteSupport.h index ed40602bc8..70de400356 100644 --- a/src/include/rewrite/rewriteSupport.h +++ b/src/include/rewrite/rewriteSupport.h @@ -19,8 +19,7 @@ extern bool IsDefinedRewriteRule(Oid owningRel, const char *ruleName); -extern void SetRelationRuleStatus(Oid relationId, bool relHasRules, - bool relIsBecomingView); +extern void SetRelationRuleStatus(Oid relationId, bool relHasRules); extern Oid get_rewrite_oid(Oid relid, const char *rulename, bool missing_ok); extern Oid get_rewrite_oid_without_relid(const char *rulename, diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 869ca8c9a9..6ba984a928 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -2327,6 +2327,13 @@ select xmin, * from fooview; -- fail, views don't have such a column ERROR: column "xmin" does not exist LINE 1: select xmin, * from fooview; ^ +select reltoastrelid, reltoastidxid, relkind, relfrozenxid + from pg_class where oid = 'fooview'::regclass; + reltoastrelid | reltoastidxid | relkind | relfrozenxid +---------------+---------------+---------+-------------- + 0 | 0 | v | 0 +(1 row) + drop view fooview; -- -- check for planner problems with complex inherited UPDATES diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql index b8d67ae9f3..4f49a0deca 100644 --- a/src/test/regress/sql/rules.sql +++ b/src/test/regress/sql/rules.sql @@ -872,6 +872,9 @@ create rule "_RETURN" as on select to fooview do instead select * from fooview; select xmin, * from fooview; -- fail, views don't have such a column +select reltoastrelid, reltoastidxid, relkind, relfrozenxid + from pg_class where oid = 'fooview'::regclass; + drop view fooview; --