Re: BUG #16109: Postgres planning time is high across version (Expose buffer usage during planning in EXPLAIN)

On 2020/03/31 10:31, Justin Pryzby wrote:
> On Wed, Jan 29, 2020 at 12:15:59PM +0100, Julien Rouhaud wrote:
>> Rebase due to conflict with 3ec20c7091e97.
>
> This is failing to apply probably since 4a539a25ebfc48329fd656a95f3c1eb2cda38af3.
> Could you rebase? (Also, not sure if this can be set as RFC?)

I updated the patch. Attached.

+/* Compute the difference between two BufferUsage */
+BufferUsage
+ComputeBufferCounters(BufferUsage *start, BufferUsage *stop)

Since BufferUsageAccumDiff() was exported, ComputeBufferCounters() is
no longer necessary. In the patched version, BufferUsageAccumDiff() is
used to calculate the difference of buffer usage.

+ if (es->summary && (planduration || es->buffers))
+ ExplainOpenGroup("Planning", "Planning", true, es);

Isn't it more appropriate to check "bufusage" instead of "es->buffers" here?
The patch changes the code so that "bufusage" is checked.

+ "Planning Time": N.N, +
+ "Shared Hit Blocks": N, +
+ "Shared Read Blocks": N, +
+ "Shared Dirtied Blocks": N,+

Doesn't this indent look strange? IMO no indent for buffer usage is
necessary when the format is either json, xml, and yaml. This looks
better at least for me. OTOH, in text format, it seems better to indent
the buffer usage for more readability. Thought?
The patch changes the code so that "es->indent" is
increment/decrement only when the format is text.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION