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

On Wed, Jul 8, 2026 at 10:39 AM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> We can remove assign_recovery_target_name()?

Yes, done in v9. recovery_target_name now stores directly into
recoveryTargetName, so recovery_target_name_string and the assign hook
are both gone.

On Tue, Jul 7, 2026 at 3:59 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> FWIW, I think that this is redundant, as the errmsg and the errdetail
> are basically saying the same thing. I'd suggest a simpler:
> errmsg: cannot specify more than one recovery target
> errdetail: Parameters set are: %s.

Applied in v9.

> I smell of an AI set of comments. We could just remove the whole and
> not lose value in understanding the meaning of the test. A bunch of
> the comments added to the TAP script could also be trimmed down quite
> a bit, made simpler..

Done, and I trimmed the rest of the TAP comments too.

> With recovery_target assign hook being removed for the case of
> immediate, a test case that checks for a conflict between immediate
> and a secondary target may be in order.

Added in v9.

Thanks to you both for the reviews.

--
JH Shin