Hi,
While this thread was effectively superseded by the 'new design' thread
[1], I'd like to address a few points raised here, as they are relevant
for the new design (at least I believe so).
[1] https://www.postgresql.org/message-id/31041.1465069...@sss.pgh.pa.us
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. Allow the relcache to return
a pointer to the list in-cache, and require the planner to copy that
data before making any more cache requests. The planner would run
through the list, skipping single-key entries and entries leading to
irrelevant tables, and copy out just the items that are useful for
the current query (probably adding query-specific table RTE indexes
at the same time).
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?
* Personally I'd probably handle the "ignore irrelevant tables" test
with a list_member_oid test on a list of relation OIDs, not a
hashtable. It's unlikely that there are enough tables in the query to
justify building a hashtable.
OK
* All of the above should happen only if the query contains multiple
tables; there is no reason to expend even one cycle on loading FK data
in a simple single-table query.
OK
... snip the part about nested loops (new design thread) ...
Also, there are serious bugs remaining, even without considering planning
speed. An example I just noticed is that quals_match_foreign_key actually
fails entirely to ensure that it's found a match to the FK, because there
is no check on whether foreignrel has anything to do with the FK. That
is, this bit:
* We make no attempt in checking that this foreign key actually
* references 'foreignrel', the reasoning here is that we may be able
* to match the foreign key to an eclass member Var of a RestrictInfo
* that's in qualslist, this Var may belong to some other relation.
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;
2) without ECs (line 4019)
if ((foreignrel->relid == leftvar->varno) &&
(fkrel->relid == rightvar->varno) &&
(fkinfo->confkeys[i] == leftvar->varattno) &&
(fkinfo->conkeys[i] == rightvar->varattno))
{
...
}
else if ((foreignrel->relid == rightvar->varno) &&
(fkrel->relid == leftvar->varno) &&
(fkinfo->confkeys[i] == rightvar->varattno) &&
(fkinfo->conkeys[i] == leftvar->varattno))
{
...
}
Or does that fail for some reason?
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
