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

On Mon, Apr 13, 2026 at 5:21 PM JoongHyuk Shin <sjh910805(at)gmail(dot)com> wrote:
> The "limited consequences" argument is true enough that this hasn't
> caused visible failures in practice, but fixing a known contract
> violation seems worthwhile.

+1

> The fix is to remove the conflict check from all five assign hooks and
> detect conflicts in validateRecoveryParameters() instead. That
> function already runs after all GUCs have been loaded (called from
> InitWalRecovery() in the startup process), so it can safely read each
> GUC's current value via GetConfigOption() and count how many are
> non-empty. If more than one is set, it reports FATAL, consistent with
> the other validation errors already in that function.

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 = ''

Regards,

--
Fujii Masao