Re: Improve the log message output of basic_archive when basic_archive.archive_directory parameter is not set

> On 13 Oct 2023, at 04:25, Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Tue, Sep 26, 2023 at 08:13:45AM +0200, Daniel Gustafsson wrote:
>>> On 26 Sep 2023, at 00:20, Nathan Bossart <nathandbossart@gmail.com> wrote:
>>>
>>> On Thu, Sep 21, 2023 at 11:18:00AM +0900, bt23nguyent wrote:
>>>> -basic_archive_configured(ArchiveModuleState *state)
>>>> +basic_archive_configured(ArchiveModuleState *state, char **logdetail)
>>>
>>> Could we do something more like GUC_check_errdetail() instead to maintain
>>> backward compatibility with v16?
>>
>> We'd still need something exported to call into which isn't in 16, so it
>> wouldn't be more than optically backwards compatible since a module written for
>> 17 won't compile for 16, or am I missing something?
>
> I only mean that a module written for v16 could continue to be used in v17
> without any changes.  You are right that a module that uses this new
> functionality wouldn't compile for v16.

Sure, but that also means that few if any existing modules will be updated to
provide this =).

> But IMHO the interface is nicer,

That's a more compelling reason IMO.  I'm not sure if I prefer the
GUC_check_errdetail-like approach better, I would for sure not be opposed to
reviewing a version of the patch doing it that way.

Tung Nguyen: are you interested in updating the patch along these lines
suggested by Nathan?

> since module authors wouldn't need to worry about allocating the space
> for the string or formatting the message.

Well, they still need to format it; and calling <new_api>_errdetail(msg),
pstrdup(msg) or psprintf(msg) isn't a world of difference.

--
Daniel Gustafsson