On Fri, Aug 25, 2017 at 2:01 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Robert Haas <robertmh...@gmail.com> writes: >> On another note, here's a second patch applying on top of my earlier >> patch to push down Limit through Gather and Gather Merge. > > I looked through this a little, and feel uncomfortable with the division > of typedefs between execnodes.h and tuplesort.h. I'm inclined to push > struct SortInstrumentation, and maybe also SharedSortInfo, into > tuplesort.h. Then tuplesort_get_stats could be given the signature > > void tuplesort_get_stats(Tuplesortstate *state, > SortInstrumentation *stats); > > which seems less messy. Thoughts?
I think moving SharedSortInfo into tuplesort.h would be a gross abstraction violation, but moving SortInstrumentation into tuplesort.h seems like a modest improvement. > I'm also pretty suspicious of the filter code you added to subselect.sql. > What happens when there's more than one worker? Uh, then somebody's changed the definition of force_parallel_mode in a really strange way. > (BTW, would it make sense to number the workers from 1 not 0 in the > EXPLAIN printout?) The main problem I see with that is that it might confuse somebody who is debugging. If you've got a backend and print out ParallelWorkerNumber, you might expect that to match what you see in the EXPLAIN output. It also seems likely to me that other parts of EXPLAIN or other parts of the system generally will eventually expose parallel worker numbers in mildly user-visible ways (e.g. maybe it'll show up in a DEBUG message someplace eventually), and if we insist on sticking a + 1 into EXPLAIN's output in the existing places we'll have to do it everywhere else, and somebody's eventually going to forget. So I'm in favor of leaving it alone; I don't think that 0-based indexing is such an obscure convention that it will flummox users. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers