On Tue, Apr 07, 2020 at 08:40:30AM -0400, James Coleman wrote: > On Tue, Apr 7, 2020 at 12:25 AM Justin Pryzby <pry...@telsasoft.com> wrote: > > On Mon, Apr 06, 2020 at 09:57:22PM +0200, Tomas Vondra wrote: > > > I've pushed the fist part of this patch series - I've reorganized it a > > > > I scanned through this again post-commit. Find attached some suggestions. > > > > Shouldn't non-text explain output always show both disk *and* mem, including > > zeros ? > > Could you give more context on this? Is there a standard to follow? > Regular sort nodes only ever report one type, so there's not a good > parallel there.
The change I proposed was like: @@ -2829,7 +2829,6 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo, ExplainPropertyList("Sort Methods Used", methodNames, es); - if (groupInfo->maxMemorySpaceUsed > 0) { long avgSpace = groupInfo->totalMemorySpaceUsed / groupInfo->groupCount; const char *spaceTypeName; ... - if (groupInfo->maxDiskSpaceUsed > 0) { ... To show in non-text format *both* disk and memory space used, even if zero. I still think that's what's desirable. If it's important to show *whether* a sort space was used, then I think there should be a boolean, or an int 0/1. But I don't think it's actually needed, since someone parsing the explain output could just check |if _dict['Peak Sort Space Used'] > 0: ... the same as you're doing, without having to write some variation on: |if 'Peak Sort Space Used' in _dict and _dict['Peak Sort Space Used'] > 0: ... -- Justin