On Thu, Apr 12, 2018 at 12:40 AM, David Rowley <david.row...@2ndquadrant.com> wrote: > I guess the problem there would be there's nothing to say that parse > analysis will shortly be followed by a call to the planner, and a call > to the planner does not mean the plan is about to be executed. So I > don't think it would be possible to keep pointers to relcache entries > between these modules, and it would be hard to determine whose > responsibility it would be to call relation_close().
Yeah, that's definitely a non-starter. > It might be possible to do something better in each module by keeping > an array indexed by RTI which have each entry NULL initially then on > first relation_open set the element in the array to that pointer. I'm not sure that makes a lot of sense in the planner, but in the executor it might be a good idea. See also https://www.postgresql.org/message-id/CA%2BTgmoYKToP4-adCFFRNrO21OGuH%3Dphx-fiB1dYoqksNYX6YHQ%40mail.gmail.com for related discussion. I think that a coding pattern where we rely on relation_open(..., NoLock) is inherently dangerous -- it's too easy to be wrong about whether the lock is sure to have been taken. It would be much better to open the relation once and hold onto the pointer, not just for performance reasons, but for robustness. BTW, looking at ExecSetupPartitionPruneState: /* * Create a sub memory context which we'll use when making calls to the * query planner's function to determine which partitions will match. The * planner is not too careful about freeing memory, so we'll ensure we * call the function in this context to avoid any memory leaking in the * executor's memory context. */ This is a sloppy cut-and-paste job, not only because somebody changed one copy of the word "planner" to "executor" and left the others untouched, but also because the rationale isn't really correct for the executor anyway, which has memory contexts all over the place and frees them all the time. I don't know whether the context is not needed at all or whether the context is needed but the rationale is different, but I don't buy that explanation. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
