On 25 April 2018 at 06:21, Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote: > Andres Freund wrote: > >> What I wonder, after skimming this change, is where the relevant >> expression context is reset? That's not really related to this change >> but the wider thread, I just noticed it while looking at this. > > Do you mean ResetExprContext? We use the plan's context, so it should > occur together with the plan reset itself, i.e. nodeAppend.c should do > it somewhere. > > ... Hmm, it appears we don't do it anywhere there actually.
It's not immediately obvious to me why this is required. All the expressions that are initialized here must live the entire length of the executor run, and since they're allocated in the ExecutorState context they'll be destroyed in FreeExecutorState(). If we do need to call ResetExprContext for these, then we'd just need to invent a teardown for ExecSetupPartitionPruneState which would free off the memory allocated and call ResetExprContext for all non-NULL ExprStates in each context->exprstates. This function would need to be called from the node's End function for any node that's set up a PartitionPruneState. Do we really need to do this given that the memory context these are allocated in will be released a moment later anyway? Just to ensure I'm not dreaming, I ran the following and couldn't see the backend's memory consumption move. create table lp (a int, value int) partition by list(a); create table lp_1 partition of lp for values in(1); create table lp_2 partition of lp for values in(2); create function lp_value(p_a int) returns int as $$ select value from lp where a = p_a; $$ language sql; insert into lp values(1,10),(2,20); select sum(lp_value(x)) from generate_Series(1,2) x, generate_series(1,10000000); -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
