Yes, It's my fault. You're right.
Pavel Borisov <pashkin.e...@gmail.com> 于2020年11月19日周四 下午11:55写道：
> One thing that doesn't matter is that the modify here seems unnecessary,
>> right?
>>
>> > mdunlinkfork(RelFileNodeBackend rnode, ForkNumber forkNum, bool isRedo)
>> > {
>> > char *path;
>> > - int ret;
>> > + int ret = 0;
>> > path = relpath(rnode, forkNum
>
>
> I suppose it is indeed necessary as otherwise the result of the comparison
> is not defined in case of 'else' block in the mdunlinkfork() :
> 346 else
> 347 {
> 348 /* Prevent other backends' fds from holding on to the disk
> space */
> 349 do_truncate(path);
> .....
> 356 * Delete any additional segments.
> 357 */
> 358 if (ret >= 0)
> ----------^^^^^^^
>
> --
> Best regards,
> Pavel Borisov
>
> Postgres Professional: http://postgrespro.com <http://www.postgrespro.com>
>
So in the present logic, *ret* is always 0 if it is not in recovery mode
(and other *if* conditions are not satisfied). But when the *if* condition
is satisfied, it is possible to skip the deletion of additional segments.
Considering that our goal is to always try to unlink additional segments,
*ret* seems unnecessary here. The code flow looks like:
> if (isRedo || .....)
> {
> int ret; /* move to here */
> ....
> }
> else
> { }
>
> /* Delete any additional segments. */
> if (true)
> ...
Or is there any reason to allow us to skip the attempt to delete additional
segments in recovery mode?
