Re: PATCH: use foreign keys to improve join estimates v1

Hi,

attached is the patch split into two parts, as proposed by Simon. 0001
just adds the stuff to relcache, 0002 actually uses it for estimation.

On 04/04/2016 12:03 PM, Amit Langote wrote:
> On 2016/04/04 17:25, Simon Riggs wrote:
>> The rel cache code you're adding uses a flag called "rd_fkeyvalid"
>> which indicates that the relcache is correctly filled. That is
>> confusing, since it has nothing to do with the concept of
>> constraint validity. We should rename that to rd_fkeycachefilled or
>> similar.
>
> Maybe I'm missing something, but is a separate bool required at all
> in this case? Wouldn't simply doing the following suffice?
>
> /* Quick exit if we already computed the list. */
> if (relation->rd_fkeylist)
> return list_copy(relation->rd_fkeylist);
>
> ISTM, rd_fkeyvalid is modeled on rd_indexvalid, where the latter
> serves to convey more info than simply whether the index list is
> valid or not, so the extra field is justified.

I think you're right. I've copied the logic for indexes, but clearly for
foreign keys we don't need this flag. I've removed it.

>
> Also, it seems the patch forgot to update RelationDestroyRelation().

Right. Fixed.

regards

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