From: Greg Nancarrow <gregn4...@gmail.com> > > (1) > Yes, you're right the wording is not right (and I don't really like > the wording used before the patch). > > Perhaps it could say: > > (Note that we do allow CREATE TABLE AS, INSERT INTO...SELECT, SELECT > INTO, and CREATE MATERIALIZED VIEW to use parallel plans. However, as > of now, other than in the case of INSERT INTO...SELECT, only the leader > backend > writes into a completely new table. In the future, we can extend it to > allow workers for the > other commands to write into the table. However, to allow parallel > updates and deletes, we > have to solve other problems, especially around combo CIDs.)
That looks good to me, thanks. > > (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/. Regards Takayuki Tsunakawa
