Thread

Commits

  1. Disallow converting an inheritance child table to a view.

  1. BUG #16856: Crash when add "_RETURN" rule on child table

    The Post Office <noreply@postgresql.org> — 2021-02-06T07:46:14Z

    The following bug has been logged on the website:
    
    Bug reference:      16856
    Logged by:          Yang Lin
    Email address:      todoubaba@gmail.com
    PostgreSQL version: 12.5
    Operating system:   ubuntu 20.04
    Description:        
    
    create table parent(a text);
    create table child() inherits (parent);
    create or replace rule "_RETURN" as
        on select
        to child
        do instead
        select *
        from (values('x')) as t(a);
    
    select * from parent;
    
    The psql crash and report:
    server closed the connection unexpectedly
            This probably means the server terminated abnormally
            before or while processing the request.
    The connection to the server was lost. Attempting reset: Failed.
    
    
  2. Re: BUG #16856: Crash when add "_RETURN" rule on child table

    Michael Paquier <michael@paquier.xyz> — 2021-02-06T12:50:24Z

    On Sat, Feb 06, 2021 at 07:46:14AM +0000, PG Bug reporting form wrote:
    > create table parent(a text);
    > create table child() inherits (parent);
    > create or replace rule "_RETURN" as
    >     on select
    >     to child
    >     do instead
    >     select *
    >     from (values('x')) as t(a);
    > 
    > select * from parent;
    
    Yes, reproduced here.  This crashes as the relcache entry of the child
    relation is not getting its rd_tableam initialized.  In 11 and older
    versions we would just have assumed in this case to use heap.  Here is
    the top of the backtrace:
    #0  table_beginscan (rel=0x7f714f574c98, snapshot=0x55e4a884b198,
    nkeys=0, key=0x0) at ../../../src/include/access/tableam.h:860
    860		return rel->rd_tableam->scan_begin(rel, snapshot,
    nkeys, key, NULL, flags);
    (gdb) p rel->rd_tableam
    $1 = (const struct TableAmRoutine *) 0x0
    
    There are two things I can see here:
    1) RelationInitTableAccessMethod() does nothing for a view in 12~.
    Obviously, because it has no physical storage.
    2) RelationGetNumberOfBlocksInFork() now asserts when attempted on a
    view.
    And ~11 actually went through those code paths but silently returned
    0.
    
    Hmm.  Maybe something needs to happen in the rewriter?  Tweaking
    directly SeqNext() would be grotty if we'd add a check for an empty
    rd_tableam.
    
    Also, the same type of issue has existed with currtid(), that got
    fixed by e786be5.  Tom has mentioned not long ago that we should have
    a set of APIs that complain when invoked on relations without storage:
    https://www.postgresql.org/message-id/14846.1586105516@sss.pgh.pa.us
    
    While this would not fix this problem, I think that we should really
    tackle that so as we can prevent crashes if a code path is missed.  I
    can see what needs to be done for that.
    --
    Michael
    
  3. Re: BUG #16856: Crash when add "_RETURN" rule on child table

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-02-06T17:20:25Z

    Michael Paquier <michael@paquier.xyz> writes:
    > On Sat, Feb 06, 2021 at 07:46:14AM +0000, PG Bug reporting form wrote:
    >> create table parent(a text);
    >> create table child() inherits (parent);
    >> create or replace rule "_RETURN" as
    >> on select
    >> to child
    >> do instead
    >> select *
    >> from (values('x')) as t(a);
    >> 
    >> select * from parent;
    
    > Yes, reproduced here.  This crashes as the relcache entry of the child
    > relation is not getting its rd_tableam initialized.
    
    I don't think it's unreasonable for the code to assume that child
    relations are tables and not views.  If you did the equivalent
    
    regression=# create table parent(a text);
    CREATE TABLE
    regression=# create view child as select *
    regression-#     from (values('x')) as t(a);
    CREATE VIEW
    regression=# alter table child inherit parent;
    ERROR:  "child" is not a table or foreign table
    
    or for that matter
    
    regression=# alter table parent inherit child;
    ERROR:  "child" is not a table or foreign table
    
    So my take is that this is an oversight in the CREATE RULE logic
    that allows converting a table to a view: if it has inheritance
    parents or children we must disallow doing that.
    
    I'd be inclined to back-patch further than 12, too.  Even if
    there's not an obvious instant crash in those branches, it's
    unlikely that their behavior is entirely sane.
    
    			regards, tom lane
    
    
    
    
  4. Re: BUG #16856: Crash when add "_RETURN" rule on child table

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-02-06T17:31:01Z

    I wrote:
    > So my take is that this is an oversight in the CREATE RULE logic
    > that allows converting a table to a view: if it has inheritance
    > parents or children we must disallow doing that.
    
    Come to think of it, applying ON INSERT/UPDATE/DELETE rules to
    inheritance tree members is a bit problematic too.  If you do
    "UPDATE parent SET ..." then any ON UPDATE rules on the child
    table are ignored (since the rewriter considers only "parent").
    This probably doesn't lead to any system malfunctions, but
    users might find it surprising.  I bet we don't document that
    adequately.
    
    			regards, tom lane