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@sss.pgh.pa.us> wrote in
>> Fujii Masao <masao.fujii@oss.nttdata.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
existingcalls to errmsg(). Removing extra parens can interfere with back-patching of the changes around those errmsg(),
can'tit?
 
>>
>> 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
wasShutdownis changed to be set to true in the following code and this changed is back-patched to v15. If we modify
onlythe log message in the following errmsg() and leave the parens around that, git cherry-pick of the change of
wasShutdownto 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