On Mon, Jan 25, 2021 at 4:37 PM tsunakawa.ta...@fujitsu.com <tsunakawa.ta...@fujitsu.com> wrote: > > > > > (4) > > You could, but I'm not sure it would make the code easier to read, > > especially for those who don't know !isParallelWorker() means it's a > > parallel leader. > ... > > I really can't see a problem. PrepareParallelMode() is only needed > > prior to execution of a parallel plan, so it's not needed for "other > > call sites" using EnterParallelMode(). > My frank first impressions were (and are): > > * Why do we have to call a separate function for preparation despite the > actual entering follows immediately? We can do necessary preparation in the > entering function. > > * Those who read the parallel index build and parallel VACUUM code for the > first time might be startled at the missing PrepareParallelMode() call: "Oh, > EnterParallelMode() is called without preparation unlike the other site I saw > the other day. Isn't this a but?" > > > > Perhaps the name can be changed to disassociate it from generic > > EnterParallelMode() usage. So far, I've only thought of long names > > like: PrepareParallelModePlanExec(). > > Ideas? > > What PrepareParallelMode() handles is the XID and command ID, which are > managed by access/transam/ module and are not executor-specific. It's > natural (or at least not unnatural) that EnterParallelMode() prepares them, > because EnterParallelMode() is part of access/transam/. > >
EnterParallelMode() is part of a generic interface for execution of a parallel operation, and EnterParallelMode() is called in several different places to enter parallel mode prior to execution of different parallel operations. At the moment it is assumed that EnterParallelMode() just essentially sets a flag to prohibit certain unsafe operations when doing the parallel operation. If I move PrepareParallelMode() into EnterParallelMode() then I need to pass in contextual information to distinguish who the caller is, and possibly extra information needed by that caller - and change the function call for each caller, and probably update the comments for each, and in other places, etc. etc. I think that it just complicates things doing this. The other callers of EnterParallelMode() are obviously currently doing their own "pre" parallel-mode code themselves, specific to whatever parallel operation they are doing - but nobody has thought it necessary to have to hook this code into EnterParallelMode(). I think the "PrepareParallelMode()" name can just be changed to something specific to plan execution, so nobody gets confused with a name like "PrepareParallelMode()", which as you point out sounds generic to all callers of EnterParallelMode(). Regards, Greg Nancarrow Fujitsu Australia
