On 2013-02-14 20:47:11 -0500, Tom Lane wrote:
> Andres Freund <[email protected]> writes:
> > The current behaviour doesn't seem to be a terribly good idea. I propose
> > to drop the toast table and reset the relfrozenxid in DefineQueryRewrite
> > in the RelisBecomingView case.
>
> Yeah, probably worth doing. At the time we thought that that code path
> was just a short-term legacy thing for loading ancient pg_dump files.
> However, given that even modern pg_dumps will use this syntax if
> necessary to break circular dependencies for views, we're probably never
> going to be rid of it completely.
What about the attached patch? I chose to move the update of relkind
from SetRelationRuleStatus to the RelisBecomingView part of
DefineQueryRewrite. As we're updating pg_class in there anyway there
doesn't seem to be any reason to spread knowledge of that any further.
Andres
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index b37f36b..7e7b16a 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/transam.h"
+#include "access/multixact.h"
+#include "access/xact.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
#include "catalog/heap.h"
@@ -500,30 +503,101 @@ 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
+ * - adjust other pg_class attributes 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);
+
+ /*
+ * Now drop the toast table which is not needed anymore, the pg_class
+ * entry is adapted below.
+ */
+ if (toastrelid != InvalidOid)
+ {
+ ObjectAddress toastobject;
+
+ /*
+ * delete the dependency of the main relation to the toast relation
+ * so we can delete the toast relation without also deleting what
+ * is becoming the view.
+ */
+ deleteDependencyRecordsFor(RelationRelationId, toastrelid,
+ false);
+
+ /* make deletion of dependency record visible */
+ CommandCounterIncrement();
+
+ /* now drop toast table, including index */
+ toastobject.classId = RelationRelationId;
+ toastobject.objectId = toastrelid;
+ toastobject.objectSubId = 0;
+ performDeletion(&toastobject, DROP_RESTRICT, PERFORM_DELETION_INTERNAL);
+ }
+
+ /*
+ * Fixup 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.
+ */
+
+ /*
+ * SetRelationRuleStatus may have updated the pg_class row, so make
+ * current version visible before we fetch the current tuple.
+ */
+ CommandCounterIncrement();
+
+ 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->relkind = RELKIND_VIEW;
+ classForm->relhasoids = 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 4729560..8af960a 100644
--- a/src/backend/rewrite/rewriteSupport.c
+++ b/src/backend/rewrite/rewriteSupport.c
@@ -42,7 +42,6 @@ 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.
*
* 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 ed40602..70de400 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 4226aad..4e58972 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2316,25 +2316,47 @@ drop view fooview;
--
-- test conversion of table to view (needed to load some pg_dump files)
--
-create table fooview (x int, y text);
-select xmin, * from fooview;
+create table fooview_toast (x int, y text);
+select xmin, * from fooview_toast;
xmin | x | y
------+---+---
(0 rows)
-create rule "_RETURN" as on select to fooview do instead
+create rule "_RETURN" as on select to fooview_toast do instead
select 1 as x, 'aaa'::text as y;
-select * from fooview;
+select * from fooview_toast;
x | y
---+-----
1 | aaa
(1 row)
-select xmin, * from fooview; -- fail, views don't have such a column
+select xmin, * from fooview_toast; -- fail, views don't have such a column
ERROR: column "xmin" does not exist
-LINE 1: select xmin, * from fooview;
+LINE 1: select xmin, * from fooview_toast;
^
-drop view fooview;
+BEGIN;
+create table fooview_toast_onetx (x int, y text);
+create rule "_RETURN" as on select to fooview_toast_onetx do instead
+ select 1 as x, 'aaa'::text as y;
+COMMIT;
+select * from fooview_toast_onetx;
+ x | y
+---+-----
+ 1 | aaa
+(1 row)
+
+create table fooview_notoast (x int);
+create rule "_RETURN" as on select to fooview_notoast do instead
+ select 1 as x;
+select * from fooview_notoast;
+ x
+---
+ 1
+(1 row)
+
+drop view fooview_toast;
+drop view fooview_toast_onetx;
+drop view fooview_notoast;
--
-- 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 b8d67ae..1173055 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -863,16 +863,33 @@ drop view fooview;
-- test conversion of table to view (needed to load some pg_dump files)
--
-create table fooview (x int, y text);
-select xmin, * from fooview;
+create table fooview_toast (x int, y text);
+select xmin, * from fooview_toast;
-create rule "_RETURN" as on select to fooview do instead
+create rule "_RETURN" as on select to fooview_toast do instead
select 1 as x, 'aaa'::text as y;
-select * from fooview;
-select xmin, * from fooview; -- fail, views don't have such a column
+select * from fooview_toast;
+select xmin, * from fooview_toast; -- fail, views don't have such a column
-drop view fooview;
+BEGIN;
+create table fooview_toast_onetx (x int, y text);
+
+create rule "_RETURN" as on select to fooview_toast_onetx do instead
+ select 1 as x, 'aaa'::text as y;
+COMMIT;
+
+select * from fooview_toast_onetx;
+
+create table fooview_notoast (x int);
+create rule "_RETURN" as on select to fooview_notoast do instead
+ select 1 as x;
+
+select * from fooview_notoast;
+
+drop view fooview_toast;
+drop view fooview_toast_onetx;
+drop view fooview_notoast;
--
-- check for planner problems with complex inherited UPDATES
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
