On Fri, Aug 25, 2017 at 2:54 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: >> I think moving SharedSortInfo into tuplesort.h would be a gross >> abstraction violation, but moving SortInstrumentation into tuplesort.h >> seems like a modest improvement. > > 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. Only nodeSort.c and explain.c need to know anything about it; tuplesort.c is totally ignorant of it and I see no reason why we'd ever want to change that. Indeed, I don't quite see how we could do so without some kind of abstraction violation. SortInstrumentation is a diffferent kettle of fish. I chose to make that an executor-specific bit as well, but an advantage of your proposed approach is that future additions to (or changes in) what gets returned by tuplesort_get_stats() wouldn't require as much tweaking elsewhere. With your proposed change, nodeSort.c wouldn't really care about the contents of that structure (as long as there are no embedded pointers), so we could add new members or rename things and only tuplesort.c and explain.c would need updating. -- 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
