On Wed, Dec 6, 2017 at 12:18 AM, Robert Haas <robertmh...@gmail.com> wrote: > On Tue, Dec 5, 2017 at 2:49 AM, Thomas Munro > <thomas.mu...@enterprisedb.com> wrote: >> As for how to aggregate the information, isn't it reasonable to show >> data from the last loop on the basis that it's representative? >> Summing wouldn't make too much sense, because you didn't use that much >> memory all at once. > > Sorts can be rescanned even without parallel query, so I guess we > should try to make the parallel case kinda like the non-parallel case. > If I'm not wrong, that will just use the stats from the most recent > execution (i.e. the last loop) -- see show_sort_info() in explain.c. >
Right and seeing that I have prepared the patch (posted above [1]) which fixes it such that it will resemble the non-parallel case. Ideally, it would have obviated the need for my previous patch which got committed as 778e78ae. However, now that is committed, I could think of below options: 1. I shall rebase it atop what is committed and actually, I have done that in the attached patch. I have also prepared a regression test case patch just to show the output with and without the patch. 2. For sort node, we can fix it by having some local_info same as shared_info in sort node and copy the shared_info in that or we could reinstate the pointer to the DSM in ExecSortReInitializeDSM() by looking it up in the TOC as suggested by Thomas. If we go this way, then we need a similar fix for hash node as well. [1] - https://www.postgresql.org/message-id/CAA4eK1JBj4YCEQKeTua5%3DBMXy7zW7zNOvoXomzBP%3Dkb_aqMF7w%40mail.gmail.com -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
fix_accum_instr_parallel_workers_v5.patch
Description: Binary data
test_sort_stats_v1.1.patch
Description: Binary data