On Tue, Feb 24, 2026 at 01:28:50PM +0900, Fujii Masao wrote:
> I agree with removing the check from the GUC check hook. My only concern is
> that simply removing it (rather than relocating it) changes the error message
> users see when archive_directory is misconfigured, which may make
> troubleshooting WAL archiving failures slightly harder.
>
> [Current]
> WARNING: invalid value for parameter
> "basic_archive.archive_directory": "not_exists"
> DETAIL: Specified archive directory does not exist.
>
> [With the patch]
> ERROR: could not create file
> "not_exists/archtemp.00000001000000000000000E.80107.1771905339058": No
> such file or directory
>
> One option would be to perform the check in check_configured_cb(),
> but as you noted, that would add an extra stat() call for each WAL archiving.
> If that overhead is unacceptable, another approach would be to wrap
> copy_file() in basic_archive_file() with PG_TRY() / PG_CATCH(). On error,
> we could stat() the archive directory and report a clearer reason if it does
> not exist.
>
> That said, if users are generally fine with the "could not create file" error,
> I'm ok with the proposed patch (i.e., just removing the check).
I think it's fine as-is. If something is wrong with the error message,
IMHO we should fix the error message. Adding extra stat() calls to try to
give a nicer message might work most of the time, but there are still race
conditions where users will see the original one. But in any case, I
believe the current message style is used in many places, so I don't see a
strong reason to do anything different here.
--
nathan
