David Rowley <david.rowley@2ndquadrant.com> writes:
> On 13 June 2018 at 16:15, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> It seems not to be that bad: we just need a shutdown call for the
>> PartitionPruneState, and then we can remember the open relation there.
>> The attached is based on David's patch from yesterday.
> I've looked over this and it seems better than mine. Especially so
> that you've done things so that the FmgrInfo is copied into a memory
> context that's not about to get reset.
Pushed, thanks for looking it over.
> One small thing is that I'd move the:
> context.partrel = NULL;
> to under:
> /* These are not valid when being called from the planner */
Judgment call I guess. I tend to initialize struct fields in field order
unless there's a good reason to do differently, just so it's easier to
confirm that none were overlooked. But I can see the point of your
suggestion too, so done that way.
There's still one thing I'm a bit confused about here. I noticed that
we weren't actually using the partopfamily and partopcintype fields in
PartitionPruneContext, so I removed them. But that still leaves both
partsupfunc and partcollation as pointers into the relcache that were
subject to this hazard. My testing agrees with lousyjack's results
that both of those were, in fact, being improperly accessed. The OID
comparison effect I mentioned upthread explains why the buildfarm's
cache-clobbering members failed to notice any problem with garbage
partsupfunc data ... but why did we not see any complaints about invalid
collation OIDs? Tis strange.
regards, tom lane