On 2018/04/11 6:32, Alvaro Herrera wrote: > Robert Haas wrote: >> On Mon, Apr 9, 2018 at 2:28 PM, Alvaro Herrera <alvhe...@alvh.no-ip.org> >> wrote: > >>>> I don't get this. The executor surely had to (and did) open all of >>>> the relations somewhere even before this patch. > >>> I was worried that this coding could be seen as breaking modularity, or >>> trying to do excessive work. However, after looking closer at it, it >>> doesn't really look like it's the case. So, nevermind. >> >> Well what I'm saying is that it shouldn't be necessary. If the >> relations are being opened already and the pointers to the relcache >> entries are being saved someplace, you shouldn't need to re-open them >> elsewhere to get pointers to the relcache entries. > > I looked a bit more into this. It turns out that we have indeed opened > the relation before -- first in parserOpenTable (for addRangeTableEntry), > then in expandRTE, then in QueryRewrite, then in subquery_planner, then > in get_relation_info. > > So, frankly, since each module thinks it's okay to open it every once in > a while, I'm not sure we should be terribly stressed about doing it once > more for partition pruning. Particularly since communicating the > pointer seems to be quite troublesome.
Maybe, Robert was saying somewhere in the executor itself, before ExecInitAppend, or more precisely, ExecSetupPartitionPruneState is called. But that doesn't seem to be the case. For the result relation case (insert/update/delete on a partitioned table), we don't need to do extra relation_opens, as InitPlan opens the needed relations when building the ResultRelInfo(s), from where they're later accessed, for example, in ExecInitModifyTable. However, in the Append/MergeAppend cases, we don't, at any earlier point in the executor, open the partitioned tables. InitPlan doesn't touch them. In ExecInitAppend, ExecLockNonLeafAppendTables that we call before calling ExecSetupPartitionPruneState does not open, just locks them using LockRelationOid. So, relation_open on partitioned tables in ExecSetupPartitionPruneState seem to be the first time they're opened *in the executor*. Thanks, Amit