Re: [PATCH] pg_walsummary: suppress limit output with --quiet

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/