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

diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index b37f36b3e6716be16ea47cbb90e63a89102abafd..a1a9808e5d94959218b415ed34c46579c478c177 100644 (file)
--- 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 47295600adbc3430282fb2be5ca4e1b0767650e6..f481c531ac7e19a257e92069db90f80dd18cd6c8 100644 (file)
--- 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 ed40602bc878f39b6b54e0328751de3fadf7b614..70de400356a1a1bdb4bfb3bee1eef6c8c14a59fb 100644 (file)
--- 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 869ca8c9a9af36ee1dc79f3540d3204ac16d737c..6ba984a92899eba1cfcdddc64256851f6eb506f9 100644 (file)
--- 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 b8d67ae9f3d473ccc004aebdaa37c9e9dfd4fc46..4f49a0deca431a2caecaee8c43d237db949c9153 100644 (file)
--- 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;
 
 --