Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit f733219

Browse files
committed
Disallow converting an inheritance child table to a view.
Generally, members of inheritance trees must be plain tables (or, in more recent versions, foreign tables). ALTER TABLE INHERIT rejects creating an inheritance relationship that has a view at either end. When DefineQueryRewrite attempts to convert a relation to a view, it already had checks prohibiting doing so for partitioning parents or children as well as traditional-inheritance parents ... but it neglected to check that a traditional-inheritance child wasn't being converted. Since the planner assumes that any inheritance child is a table, this led to making plans that tried to do a physical scan on a view, causing failures (or even crashes, in recent versions). One could imagine trying to support such a case by expanding the view normally, but since the rewriter runs before the planner does inheritance expansion, it would take some very fundamental refactoring to make that possible. There are probably a lot of other parts of the system that don't cope well with such a situation, too. For now, just forbid it. Per bug #16856 from Yang Lin. Back-patch to all supported branches. (In versions before v10, this includes back-patching the portion of commit 501ed02 that added has_superclass(). Perhaps the lack of that infrastructure partially explains the missing check.) Discussion: https://postgr.es/m/16856-0363e05c6e1612fd@postgresql.org
1 parent 9b0ce89 commit f733219

File tree

4 files changed

+52
-16
lines changed

4 files changed

+52
-16
lines changed

src/backend/catalog/pg_inherits.c

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
* pg_inherits.c
44
* routines to support manipulation of the pg_inherits relation
55
*
6-
* Note: currently, this module only contains inquiry functions; the actual
7-
* creation and deletion of pg_inherits entries is done in tablecmds.c.
6+
* Note: currently, this module mostly contains inquiry functions; actual
7+
* creation and deletion of pg_inherits entries is mostly done in tablecmds.c.
88
* Perhaps someday that code should be moved here, but it'd have to be
99
* disentangled from other stuff such as pg_depend updates.
1010
*
@@ -272,9 +272,11 @@ has_subclass(Oid relationId)
272272
}
273273

274274
/*
275-
* has_superclass - does this relation inherit from another? The caller
276-
* should hold a lock on the given relation so that it can't be concurrently
277-
* added to or removed from an inheritance hierarchy.
275+
* has_superclass - does this relation inherit from another?
276+
*
277+
* Unlike has_subclass, this can be relied on to give an accurate answer.
278+
* However, the caller must hold a lock on the given relation so that it
279+
* can't be concurrently added to or removed from an inheritance hierarchy.
278280
*/
279281
bool
280282
has_superclass(Oid relationId)

src/backend/rewrite/rewriteDefine.c

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "catalog/indexing.h"
2727
#include "catalog/namespace.h"
2828
#include "catalog/objectaccess.h"
29+
#include "catalog/pg_inherits.h"
2930
#include "catalog/pg_rewrite.h"
3031
#include "catalog/storage.h"
3132
#include "commands/policy.h"
@@ -413,13 +414,14 @@ DefineQueryRewrite(const char *rulename,
413414
* Are we converting a relation to a view?
414415
*
415416
* If so, check that the relation is empty because the storage for the
416-
* relation is going to be deleted. Also insist that the rel not have
417-
* any triggers, indexes, child tables, policies, or RLS enabled.
418-
* (Note: these tests are too strict, because they will reject
419-
* relations that once had such but don't anymore. But we don't
420-
* really care, because this whole business of converting relations to
421-
* views is just a kluge to allow dump/reload of views that
422-
* participate in circular dependencies.)
417+
* relation is going to be deleted. Also insist that the rel not be
418+
* involved in partitioning, nor have any triggers, indexes, child or
419+
* parent tables, RLS policies, or RLS enabled. (Note: some of these
420+
* tests are too strict, because they will reject relations that once
421+
* had such but don't anymore. But we don't really care, because this
422+
* whole business of converting relations to views is just an obsolete
423+
* kluge to allow dump/reload of views that participate in circular
424+
* dependencies.)
423425
*/
424426
if (event_relation->rd_rel->relkind != RELKIND_VIEW &&
425427
event_relation->rd_rel->relkind != RELKIND_MATVIEW)
@@ -434,6 +436,9 @@ DefineQueryRewrite(const char *rulename,
434436
errmsg("cannot convert partitioned table \"%s\" to a view",
435437
RelationGetRelationName(event_relation))));
436438

439+
/* only case left: */
440+
Assert(event_relation->rd_rel->relkind == RELKIND_RELATION);
441+
437442
if (event_relation->rd_rel->relispartition)
438443
ereport(ERROR,
439444
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
@@ -471,6 +476,12 @@ DefineQueryRewrite(const char *rulename,
471476
errmsg("could not convert table \"%s\" to a view because it has child tables",
472477
RelationGetRelationName(event_relation))));
473478

479+
if (has_superclass(RelationGetRelid(event_relation)))
480+
ereport(ERROR,
481+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
482+
errmsg("could not convert table \"%s\" to a view because it has parent tables",
483+
RelationGetRelationName(event_relation))));
484+
474485
if (event_relation->rd_rel->relrowsecurity)
475486
ereport(ERROR,
476487
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),

src/test/regress/expected/rules.out

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2693,16 +2693,27 @@ select reltoastrelid, relkind, relfrozenxid
26932693
(1 row)
26942694

26952695
drop view rules_fooview;
2696-
-- trying to convert a partitioned table to view is not allowed
2696+
-- cannot convert an inheritance parent or child to a view, though
2697+
create table rules_fooview (x int, y text);
2698+
create table rules_fooview_child () inherits (rules_fooview);
2699+
create rule "_RETURN" as on select to rules_fooview do instead
2700+
select 1 as x, 'aaa'::text as y;
2701+
ERROR: could not convert table "rules_fooview" to a view because it has child tables
2702+
create rule "_RETURN" as on select to rules_fooview_child do instead
2703+
select 1 as x, 'aaa'::text as y;
2704+
ERROR: could not convert table "rules_fooview_child" to a view because it has parent tables
2705+
drop table rules_fooview cascade;
2706+
NOTICE: drop cascades to table rules_fooview_child
2707+
-- likewise, converting a partitioned table or partition to view is not allowed
26972708
create table rules_fooview (x int, y text) partition by list (x);
26982709
create rule "_RETURN" as on select to rules_fooview do instead
26992710
select 1 as x, 'aaa'::text as y;
27002711
ERROR: cannot convert partitioned table "rules_fooview" to a view
2701-
-- nor can one convert a partition to view
27022712
create table rules_fooview_part partition of rules_fooview for values in (1);
27032713
create rule "_RETURN" as on select to rules_fooview_part do instead
27042714
select 1 as x, 'aaa'::text as y;
27052715
ERROR: cannot convert partition "rules_fooview_part" to a view
2716+
drop table rules_fooview;
27062717
--
27072718
-- check for planner problems with complex inherited UPDATES
27082719
--

src/test/regress/sql/rules.sql

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -901,16 +901,28 @@ select reltoastrelid, relkind, relfrozenxid
901901

902902
drop view rules_fooview;
903903

904-
-- trying to convert a partitioned table to view is not allowed
904+
-- cannot convert an inheritance parent or child to a view, though
905+
create table rules_fooview (x int, y text);
906+
create table rules_fooview_child () inherits (rules_fooview);
907+
908+
create rule "_RETURN" as on select to rules_fooview do instead
909+
select 1 as x, 'aaa'::text as y;
910+
create rule "_RETURN" as on select to rules_fooview_child do instead
911+
select 1 as x, 'aaa'::text as y;
912+
913+
drop table rules_fooview cascade;
914+
915+
-- likewise, converting a partitioned table or partition to view is not allowed
905916
create table rules_fooview (x int, y text) partition by list (x);
906917
create rule "_RETURN" as on select to rules_fooview do instead
907918
select 1 as x, 'aaa'::text as y;
908919

909-
-- nor can one convert a partition to view
910920
create table rules_fooview_part partition of rules_fooview for values in (1);
911921
create rule "_RETURN" as on select to rules_fooview_part do instead
912922
select 1 as x, 'aaa'::text as y;
913923

924+
drop table rules_fooview;
925+
914926
--
915927
-- check for planner problems with complex inherited UPDATES
916928
--

0 commit comments

Comments
 (0)