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
 --