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
-- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers