Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> writes: > There's code in add_paths_to_joinrel() which computes the set of > target relations that should overlap parameterization of any proposed > join path. > ... > The calculations that follow are based on joinrel->relids (baserels > covered by the join) and SpecialJoinInfo list in PlannerInfo. It is > not based on specific combination of relations being joined or the > paths being generated. We should probably do this computation once and > store the result in the joinrel and use it multiple times. That way we > can avoid computing the same set again and again for every pair of > joining relations and their order. Any reasons why we don't do this?
I'm not terribly excited about this. The issue is strictly local to add_paths_to_joinrel, but putting that set in a global data structure makes it nonlocal, and makes it that much harder to tweak the algorithm if we think of a better way. (In particular, I think it's not all that obvious that the set must be independent of which two subset relations we are currently joining.) If you can show a measurable performance improvement from this change, then maybe those downsides are acceptable. But I do not think we should commit it without a demonstrated performance benefit from the added complexity and loss of flexibility. > Also, the way this code has been written, the declaration of variable > sjinfo masks the earlier declaration with the same name. I am not sure > if that's intentional, but may be we should use another variable name > for the inner sjinfo. I have not included that change in the patch. Hmm, yeah, that's probably not terribly good coding practice. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers