| From: | Haibo Yan <tristan(dot)yim(at)gmail(dot)com> |
|---|---|
| To: | Andres Freund <andres(at)anarazel(dot)de> |
| Cc: | Peter Eisentraut <peter(at)eisentraut(dot)org>, pj(at)illuminatedcomputing(dot)com, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Temporal fkey bugs |
| Date: | 2026-09-11 21:20:35 |
| Message-ID: | CABXr29F23WjnSJ=uSZHJahXsiHJfHKEqQabAi5KxGC5N_M3gaQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, Sep 11, 2026 at 2:07 PM Andres Freund <andres(at)anarazel(dot)de> 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:
Thanks,
Haibo
>
>
> Greetings,
>
> Andres Freund
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | Haibo Yan | 2026-09-11 21:16:02 | [PATCH v1 0/2] Preserve input sign across numeric parameter parsing |