I think this patch is mostly OK and decided to use it to test something else. In doing so I noticed one small thing that bothers me: if planning uses buffers, and those were requested to be reported, we get something like this at the end of the explain
Planning: Buffers: shared hit=120 read=30 Planning Memory: used=67944 bytes allocated=73728 bytes Planning Time: 0.892 ms This looks a bit unpleasant. Wouldn't it be much nicer if the Planning thingies were all reported together in the Planning group? Planning: Buffers: shared hit=120 read=30 Memory: used=67944 bytes allocated=73728 bytes Time: 0.892 ms This is easier said than done. First, moving "Time" to be in the same group would break tools that are already parsing the current format. So maybe we should leave that one alone. So that means we end up with this: Planning: Buffers: shared hit=120 read=30 Memory: used=67944 bytes allocated=73728 bytes Planning Time: 0.892 ms Better than the original, I think, even if not perfect. Looking at the code, this is a bit of a mess to implement, because of the way show_buffer_usage is coded; currently it has an internal increment/decrement of indentation, so in order to report memory we would have to move the indentation change to outside show_buffer_usage (I think it does belong outside, but the determination of whether to print or not is quite messy, so we'd need a new function returning boolean). Alternatively we could move the memory reportage to within show_buffer_usage, but that seems worse. Or we could leave it as you have it, but to me that's akin to giving up on doing it nicely. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "I think my standards have lowered enough that now I think 'good design' is when the page doesn't irritate the living f*ck out of me." (JWZ)
