On Fri, Mar 10, 2017 at 5:43 AM, Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> wrote: > PFA the zip containing all the patches rebased on > 56018bf26eec1a0b4bf20303c98065a8eb1b0c5d and contain the patch to free > memory consumed by paths using a separate path context.
Some very high-level thoughts based on a look through these patches: In 0001, you've removed a comment about how GEQO needs special handling, but it doesn't look as if you've made any compensating change elsewhere. That seems unlikely to be correct. If GEQO needs some paths to survive longer than others, how can it be right for this code to create them all in the same context? Incidentally, geqo_eval() seems to be an existing precedent for the idea of throwing away paths and RelOptInfos, so we might want to use similar code for partitionwise join. 0002 and 0003 look OK. Probably 0004 is OK too, although that seems to be adding some overhead to existing callers for the benefit of new ones. Might be insignificant, though. 0005 looks OK, except that add_join_rel's definition is missing a "static" qualifier. That's not just cosmetic; based on previous expereince, this will break the BF. 0006 seems to be unnecessary; the new function isn't used in later patches. Haven't looked at 0007 yet. 0008 is, as previously mentioned, more than we probably want to commit. Haven't looked at 0009 yet. 0010 - 0012 seem to be various fixes which would need to be done before or along with 0009, rather than afterward, so I am confused about the ordering of those patches in the patch series. The commit message for 0013 is a bit unclear about what it's doing, although I can guess, a bit, based on the commit message for 0007. -- 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
