On 2013-02-14 20:47:11 -0500, Tom Lane wrote: > Andres Freund <and...@2ndquadrant.com> 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 (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers