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

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.

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

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

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