Skip to content

Commit a4e8680

Browse files
tglsfdctglsfdc
committed
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.
1 parent f4c4335 commit a4e8680

File tree

5 files changed

+88
-2
lines changed

5 files changed

+88
-2
lines changed

‎src/backend/catalog/heap.c

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1434,6 +1434,47 @@ DeleteAttributeTuples(Oid relid)
14341434
heap_close(attrel, RowExclusiveLock);
14351435
}
14361436

1437+
/*
1438+
* DeleteSystemAttributeTuples
1439+
*
1440+
* Remove pg_attribute rows for system columns of the given relid.
1441+
*
1442+
* Note: this is only used when converting a table to a view. Views don't
1443+
* have system columns, so we should remove them from pg_attribute.
1444+
*/
1445+
void
1446+
DeleteSystemAttributeTuples(Oid relid)
1447+
{
1448+
Relation attrel;
1449+
SysScanDesc scan;
1450+
ScanKeyData key[2];
1451+
HeapTuple atttup;
1452+
1453+
/* Grab an appropriate lock on the pg_attribute relation */
1454+
attrel = heap_open(AttributeRelationId, RowExclusiveLock);
1455+
1456+
/* Use the index to scan only system attributes of the target relation */
1457+
ScanKeyInit(&key[0],
1458+
Anum_pg_attribute_attrelid,
1459+
BTEqualStrategyNumber, F_OIDEQ,
1460+
ObjectIdGetDatum(relid));
1461+
ScanKeyInit(&key[1],
1462+
Anum_pg_attribute_attnum,
1463+
BTLessEqualStrategyNumber, F_INT2LE,
1464+
Int16GetDatum(0));
1465+
1466+
scan = systable_beginscan(attrel, AttributeRelidNumIndexId, true,
1467+
SnapshotNow, 2, key);
1468+
1469+
/* Delete all the matching tuples */
1470+
while ((atttup = systable_getnext(scan)) != NULL)
1471+
simple_heap_delete(attrel, &atttup->t_self);
1472+
1473+
/* Clean up after the scan */
1474+
systable_endscan(scan);
1475+
heap_close(attrel, RowExclusiveLock);
1476+
}
1477+
14371478
/*
14381479
* RemoveAttributeById
14391480
*

‎src/backend/rewrite/rewriteDefine.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "access/htup_details.h"
1919
#include "catalog/catalog.h"
2020
#include "catalog/dependency.h"
21+
#include "catalog/heap.h"
2122
#include "catalog/indexing.h"
2223
#include "catalog/namespace.h"
2324
#include "catalog/objectaccess.h"
@@ -510,13 +511,19 @@ DefineQueryRewrite(char *rulename,
510511
}
511512

512513
/*
513-
* IF the relation is becoming a view, delete the storage files associated
514-
* with it. NB: we had better have AccessExclusiveLock to do this ...
514+
* If the relation is becoming a view, delete the storage files associated
515+
* with it. Also, get rid of any system attribute entries in pg_attribute,
516+
* because a view shouldn't have any of those.
517+
*
518+
* NB: we had better have AccessExclusiveLock to do this ...
515519
*
516520
* XXX what about getting rid of its TOAST table? For now, we don't.
517521
*/
518522
if (RelisBecomingView)
523+
{
519524
RelationDropStorage(event_relation);
525+
DeleteSystemAttributeTuples(event_relid);
526+
}
520527

521528
/* Close rel, but keep lock till commit... */
522529
heap_close(event_relation, NoLock);

‎src/include/catalog/heap.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ extern Node *cookDefault(ParseState *pstate,
107107

108108
extern void DeleteRelationTuple(Oid relid);
109109
extern void DeleteAttributeTuples(Oid relid);
110+
extern void DeleteSystemAttributeTuples(Oid relid);
110111
extern void RemoveAttributeById(Oid relid, AttrNumber attnum);
111112
extern void RemoveAttrDefault(Oid relid, AttrNumber attnum,
112113
DropBehavior behavior, bool complain, bool internal);

‎src/test/regress/expected/rules.out

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1457,6 +1457,28 @@ ERROR: cannot drop rule _RETURN on view fooview because view fooview requires i
14571457
HINT: You can drop view fooview instead.
14581458
drop view fooview;
14591459
--
1460+
-- test conversion of table to view (needed to load some pg_dump files)
1461+
--
1462+
create table fooview (x int, y text);
1463+
select xmin, * from fooview;
1464+
xmin | x | y
1465+
------+---+---
1466+
(0 rows)
1467+
1468+
create rule "_RETURN" as on select to fooview do instead
1469+
select 1 as x, 'aaa'::text as y;
1470+
select * from fooview;
1471+
x | y
1472+
---+-----
1473+
1 | aaa
1474+
(1 row)
1475+
1476+
select xmin, * from fooview; -- fail, views don't have such a column
1477+
ERROR: column "xmin" does not exist
1478+
LINE 1: select xmin, * from fooview;
1479+
^
1480+
drop view fooview;
1481+
--
14601482
-- check for planner problems with complex inherited UPDATES
14611483
--
14621484
create table id (id serial primary key, name text);

‎src/test/regress/sql/rules.sql

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,21 @@ create view fooview as select 'foo'::text;
859859
drop rule "_RETURN" on fooview;
860860
drop view fooview;
861861

862+
--
863+
-- test conversion of table to view (needed to load some pg_dump files)
864+
--
865+
866+
create table fooview (x int, y text);
867+
select xmin, * from fooview;
868+
869+
create rule "_RETURN" as on select to fooview do instead
870+
select 1 as x, 'aaa'::text as y;
871+
872+
select * from fooview;
873+
select xmin, * from fooview; -- fail, views don't have such a column
874+
875+
drop view fooview;
876+
862877
--
863878
-- check for planner problems with complex inherited UPDATES
864879
--

0 commit comments

Comments
 (0)