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
