On Sat, Apr 30, 2016 at 1:35 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Julien Rouhaud <julien.rouh...@dalibo.com> writes: >> On 29/04/2016 18:05, Tom Lane wrote: >>> Julien Rouhaud <julien.rouh...@dalibo.com> writes: >>>> The segfault is caused by quals_match_foreign_key() calling get_leftop() >>>> and get_rightop() on a ScalarArrayOpExpr node. >>>> >>>> I'm not sure that assuming this compatibility is the right way to fix >>>> this though. > >>> It certainly isn't. > >> Agreed. New attached patch handles explicitly each node tag. > > No, this is completely nuts. The problem is that quals_match_foreign_key > is simply assuming that every clause in the list it's given is an OpExpr, > which is quite wrong. It should just ignore non-OpExpr quals, since it > cannot do anything useful with them anyway. There's a comment claiming > that non-OpExpr quals were already rejected: > > * Here since 'usefulquals' only contains bitmap indexes for quals > * of type "var op var" we can safely skip checking this. > > but that doesn't appear to have anything to do with current reality. > > While this in itself is about a two-line fix, after reviewing > 137805f89acb3611 I'm pretty unhappy that it got committed at all. > I think this obvious bug is a good sign that it wasn't ready. > Other unfinished aspects like invention of an undocumented GUC > don't leave a good impression either. > > Moreover, it looks to me like this will add quite a lot of overhead, > probably far more than is justified, because clauselist_join_selectivity > is basically O(N^2) in the relation-footprint of the current join --- and > not with a real small multiplier, either, as the functions it calls > contain about four levels of nested loops themselves. Maybe that's > unmeasurable on trivial test cases but it's going to be disastrous in > large queries, or for relations with large numbers of foreign keys. > > I think this should be reverted and pushed out to 9.7 development. > It needs a significant amount of rewriting to fix the performance > issue, and now's not the time to be doing that.
If this gets reverted, then probably 015e88942aa50f0d419ddac00e63bb06d6e62e86 should also be reverted. We need to make some decisions here PDQ. I haven't had time to look at this issue in any technical detail yet. Simon, anyone else, please weigh in. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers