On Fri, Aug 25, 2017 at 5:12 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Robert Haas <robertmh...@gmail.com> writes: >> On Fri, Aug 25, 2017 at 2:54 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: >>> Hmm, I'm not sure why SortInstrumentation belongs naturally to >>> tuplesort.h but putting an array of them there would be a "gross >>> abstraction violation". Perhaps it would help to rename >>> struct SharedSortInfo to SortInstrumentationArray, and change its >>> field names to be less specific to the parallel-worker use case? > >> What other use case could there be? I think an array of >> SortInstrumentation objects intended to be stored in DSM is fairly >> clearly a bit of executor-specific machinery and thus properly >> declared along with the node that contains it. > > I'm not really convinced, but it's not worth arguing further. > > Here's a reviewed version of the second patch. I fixed one bug > (wrong explain group nesting) and made some additional cosmetic > improvements beyond the struct relocation.
The capitalization of TupleSortInstrumentation is inconsistent with TuplesortSpaceType, TuplesortMethod, and Tuplesortstate; I recommend we lower-case the S. - * Ordinary plan nodes won't do anything here, but parallel-aware plan - * nodes may need to initialize shared state in the DSM before parallel - * workers are available. They can allocate the space they previously + * Most plan nodes won't do anything here, but plan nodes that allocated + * DSM may need to initialize shared state in the DSM before parallel + * workers are launched. They can allocate the space they previously On reading this again, it seems to me that the first instance of "allocated" in this comment block needs to be changed to "estimated", because otherwise saying they can allocated it later on is unintelligible. Other than that, LGTM. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
