| From: | Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> |
|---|---|
| To: | Simon Riggs <simon(at)2ndQuadrant(dot)com> |
| Cc: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, David Steele <david(at)pgmasters(dot)net>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: PATCH: use foreign keys to improve join estimates v1 |
| Date: | 2016-04-04 10:03:33 |
| Message-ID: | 57023BF5.4010109@lab.ntt.co.jp |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
Also, it seems the patch forgot to update RelationDestroyRelation().
Thanks,
Amit
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Etsuro Fujita | 2016-04-04 10:49:02 | Re: Optimization for updating foreign tables in Postgres FDW |
| Previous Message | Andres Freund | 2016-04-04 10:01:16 | Re: Timeline following for logical slots |