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

On Mon, Apr 27, 2026 at 10:52 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> 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?

With those settings, how should recovery behave? I would expect it to
behave as in master, i.e., detect that multiple targets were specified
and report an error. Alternatively, it might be OK for me to proceed
with recovery_target_xid = '9999' and ignore recovery_target_time = '',
since that matches the default.

With the proposed patch, however, both settings are ignored and
recovery starts with no target. That seems unexpected to me.


> +1 for reworking that on HEAD, at least.

I was thinking the same. +1

Regards,

-- 
Fujii Masao