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

Thanks for the reviews.

v2 attached.

* Conflict check moved to a new static CheckRecoveryTargetConflicts(),
called from validateRecoveryParameters() before its early return.
It runs at every startup, so misconfiguration is caught as in master.
I kept it in startup process rather than PostmasterMain
(Greg'ssuggestion),
matching the existing recovery validation there.

* Removed each assign hook's `else recoveryTarget = UNSET` branch
(B in Fujii's framing). Fixes the empty-string clobber Fujii reported,
`recovery_target_xid='9999' + recovery_target_time=''` was silently
running with no target.
003_recovery_targets.pl now covers it (fails on v1, passes on v2).

* errdetail "may" -> "can" (Greg).

* TAP test that asserted the v1 regression is replaced with one
asserting conflict rejection at every startup.

Agreed: HEAD only, no backpatch.

--
JH Shin

On Mon, Apr 27, 2026 at 3:00 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:

> On Mon, Apr 27, 2026 at 02:36:11PM +0900, Fujii Masao wrote:
> > With the proposed patch, however, both settings are ignored and
> > recovery starts with no target. That seems unexpected to me.
>
> If that's the case (not tested myself), agreed.
> --
> Michael
>