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