Re: pg9.6 segfault using simple query (related to use fk for join estimates)

Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> writes:
> On 06/04/2016 08:15 PM, Tom Lane wrote:
>> * Make RelationGetFKeyList cache a list of ForeignKeyOptInfo structs,
>> not just constraint OIDs. It's insane that the relcache scans
>> pg_constraint to collect those OIDs and then the planner re-reads all
>> those same rows on every planning cycle.

> That seems like a fairly straightforward change, and I'm not opposed to
> doing that. However RelationGetFKeyList is basically a modified copy of
> RelationGetIndexList, so it shares the same general behavior, including
> caching of OIDs and then constructing IndexOptInfo objects on each
> get_relation_info() call. Why is it 'insane' for foreign keys but not
> for indexes? Or what am I missing?

I would not be in favor of migrating knowledge of IndexOptInfo into the
relcache; it's too planner-specific. Also, it mostly copies info from
the index's relcache entry, not the parent relation's (which for one
thing would imply locking hazards if we tried to cache that info in the
parent rel). But for foreign keys, we can cache an image of certain
well-defined fields of certain well-defined rows of pg_constraint, and
that seems like a reasonably arm's-length definition of a responsibility
to give the relcache, especially when it has to visit all and only those
same rows to construct what it's caching now.

>> would be okay if you checked after identifying a matching eclass member
>> that it belonged to the FK's referenced table, but AFAICS there is no such
>> check anywhere.

> Perhaps I'm missing something, but I thought this is checked by these
> conditions in quals_match_foreign_key():

> 1) with ECs (line 3990)

> if (foreignrel->relid == var->varno &&
> fkinfo->confkeys[i] == var->varattno)
> foundvarmask |= 1;

This checks that you found a joinclause mentioning foreignrel. But
foreignrel need have nothing to do with the foreign key; it could be any
table in the query. That comparison of confkeys[] and varattno is thus
checking for column-number equality of two columns that might be from
different relations. That is, if we have an FK "A.X references B.Y",
and the query contains "A.X = C.Z", this code could match the FK to that
clause if Y and Z happen to have the same data types and column numbers.

regards, tom lane