Amit Kapila <amit.kapila16@gmail.com> writes:
> On Wed, Apr 12, 2017 at 12:40 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Anyone want to draft a patch for this?
> Please find patch attached based on above discussion.
This patch seems fairly incomplete: you can't just whack around major data
structures like PlannedStmt and PlannerGlobal without doing the janitorial
work of cleaning up support logic such as outfuncs/readfuncs.
Also, when you think about what will happen when doing a copyObject()
on a completed plan, it seems like a pretty bad idea to link subplans
into two independent lists. We'll end up with two separate copies of
those subtrees in places like the plan cache.
I'm inclined to think the other approach of adding a parallel_safe
flag to struct Plan is a better answer in the long run. It's basically
free to have it there because of alignment considerations, and it's
hard to believe that we're not going to need that labeling at some
point in the future anyway.
I had been a bit concerned about having to have some magic in outfuncs
and/or readfuncs to avoid transferring the unsafe subplan(s), but I see
from your patch there's a better way: we can have ExecSerializePlan modify
the subplan list as it transfers it into its private PlannedStmt struct.
But I think iterating over the list and examining each subplan's
parallel_safe marking is a better way to do that.
Will work on this approach.
regards, tom lane