8000 Disallow converting an inheritance child table to a view. · postgrespro/postgres@dd705a0 · GitHub
[go: up one dir, main page]
Skip to content

Commit dd705a0

Browse files
tglsfdctglsfdc
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 f740082 commit dd705a0

File tree

4 files changed

+52
-16
lines changed
  • test/regress

    • 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
    *
    @@ -277,9 +277,11 @@ has_subclass(Oid relationId)
    277277
    }
    278278

    279279
    /*
    280-
    * has_superclass - does this relation inherit from another? The caller
    281-
    * should hold a lock on the given relation so that it can't be concurrently
    282-
    * added to or removed from an inheritance hierarchy.
    280+
    * has_superclass - does this relation inherit from another?
    281+
    *
    282+
    * Unlike has_subclass, this can be relied on to give an accurate answer.
    283+
    * However, the caller must hold a lock on the given relation so that it
    284+
    * can't be concurrently added to or removed from an inheritance hierarchy.
    283285
    */
    284286
    bool
    285287
    has_superclass(Oid relationId)

    ‎src/backend/rewrite/rewriteDefine.c

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

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

    478+
    if (has_superclass(RelationGetRelid(event_relation)))
    479+
    ereport(ERROR,
    480+
    (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
    481+
    errmsg("could not convert table \"%s\" to a view because it has parent tables",
    482+
    RelationGetRelationName(event_relation))));
    483+
    473484
    if (event_relation->rd_rel->relrowsecurity)
    474485
    ereport(ERROR,
    475486
    (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
    @@ -2801,16 +2801,27 @@ select reltoastrelid, relkind, relfrozenxid
    28012801
    (1 row)
    28022802

    28032803
    drop view rules_fooview;
    2804-
    -- trying to convert a partitioned table to view is not allowed
    2804+
    -- cannot convert an inheritance parent or child to a view, though
    2805+
    create table rules_fooview (x int, y text);
    2806+
    create table rules_fooview_child () inherits (rules_fooview);
    2807+
    create rule "_RETURN" as on select to rules_fooview do instead
    2808+
    select 1 as x, 'aaa'::text as y;
    2809+
    ERROR: could not convert table "rules_fooview" to a view because it has child tables
    2810+
    create rule "_RETURN" as on select to rules_fooview_child do instead
    2811+
    select 1 as x, 'aaa'::text as y;
    2812+
    ERROR: could not convert table "rules_fooview_child" to a view because it has parent tables
    2813+
    drop table rules_fooview cascade;
    2814+
    NOTICE: drop cascades to table rules_fooview_child
    2815+
    -- likewise, converting a partitioned table or partition to view is not allowed
    28052816
    create table rules_fooview (x int, y text) partition by list (x);
    28062817
    create rule "_RETURN" as on select to rules_fooview do instead
    28072818
    select 1 as x, 'aaa'::text as y;
    28082819
    ERROR: cannot convert partitioned table "rules_fooview" to a view
    2809-
    -- nor can one convert a partition to view
    28102820
    create table rules_fooview_part partition of rules_fooview for values in (1);
    28112821
    create rule "_RETURN" as on select to rules_fooview_part do instead
    28122822
    select 1 as x, 'aaa'::text as y;
    28132823
    ERROR: cannot convert partition "rules_fooview_part" to a view
    2824+
    drop table rules_fooview;
    28142825
    --
    28152826
    -- check for planner problems with complex inherited UPDATES
    28162827
    --

    ‎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)
    0