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

> * Function structure: the recovery_target_* set has been
> historically stable, so array + loop abstraction adds limited
> value; function size grows ~34% (32 -> 43 lines) for one line of
> savings on a hypothetical sixth GUC, while the closest precedent
> (archive_command / archive_library in pgarch.c) is a hard-coded literal.
>

> * errhint vs errdetail: errhint("At most one of %s can be set.")
> reads more like a constraint than an action hint. The closest
> precedent, archive_command / archive_library in pgarch.c
> (ProcessPgArchInterrupts() / LoadArchiveLibrary()), keeps the
> enumeration in errdetail and omits errhint entirely.
>

> * TAP regex: the added like() uses [^"]+ for the values, which
> passes regardless of the actual value. Using quotemeta on the
> expected values would verify the actual content, and anchoring
> would also avoid accidentally matching the same tokens inside
> errhint.

Thanks for taking a look. I attached a v2 that applies your suggestions
and uses "set to" instead of "=" to match convention. What do you think?

Sample output:

FATAL: multiple recovery targets specified
DETAIL: At most one of "recovery_target", "recovery_target_lsn",
"recovery_target_name", "recovery_target_time",
"recovery_target_xid" can be set. Currently set:
"recovery_target_time" set to "2026-01-01 00:00:00",
"recovery_target_xid" set to "700".

--
Scott Ray