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

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