I wrote:
> Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
>> Should this check that we're not in a parallel worker process?
> Hmm. I've not seen any failures in the parallel parts of the regular
> regression tests, but maybe I'd better do a force_parallel_mode
> run before committing.
> In general, I'm not on board with the idea that parallel workers don't
> need to get their own locks, so I don't really want to exclude parallel
> workers from this check. But if it's not safe for that today, fixing it
> is beyond the scope of this particular patch.
So the place where that came out in the wash is the commit I just made
(9a3cebeaa) to change the executor from taking table locks to asserting
that somebody else took them already. To make that work, I had to make
both ExecOpenScanRelation and relation_open skip checking for lock-held
if IsParallelWorker().
This makes me entirely uncomfortable with the idea that parallel workers
can be allowed to not take any locks of their own. There is no basis
for arguing that we have field proof that that's safe, because *up to
now, parallel workers in fact did take their own locks*. And it seems
unsafe on its face, because there's nothing that really guarantees that
the parent process won't go away while children are still running.
(elog(FATAL) seems like a counterexample, for instance.)
I think that we ought to adjust parallel query to insist that children
do take locks, and then revert the IsParallelWorker() exceptions I made
here. I plan to leave that point in abeyance till we've got the rest
of these changes in place, though. The easiest way to do it will
doubtless change once we've centralized the executor's table-opening
logic, so trying to code it right now seems like a waste of effort.
regards, tom lane
