Re: Remove useless arguments in ReadCheckpointRecord().

On 2022/07/26 9:42, Kyotaro Horiguchi wrote:
> At Sun, 24 Jul 2022 22:40:16 -0400, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in
>> Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> writes:
>>> On 2022/07/22 17:31, Kyotaro Horiguchi wrote:
>>>> I believed that it is recommended to move to the style not having the
>>>> outmost parens. That style has been introduced by e3a87b4991.
>>
>>> I read the commit log, but I'm not sure yet if it's really recommended to remove extra parens even from the existing calls to errmsg(). Removing extra parens can interfere with back-patching of the changes around those errmsg(), can't it?
>>
>> Right, so I wouldn't be in a hurry to change existing calls. If you're
>> editing an ereport call for some other reason, it's fine to remove the
>> excess parens in it, because you're creating a backpatch hazard there
>> anyway. But otherwise, I think such changes are make-work in themselves
>> and risk creating more make-work for somebody else later.
>
> So, I meant to propose to remove extra parens for errmsg()'s where the
> message string is edited. Is it fine in that criteria?

Even in that case, removing parens may interfere with the back-patch. For example, please imagine the case where wasShutdown is changed to be set to true in the following code and this changed is back-patched to v15. If we modify only the log message in the following errmsg() and leave the parens around that, git cherry-pick of the change of wasShutdown to v15 would be completed successfully. But if we remove the parens, git cherry-pick would fail.

ereport(FATAL,
(errmsg("could not locate required checkpoint record"),
errhint("If you are restoring from a backup, touch \"%s/recovery.signal\" and add required recovery options.\n"
"If you are not restoring from a backup, try removing the file \"%s/backup_label\".\n"
"Be careful: removing \"%s/backup_label\" will result in a corrupt cluster if restoring from a backup.",
DataDir, DataDir, DataDir)));
wasShutdown = false; /* keep compiler quiet */

Regards,

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