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@paquier.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
>