On Wed, Apr 14, 2021 at 4:57 PM <e.sokol...@postgrespro.ru> wrote: > > Thank you for working on this issue. Your comments helped me make this > patch more correct. > > > Lines with "colon" format shouldn't use equal signs, and should use two > > spaces > > between fields. > Done. Now extra line looks like "Loop min_rows: %.0f max_rows: %.0f > total_rows: %.0f" or "Loop min_time: %.3f max_time: %.3f min_rows: > %.0f max_rows: %.0f total_rows: %.0f". > > > Since this is now on a separate line, the "if (nloops > 1 && > > es->verbose)" > > can be after the existing "if (es->timing)", and shouldn't need its own > > "if (es->timing)". It should conditionally add a separate line, rather > > than > > duplicating the "(actual.*" line. > > > >> - if (es->timing) > >> + if (nloops > 1 && es->verbose) > New version of patch contains this correction. It helped make the patch > shorter. > > > In non-text mode, think you should not check "nloops > 1". Rather, > > print the > > field as 0. > The fields will not be zeros. New line will almost repeat the line with > main sttistics. > > > I think the labels in non-text format should say "Loop Min Time" or > > similar. > > > > And these variables should have a loop_ prefix like loop_min_t ? > There are good ideas. I changed it. > > I apply new version of this patch. I hope it got better. > Please don't hesitate to share any thoughts on this topic.
The patch does not apply on Head, I'm changing the status to "Waiting for Author": 1 out of 2 hunks FAILED -- saving rejects to file src/test/regress/expected/partition_prune.out.rej patching file src/test/regress/sql/partition_prune.sql Hunk #1 FAILED at 467. Hunk #2 succeeded at 654 (offset -3 lines). 1 out of 2 hunks FAILED -- saving rejects to file src/test/regress/sql/partition_prune.sql.rej Please post a new patch rebased on head. Regards, Vignesh
