Re: [PATCH] Don't call ereport(ERROR) from recovery target GUC assign hooks

On Fri, Apr 24, 2026 at 10:08:04PM +0900, Fujii Masao wrote:
> In the master, when the following two recovery targets are specified,
> the recovery target assign hook detects that multiple targets were given
> and reports an error. With the patch, however, the same settings do not
> raise an error, recoveryTarget is set to RECOVERY_TARGET_UNSET, and
> recovery unexpectedly proceeds with no target. Could this be a bug
> in the patch?
>
>     recovery_target_xid = '9999'
>     recovery_target_time = ''

Don't think so.  You are specifying for recovery_target_time the same
thing as the default, as in "I don't know and do nothing about the
time".  Why would it matter to make the difference between a default
value set and what's stored by default if nothing is set in this case?

FWIW, I am wondering if we should seriously consider this stuff as
candidate for a backpatch because this is a design mistake: we should
never *ever* rely on the GUC hooks to do cross-checks of multiple
values, f2cbffc7a618 deciding that it was a right thing to do.  It's
not.  The risk of breaking something may not justify that a backpatch.

+1 for reworking that on HEAD, at least.
--
Michael