Re: doc: clarify wal_sender_shutdown_timeout behavior for small values
Re: doc: clarify wal_sender_shutdown_timeout behavior for small values
От:
Chao Li <li.evan.chao@gmail.com>
Дата:
> On Jul 17, 2026, at 18:41, Daniel Gustafsson wrote: > >> On 17 Jul 2026, at 12:00, Fujii Masao wrote: > >> This is docs, not error message, but as error message style guide suggests, >> the word "non-negative" basically should be avoided since it's ambiguous >> about whether it accepts zero? > > Yes, "non-negative" should absolutely be avoided. > >> If we document what a value of 0 means, isn't it better to add a brief >> description about the value 0 to postgresql.conf.sample and guc_parameters.dat >> as well? > > I think the bigger picture here is that IMO a user can be forgiven for thinking > at -1 and 0 are equal based on this sentence: > > "A value of -1 (the default) disables the timeout > mechanism" > > To me as a non-native speaker reading that a timeout is disabled can be > interpreted as if there is no waiting done, just like the in the case of 0. +1 > on documenting that 0 means immediate shutdown (though that is sort of implied) > but I think we at the same time should expand the above sentence to clarify > that -1 means waiting. After reading the docs I had to read the code to be > sure I had it right. > > -- > Daniel Gustafsson > Thanks, everyone, for your comments. PFA v3 that attempts to address them all. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
Re: doc: clarify wal_sender_shutdown_timeout behavior for small values
От:
Chao Li <li.evan.chao@gmail.com>
Дата:
> On Aug 5, 2026, at 10:44, Fujii Masao wrote: > > On Wed, Aug 5, 2026 at 9:37 AM Chao Li wrote: >> Thanks for updating the patch. V4 LGTM. > > I've pushed the patch. Thanks! > > Regards, > > -- > Fujii Masao Thank you very much for taking care of this patch. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
Re: doc: clarify wal_sender_shutdown_timeout behavior for small values
От:
Daniel Gustafsson <daniel@yesql.se>
Дата:
> On 17 Jul 2026, at 12:00, Fujii Masao wrote: > This is docs, not error message, but as error message style guide suggests, > the word "non-negative" basically should be avoided since it's ambiguous > about whether it accepts zero? Yes, "non-negative" should absolutely be avoided. > If we document what a value of 0 means, isn't it better to add a brief > description about the value 0 to postgresql.conf.sample and guc_parameters.dat > as well? I think the bigger picture here is that IMO a user can be forgiven for thinking at -1 and 0 are equal based on this sentence: "A value of -1 (the default) disables the timeout mechanism" To me as a non-native speaker reading that a timeout is disabled can be interpreted as if there is no waiting done, just like the in the case of 0. +1 on documenting that 0 means immediate shutdown (though that is sort of implied) but I think we at the same time should expand the above sentence to clarify that -1 means waiting. After reading the docs I had to read the code to be sure I had it right. -- Daniel Gustafsson
Re: doc: clarify wal_sender_shutdown_timeout behavior for small values
От:
Ian Lawrence Barwick <barwick@gmail.com>
Дата:
Hi 2026年7月17日(金) 14:37 Chao Li : > > Hi, > > While reading the doc for wal_sender_shutdown_timeout, I felt that it does not explicitly describe the behavior of a value of 0. According to the implementation, when wal_sender_shutdown_timeout is 0, the WAL sender terminates without waiting for the receiver to catch up by calling WalSndDoneImmediate(). When the value is very small, such as 1 millisecond, the timeout expires shortly afterward if the receiver has not already caught up, so from a user's perspective, the behavior may be very similar to that of 0. > > As I understand it, wal_sender_shutdown_timeout is intended to prevent shutdown from taking unexpectedly long. A value of 0 requests no waiting, while a value that is too small may provide insufficient time for the receiver to catch up. Such values should therefore be used with caution, especially for physical replication. > > The attached patch clarifies that the timeout is enabled for nonnegative values, documents the behavior of 0, and cautions users against setting a value too small for the receiver to catch up. Rather than warn against "too small" values, which raises the question of how you define "too small", I think it'd be better to note that you'd want a value which is sufficiently high to ensure replication terminates properly under normal circumstances (which will vary according to environment etc., so it will be up to the user to work out what is best for them). I took the liberty of revising your patch a bit (easier than trying to describe suggested changes in an email), which: - moves the description of what a value of 0 does to the first paragraph, which handles what various values do - revises the caution about small values to a note about selecting sufficiently high values (per above) - (nitpick) changed "nonnegative" to "non-negative", as that's the majority variant in the docs > While touching this part of the doc, I also noticed that the preceding paragraph was missing one level of indentation and that there was no blank line between paragraphs. I was previously advised that paragraphs should be separated by a blank line, so the patch fixes these formatting issues as well. +1, this matches the established formatting. Regards Ian Barwick
Re: Temporal fkey bugs
От:
Haibo Yan <tristan.yim@gmail.com>
Дата:
On Fri, Sep 11, 2026 at 2:07 PM Andres Freund wrote:
>
> On 2026-09-11 15:23:16 -0400, Andres Freund wrote:
> > Hi,
> >
> > On 2026-09-11 14:51:52 -0400, Andres Freund wrote:
> > > While looking at FOR PORTION OF (see [1]), some AI tool noted that FPO can
> > > lead to issues with temporal foreign keys. I don't think the issues were
> > > really related to FPO, hence this new thread.
> >
> > Grmpf, it also found some stuff in temporal keys:
> >
> > - RelationFindReplTupleByIndex() skips equality checks for
> > primary-key/replica-identity indexes without xs_recheck
> >
> > Temporal keys can use GiST, where a returned candidate need not be an exact
> > match. That can lead to modifying the wrong row, which is ... not good.
> >
> > - The outer constraint scan honors NULLS NOT DISTINCT, but
> > index_recheck_constraint() unconditionally treats an existing NULL as
> > disproving a conflict. That is a problem when needing a recheck.
> >
> > See constraint.sql.
>
> And I just saw that GPT-6 found something additional. I'm too tired to edit
> these into non-AI-ese:
>
>
> 1. Deleted-row conflict detection also trusts lossy identity-index matches
>
> Location: src/backend/executor/execReplication.c:657–694, RelationFindDeletedTupleInfoByIndex().
>
> This repeats the live-row lookup’s shortcut: skip equality checking when using the primary-key/replica-identity index, without honoring xs_recheck.
>
> Verified: with retain_dead_tuples and track_commit_timestamp enabled:
>
> - The target row is missing, and its actual previous deletion has been vacuumed.
> - A different multirange key with the same bounding range is subsequently inserted and deleted.
> - A replicated UPDATE for the missing target reports update_deleted, naming the transaction that deleted the unrelated row.
>
> Changing only that unrelated key to have a different bounding range produces update_missing.
>
> The observed consequence is incorrect conflict classification and deletion metadata. This particular path does not itself modify the wrong row.
>
>
> 2. Parser functional-dependency inference assumes compatible PK equality
>
> Location: src/backend/catalog/pg_constraint.c:1755–1777, check_functional_grouping().
>
> This is an equality-semantics problem rather than missing xs_recheck. The function accepts ungrouped columns whenever the grouping columns contain the PK attributes, without checking the equality semantics.
>
> Using the shipped citext and btree_gist extensions:
>
> CREATE TABLE temporal_grouping (
> k citext COLLATE "C",
> valid_at int4range,
> payload text,
> PRIMARY KEY (k, valid_at WITHOUT OVERLAPS)
> );
>
> The default GiST opclass uses text equality for k, permitting both:
>
> a | [1,5) | first
> A | [1,5) | second
>
> But GROUP BY uses citext equality. This query is nevertheless accepted:
>
> SELECT k, valid_at, payload, count(*)
> FROM temporal_grouping
> GROUP BY k, valid_at;
>
> Observed result:
>
> a | [1,5) | first | 2
>
> An explicit array_agg(payload) shows {first,second} in that group. The
> ungrouped payload is not functionally determined by the actual grouping
> key. Dropping the PK makes the same query correctly fail with the
> ungrouped-column error.
>
Hi Andres,
Regarding the QueueFKConstraintValidation() issue, I independently ran into the
same problem while investigating a PERIOD FK validation bug reported by Jian.
The patch there fixes this by propagating con->conperiod into
NewConstraint.conwithperiod.
The same audit also found two related cases in addFkRecurseReferencing() and
ATExecAlterFKConstrEnforceability(), so the patch fixes all three together and
adds regression coverage for ATTACH PARTITION, VALIDATE CONSTRAINT, and ALTER
CONSTRAINT … ENFORCED.
The patch and discussion are here:
https://www.postgresql.org/message-id/flat/CACJufxHnEu9UfoZsVN2v8FrKopDG+PKCfAGU6fpx7GhGcOa3xg@mail.gmail.com
Thanks,
Haibo
>
>
> Greetings,
>
> Andres Freund
Re: doc: clarify wal_sender_shutdown_timeout behavior for small values
От:
Fujii Masao <masao.fujii@gmail.com>
Дата:
On Fri, Jul 17, 2026 at 6:01 PM Ian Lawrence Barwick wrote: > Rather than warn against "too small" values, which raises the question of how > you define "too small", I think it'd be better to note that you'd want a > value which is sufficiently high to ensure replication terminates properly under > normal circumstances (which will vary according to environment etc., so it will > be up to the user to work out what is best for them). > > I took the liberty of revising your patch a bit (easier than trying to > describe suggested > changes in an email), which: > > - moves the description of what a value of 0 does to the first > paragraph, which handles > what various values do > - revises the caution about small values to a note about selecting sufficiently > high values (per above) > - (nitpick) changed "nonnegative" to "non-negative", as that's the > majority variant in > the docs This is docs, not error message, but as error message style guide suggests, the word "non-negative" basically should be avoided since it's ambiguous about whether it accepts zero? If we document what a value of 0 means, isn't it better to add a brief description about the value 0 to postgresql.conf.sample and guc_parameters.dat as well? Regards, -- Fujii Masao
