Re: Different compression methods for FPI

On Tue, Sep 06, 2022 at 03:47:05PM +0900, Michael Paquier wrote:
> On Sun, Sep 04, 2022 at 07:23:20PM -0500, Justin Pryzby wrote:
>>          page = BufferGetPage(*buf);
>>          if (!RestoreBlockImage(record, block_id, page))
>> -            elog(ERROR, "failed to restore block image");
>> +            ereport(ERROR,
>> +                    errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
>> +                    errmsg("failed to restore block image"),
>> +                    errdetail("%s", record->errormsg_buf));
>> +
>
> Yes, you are right here.  elog()'s should never be used for things
> that could be triggered by the user, even if this one depends on the
> build options.  I think that the error message ought to be updated as
> "could not restore block image" instead, to be more in line with the
> project policy.

At the end, I have not taken the approach to use errdetail() for this
problem as errormsg_buf is designed to include an error string.  So, I
have finished by refining the error messages generated in
RestoreBlockImage(), consuming them with an ERRCODE_INTERNAL_ERROR.

This approach addresses a second issue, actually, because we have
never provided any context when there are inconsistencies in the
decoded record for max_block_id, has_image or in_use when restoring a
block image.  This one is older than v15, but we have received
complaints about that for ~14 as far as I know, so I would leave this
change for HEAD and REL_15_STABLE.
--
Michael