I wrote:
> Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
>> Commit ac2b095088 assumes that clauselist_selectivity is being passed a
>> list of RelOptInfo, but postgres_fdw is passing it a list of bare
>> clauses. One of them is wrong :-)
> It's a bit scary that apparently none of the committed regression tests
> caught that.
Experimentation shows that actually, the standard regression tests
provide dozens of opportunities for find_relation_from_clauses to fail on
non-RestrictInfo input. However, it lacks any IsA check, and the only
thing that it does with the alleged rinfo is if (bms_get_singleton_member(rinfo->clause_relids, &relid))
As long as there's some kind of object pointer where the clause_relids
field would be, it's highly likely that bms_get_singleton_member will just
return false without crashing, thereby obscuring the fault. Andreas'
example kills it by causing the argument to be a Param node, whose field
layout doesn't put a pointer there.
This makes me wonder whether we were being penny-wise and pound-foolish
by not making Bitmapsets be a kind of Node, so that there could be IsA
assertions in the bitmapset.c routines, as there are for Lists. Most
Bitmapsets in a typical backend probably have only one payload word
(ie highest member < 32), so right now they occupy 8 bytes. Adding
a nodetag would kick them up to the next palloc category, 16 bytes,
which is probably why I didn't do it like that to begin with.
Still, that decision is looking unduly byte-miserly in 2017.
regards, tom lane
