Remove logic for converting a table to a view.

Up to now we have allowed manual creation of an ON SELECT rule on
a table to convert it into a view.  That was never anything but a
horrid, error-prone hack though.  pg_dump used to rely on that
behavior to deal with cases involving circular dependencies,
where a dependency loop could be broken by separating the creation
of a view from installation of its ON SELECT rule.  However, we
changed pg_dump to use CREATE OR REPLACE VIEW for that in commit
d8c05af (which was later back-patched as far as 9.4), so there's
not a good argument anymore for continuing to support the behavior.

The proximate reason for axing it now is that we found that the
new statistics code has failure modes associated with the relkind
change caused by this behavior.  We'll patch around that in v15,
but going forward it seems like a better idea to get rid of the
need to support relkind changes.

Discussion: https://postgr.es/m/CALDaNm2yXz+zOtv7y5zBd5WKT8O0Ld3YxikuU3dcyCvxF7gypA@mail.gmail.com

Author: tglsfdc

doc/src/sgml/rules.sgml

@@ -280,28 +280,34 @@
 
 <para>
     Views in <productname>PostgreSQL</productname> are implemented
-    using the rule system. In fact, there is essentially no difference
-    between:
+    using the rule system.  A view is basically an empty table (having no
+    actual storage) with an <literal>ON SELECT DO INSTEAD</literal> rule.
+    Conventionally, that rule is named <literal>_RETURN</literal>.
+    So a view like
 
 <programlisting>
 CREATE VIEW myview AS SELECT * FROM mytab;
 </programlisting>
 
-    compared against the two commands:
+    is very nearly the same thing as
 
 <programlisting>
 CREATE TABLE myview (<replaceable>same column list as mytab</replaceable>);
 CREATE RULE "_RETURN" AS ON SELECT TO myview DO INSTEAD
     SELECT * FROM mytab;
 </programlisting>
 
-    because this is exactly what the <command>CREATE VIEW</command>
-    command does internally.  This has some side effects. One of them
-    is that the information about a view in the
-    <productname>PostgreSQL</productname> system catalogs is exactly
-    the same as it is for a table. So for the parser, there is
-    absolutely no difference between a table and a view. They are the
-    same thing: relations.
+    although you can't actually write that, because tables are not
+    allowed to have <literal>ON SELECT</literal> rules.
+</para>
+
+<para>
+    A view can also have other kinds of <literal>DO INSTEAD</literal>
+    rules, allowing <command>INSERT</command>, <command>UPDATE</command>,
+    or <command>DELETE</command> commands to be performed on the view
+    despite its lack of underlying storage.
+    This is discussed further below, in
+    <xref linkend="rules-views-update"/>.
 </para>
 
 <sect2 id="rules-select">
@@ -1111,7 +1117,7 @@ SELECT word FROM words ORDER BY word &lt;-&gt; 'caterpiler' LIMIT 10;
 <para>
     Rules that are defined on <command>INSERT</command>, <command>UPDATE</command>,
     and <command>DELETE</command> are significantly different from the view rules
-    described in the previous section. First, their <command>CREATE
+    described in the previous sections. First, their <command>CREATE
     RULE</command> command allows more:
 
     <itemizedlist>

src/backend/rewrite/rewriteDefine.c

@@ -239,7 +239,6 @@ DefineQueryRewrite(const char *rulename,
 	Relation	event_relation;
 	ListCell   *l;
 	Query	   *query;
-	bool		RelisBecomingView = false;
 	Oid			ruleId = InvalidOid;
 	ObjectAddress address;
 
@@ -311,7 +310,18 @@ DefineQueryRewrite(const char *rulename,
 		/*
 		 * Rules ON SELECT are restricted to view definitions
 		 *
-		 * So there cannot be INSTEAD NOTHING, ...
+		 * So this had better be a view, ...
+		 */
+		if (event_relation->rd_rel->relkind != RELKIND_VIEW &&
+			event_relation->rd_rel->relkind != RELKIND_MATVIEW)
+			ereport(ERROR,
+					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+					 errmsg("relation \"%s\" cannot have ON SELECT rules",
+							RelationGetRelationName(event_relation)),
+					 errdetail_relkind_not_supported(event_relation->rd_rel->relkind)));
+
+		/*
+		 * ... there cannot be INSTEAD NOTHING, ...
 		 */
 		if (action == NIL)
 			ereport(ERROR,
@@ -407,93 +417,6 @@ DefineQueryRewrite(const char *rulename,
 								ViewSelectRuleName)));
 			rulename = pstrdup(ViewSelectRuleName);
 		}
-
-		/*
-		 * Are we converting a relation to a view?
-		 *
-		 * If so, check that the relation is empty because the storage for the
-		 * relation is going to be deleted.  Also insist that the rel not be
-		 * involved in partitioning, nor have any triggers, indexes, child or
-		 * parent tables, RLS policies, or RLS enabled.  (Note: some of these
-		 * tests are too 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 an obsolete
-		 * kluge to allow dump/reload of views that participate in circular
-		 * dependencies.)
-		 */
-		if (event_relation->rd_rel->relkind != RELKIND_VIEW &&
-			event_relation->rd_rel->relkind != RELKIND_MATVIEW)
-		{
-			TableScanDesc scanDesc;
-			Snapshot	snapshot;
-			TupleTableSlot *slot;
-
-			if (event_relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
-				ereport(ERROR,
-						(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-						 errmsg("cannot convert partitioned table \"%s\" to a view",
-								RelationGetRelationName(event_relation))));
-
-			/* only case left: */
-			Assert(event_relation->rd_rel->relkind == RELKIND_RELATION);
-
-			if (event_relation->rd_rel->relispartition)
-				ereport(ERROR,
-						(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-						 errmsg("cannot convert partition \"%s\" to a view",
-								RelationGetRelationName(event_relation))));
-
-			snapshot = RegisterSnapshot(GetLatestSnapshot());
-			scanDesc = table_beginscan(event_relation, snapshot, 0, NULL);
-			slot = table_slot_create(event_relation, NULL);
-			if (table_scan_getnextslot(scanDesc, ForwardScanDirection, slot))
-				ereport(ERROR,
-						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-						 errmsg("could not convert table \"%s\" to a view because it is not empty",
-								RelationGetRelationName(event_relation))));
-			ExecDropSingleTupleTableSlot(slot);
-			table_endscan(scanDesc);
-			UnregisterSnapshot(snapshot);
-
-			if (event_relation->rd_rel->relhastriggers)
-				ereport(ERROR,
-						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-						 errmsg("could not convert table \"%s\" to a view because it has triggers",
-								RelationGetRelationName(event_relation)),
-						 errhint("In particular, the table cannot be involved in any foreign key relationships.")));
-
-			if (event_relation->rd_rel->relhasindex)
-				ereport(ERROR,
-						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-						 errmsg("could not convert table \"%s\" to a view because it has indexes",
-								RelationGetRelationName(event_relation))));
-
-			if (event_relation->rd_rel->relhassubclass)
-				ereport(ERROR,
-						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-						 errmsg("could not convert table \"%s\" to a view because it has child tables",
-								RelationGetRelationName(event_relation))));
-
-			if (has_superclass(RelationGetRelid(event_relation)))
-				ereport(ERROR,
-						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-						 errmsg("could not convert table \"%s\" to a view because it has parent tables",
-								RelationGetRelationName(event_relation))));
-
-			if (event_relation->rd_rel->relrowsecurity)
-				ereport(ERROR,
-						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-						 errmsg("could not convert table \"%s\" to a view because it has row security enabled",
-								RelationGetRelationName(event_relation))));
-
-			if (relation_has_policies(event_relation))
-				ereport(ERROR,
-						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-						 errmsg("could not convert table \"%s\" to a view because it has row security policies",
-								RelationGetRelationName(event_relation))));
-
-			RelisBecomingView = true;
-		}
 	}
 	else
 	{
@@ -569,94 +492,6 @@ DefineQueryRewrite(const char *rulename,
 		SetRelationRuleStatus(event_relid, true);
 	}
 
-	/* ---------------------------------------------------------------------
-	 * 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 ...
-	 * ---------------------------------------------------------------------
-	 */
-	if (RelisBecomingView)
-	{
-		Relation	relationRelation;
-		Oid			toastrelid;
-		HeapTuple	classTup;
-		Form_pg_class classForm;
-
-		relationRelation = table_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 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->relam = InvalidOid;
-		classForm->reltablespace = InvalidOid;
-		classForm->relpages = 0;
-		classForm->reltuples = -1;
-		classForm->relallvisible = 0;
-		classForm->reltoastrelid = InvalidOid;
-		classForm->relhasindex = false;
-		classForm->relkind = RELKIND_VIEW;
-		classForm->relfrozenxid = InvalidTransactionId;
-		classForm->relminmxid = InvalidMultiXactId;
-		classForm->relreplident = REPLICA_IDENTITY_NOTHING;
-
-		CatalogTupleUpdate(relationRelation, &classTup->t_self, classTup);
-
-		heap_freetuple(classTup);
-		table_close(relationRelation, RowExclusiveLock);
-	}
-
 	ObjectAddressSet(address, RewriteRelationId, ruleId);
 
 	/* Close rel, but keep lock till commit... */

src/test/modules/test_ddl_deparse/expected/create_rule.out

@@ -1,6 +1,8 @@
 ---
 --- CREATE_RULE
 ---
+--- Note: views' ON SELECT rules are tested elsewhere.
+---
 CREATE RULE rule_1 AS
   ON INSERT
   TO datatype_table
@@ -16,12 +18,6 @@ CREATE RULE rule_3 AS
   TO datatype_table
   DO ALSO NOTHING;
 NOTICE:  DDL test: type simple, tag CREATE RULE
-CREATE RULE "_RETURN" AS
-  ON SELECT
-  TO like_datatype_table
-  DO INSTEAD
-    SELECT * FROM datatype_view;
-NOTICE:  DDL test: type simple, tag CREATE RULE
 CREATE RULE rule_3 AS
   ON DELETE
   TO like_datatype_table

src/test/modules/test_ddl_deparse/sql/create_rule.sql

@@ -1,6 +1,8 @@
 ---
 --- CREATE_RULE
 ---
+--- Note: views' ON SELECT rules are tested elsewhere.
+---
 
 
 CREATE RULE rule_1 AS
@@ -18,12 +20,6 @@ CREATE RULE rule_3 AS
   TO datatype_table
   DO ALSO NOTHING;
 
-CREATE RULE "_RETURN" AS
-  ON SELECT
-  TO like_datatype_table
-  DO INSTEAD
-    SELECT * FROM datatype_view;
-
 CREATE RULE rule_3 AS
   ON DELETE
   TO like_datatype_table

src/test/regress/expected/rowsecurity.out

@@ -3942,28 +3942,6 @@ DROP ROLE regress_rls_frank; -- succeeds
 ROLLBACK TO q;
 ROLLBACK; -- cleanup
 --
--- Converting table to view
---
-BEGIN;
-CREATE TABLE t (c int);
-CREATE POLICY p ON t USING (c % 2 = 1);
-ALTER TABLE t ENABLE ROW LEVEL SECURITY;
-SAVEPOINT q;
-CREATE RULE "_RETURN" AS ON SELECT TO t DO INSTEAD
-  SELECT * FROM generate_series(1,5) t0(c); -- fails due to row-level security enabled
-ERROR:  could not convert table "t" to a view because it has row security enabled
-ROLLBACK TO q;
-ALTER TABLE t DISABLE ROW LEVEL SECURITY;
-SAVEPOINT q;
-CREATE RULE "_RETURN" AS ON SELECT TO t DO INSTEAD
-  SELECT * FROM generate_series(1,5) t0(c); -- fails due to policy p on t
-ERROR:  could not convert table "t" to a view because it has row security policies
-ROLLBACK TO q;
-DROP POLICY p ON t;
-CREATE RULE "_RETURN" AS ON SELECT TO t DO INSTEAD
-  SELECT * FROM generate_series(1,5) t0(c); -- succeeds
-ROLLBACK;
---
 -- Policy expression handling
 --
 BEGIN;