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

From: JoongHyuk Shin <sjh910805(at)gmail(dot)com>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH] Don't call ereport(ERROR) from recovery target GUC assign hooks
Date: 2026-05-11 06:01:56
Message-ID: CACSdjfMohHudUh7Fq=pD=Zk+=J=4E65c6VGgScG3CAXvj_Sorg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

CFBot flagged a Windows MSVC failure on v3. Root cause: the new TAP
cases pass GUC values to pg_ctl via "--options" using single-quoted
shell tokens, which Windows cmd.exe does not strip the way POSIX
shells do, so postgres receives the quotes verbatim and rejects the
values.

v4 drops the single quotes and switches recovery_target_time from the
space-containing now() format to ISO 8601 with the T separator, so
each value is a single token without quoting. All other platforms
already passed; this only affected the new TAP cases on Windows MSVC.

No functional change.

--
JH Shin

On Mon, May 11, 2026 at 12:17 PM JoongHyuk Shin <sjh910805(at)gmail(dot)com> wrote:

> Thanks for the reviews.
>
> v3 attached.
>
> Expected behavior matrix:
>
> Same GUC, non-empty then empty -> the empty wins; target unset
> Same GUC, empty then non-empty -> the non-empty wins; target set
> Cross-GUC, both non-empty -> error (multiple recovery targets)
> Cross-GUC, one empty -> the non-empty GUC's target stands
> All empty -> no target, end-of-WAL recovery
>
> * Restored same-GUC last-wins (row 1). v2 dropped each hook's
> `else recoveryTarget = UNSET`; v3 narrows it to
> `else if (recoveryTarget == MY_TYPE)`.
>
> * Cross-GUC empty stays a no-op (row 4), as v2 introduced.
> Strict reject via a source-aware variant is feasible
> if reviewers prefer.
>
> * 003_recovery_targets.pl gains seven CLI-path cases;
> postgresql.conf dedup cannot exercise the same-GUC clear path.
>
> --
> JH Shin
>
> On Fri, May 1, 2026 at 2:53 PM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
>> On Wed, Apr 29, 2026 at 6:30 PM JoongHyuk Shin <sjh910805(at)gmail(dot)com>
>> wrote:
>> >
>> > Thanks for the reviews.
>> >
>> > v2 attached.
>>
>> Thanks for updating the patch!
>>
>> When I started postgres with the following command, recovery_target_xid
>> was
>> treated as unset in the master, but with the patch the
>> recovery_target_xid=700
>> setting was used instead. This behavior seems unexpected to me. Thoughts?
>>
>> postgres -D data -c "recovery_target_xid=700" -c
>> "recovery_target_xid="
>>
>> Regards,
>>
>>
>> --
>> Fujii Masao
>>
>

Attachment Content-Type Size
v4-0001-Don-t-call-ereport-ERROR-from-recovery-target-GUC.patch application/octet-stream 18.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Chao Li 2026-05-11 06:13:27 Fix pg_stat_statements display of normalized FETCH counts
Previous Message solaimurugan vellaipandiyan 2026-05-11 06:01:04 Re: Function scan FDW pushdown