When converting a table to a view, remove its system columns.

Views should not have any pg_attribute entries for system columns.
However, we forgot to remove such entries when converting a table to a
view.  This could lead to crashes later on, if someone attempted to
reference such a column, as reported by Kohei KaiGai.

Patch in HEAD only.  This bug has been there forever, but in the back
branches we will have to defend against existing mis-converted views,
so it doesn't seem worthwhile to change the conversion code too.

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 6edd11fb98679be7ddf138e224dbd43a8e63275d..8818b680c8b62a8d5e9bbd2841a8b6a57ad9eeb9 100644 (file)
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -1434,6 +1434,47 @@ DeleteAttributeTuples(Oid relid)
    heap_close(attrel, RowExclusiveLock);
 }
 
+/*
+ *     DeleteSystemAttributeTuples
+ *
+ * Remove pg_attribute rows for system columns of the given relid.
+ *
+ * Note: this is only used when converting a table to a view.  Views don't
+ * have system columns, so we should remove them from pg_attribute.
+ */
+void
+DeleteSystemAttributeTuples(Oid relid)
+{
+   Relation    attrel;
+   SysScanDesc scan;
+   ScanKeyData key[2];
+   HeapTuple   atttup;
+
+   /* Grab an appropriate lock on the pg_attribute relation */
+   attrel = heap_open(AttributeRelationId, RowExclusiveLock);
+
+   /* Use the index to scan only system attributes of the target relation */
+   ScanKeyInit(&key[0],
+               Anum_pg_attribute_attrelid,
+               BTEqualStrategyNumber, F_OIDEQ,
+               ObjectIdGetDatum(relid));
+   ScanKeyInit(&key[1],
+               Anum_pg_attribute_attnum,
+               BTLessEqualStrategyNumber, F_INT2LE,
+               Int16GetDatum(0));
+
+   scan = systable_beginscan(attrel, AttributeRelidNumIndexId, true,
+                             SnapshotNow, 2, key);
+
+   /* Delete all the matching tuples */
+   while ((atttup = systable_getnext(scan)) != NULL)
+       simple_heap_delete(attrel, &atttup->t_self);
+
+   /* Clean up after the scan */
+   systable_endscan(scan);
+   heap_close(attrel, RowExclusiveLock);
+}
+
 /*
  *     RemoveAttributeById
  *

diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index 8efc9fc9d521b6f72f620d8429a17f7b60036378..55b0fed5f79c8e28fd51ef921fd2a799682cade0 100644 (file)
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -18,6 +18,7 @@
 #include "access/htup_details.h"
 #include "catalog/catalog.h"
 #include "catalog/dependency.h"
+#include "catalog/heap.h"
 #include "catalog/indexing.h"
 #include "catalog/namespace.h"
 #include "catalog/objectaccess.h"
@@ -510,13 +511,19 @@ DefineQueryRewrite(char *rulename,
    }
 
    /*
-    * IF the relation is becoming a view, delete the storage files associated
-    * with it.  NB: we had better have AccessExclusiveLock to do this ...
+    * 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.
+    *
+    * 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)
+   {
        RelationDropStorage(event_relation);
+       DeleteSystemAttributeTuples(event_relid);
+   }
 
    /* Close rel, but keep lock till commit... */
    heap_close(event_relation, NoLock);

diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h
index 1465456cc7d1498d824745d5d500f1f4fac1c17d..a35829bb7b98e733c90d6774c3912295d03e57df 100644 (file)
--- a/src/include/catalog/heap.h
+++ b/src/include/catalog/heap.h
@@ -107,6 +107,7 @@ extern Node *cookDefault(ParseState *pstate,
 
 extern void DeleteRelationTuple(Oid relid);
 extern void DeleteAttributeTuples(Oid relid);
+extern void DeleteSystemAttributeTuples(Oid relid);
 extern void RemoveAttributeById(Oid relid, AttrNumber attnum);
 extern void RemoveAttrDefault(Oid relid, AttrNumber attnum,
                  DropBehavior behavior, bool complain, bool internal);

diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index ad1591be598405c34085d258a40aeb88d98c827c..a235571b3dff00aa0732e62d6caac04a5e28bb71 100644 (file)
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1457,6 +1457,28 @@ ERROR:  cannot drop rule _RETURN on view fooview because view fooview requires i
 HINT:  You can drop view fooview instead.
 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;
+ xmin | x | y 
+------+---+---
+(0 rows)
+
+create rule "_RETURN" as on select to fooview do instead
+  select 1 as x, 'aaa'::text as y;
+select * from fooview;
+ x |  y  
+---+-----
+ 1 | aaa
+(1 row)
+
+select xmin, * from fooview;  -- fail, views don't have such a column
+ERROR:  column "xmin" does not exist
+LINE 1: select xmin, * from fooview;
+               ^
+drop view fooview;
+--
 -- check for planner problems with complex inherited UPDATES
 --
 create table id (id serial primary key, name text);

diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index 1de5b0b68533daf131f65cc16559c9fa895231ad..458c2f026c0fbce861c7c4dc5898b2749b71af55 100644 (file)
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -859,6 +859,21 @@ create view fooview as select 'foo'::text;
 drop rule "_RETURN" on fooview;
 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 rule "_RETURN" as on select to fooview 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
+
+drop view fooview;
+
 --
 -- check for planner problems with complex inherited UPDATES
 --