Re: pg9.6 segfault using simple query (related to use fk for join estimates) - Mailing list pgsql-hackers

On 06/06/2016 06:15 PM, Tom Lane wrote:
> Tomas Vondra <tomas.vondra@2ndquadrant.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.

I don't follow. How could it have 'nothing to do with the foreign key'? 
We're explicitly checking both varno and varattno on both sides of the 
foreign key, and we only consider the column is considered matched if 
both the checks pass.

I've tried to come up with an example triggering the issue, but 
unsuccessfully ...


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services