Fujii-san, Kuroda-san,
Thank you both for reviewing the patch.
> Testing both forms with the same input and expected output seems a bit
> overkill for this fix, since both set the same flag. Isn't testing just
> one of them be sufficient?
> I feel it's enough to call command_checks_all() once, thought?
Agreed. In v2, I kept only the -q test, so command_checks_all() is
called once.
> Isn't this primarily adding test coverage for existing quiet-mode behavior,
> rather than testing the bug being fixed? If so, it might be better to leave
> this out of the bug-fix patch intended for backpatching. Thought?
Agreed. I removed the truncated-file test.
> It might be matter of taste, but how about moving printf() after the
> `if (opt->quiet)`? The code might be clearer.
I moved the limit-block printf() after the quiet check.
> Also, this fix should be backpatched to v17, where pg_walsummary was
> introduced?
Agreed. I tested the updated patch against REL_17_STABLE and
REL_18_STABLE, and the pg_walsummary test suite passes on both.
The patch applies cleanly to REL_18_STABLE. On REL_17_STABLE, the
code change applies cleanly, but the test needs a small context
adjustment because that branch does not contain the pg_stat_io check.
Regards,
Koshi Shibagaki
FUJITSU LIMITED
https://www.fujitsu.com/
