Re: [PATCH] Don't call ereport(ERROR) from recovery target GUC assign hooks - Mailing list pgsql-hackers

From Fujii Masao
Subject Re: [PATCH] Don't call ereport(ERROR) from recovery target GUC assign hooks
Date
Msg-id CAHGQGwExsY9XvxN=u4ip7pA+_gO-ms8EwgcRZF3Nj84Vi8yeBQ@mail.gmail.com
Whole thread
In response to [PATCH] Don't call ereport(ERROR) from recovery target GUC assign hooks  (JoongHyuk Shin <sjh910805@gmail.com>)
List pgsql-hackers
On Mon, Apr 13, 2026 at 5:21 PM JoongHyuk Shin <sjh910805@gmail.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



pgsql-hackers by date:

Previous
From: David Rowley
Date:
Subject: Re: Redundant/mis-use of _(x) gettext macro?
Next
From: Greg Lamberson
Date:
Subject: Re: Comments on Custom RMGRs