Re: PG19 FK fast path: OOB write and missed FK checks during batched

On Sat, Aug 22, 2026 at 4:43 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Thu, Aug 20, 2026 at 5:34 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > On Wed, Aug 19, 2026 at 10:12 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > > On Wed, Aug 19, 2026 at 12:09 AM Noah Misch <noah(at)leadboat(dot)com> wrote:
> > > > On Tue, Aug 18, 2026 at 10:42:14PM +0900, Amit Langote wrote:
> > > > > After considering Peter's report of another bug [1] that is fixed by
> > > > > 0001, I tested both his reproducer and nested firing involving the
> > > > > same constraint.
> > > > >
> > > > > The latter exposed a problem in v1: the cache was still keyed only by
> > > > > constraint OID, so a nested check of the same constraint reused the
> > > > > outer entry and did not register a callback at the nested query depth.
> > > > > In v2, the key is now (constraint OID, query depth), giving each
> > > > > firing level its own entry and callback.
> > > > >
> > > > > 0001 now includes regression tests for both cases. 0002 and 0003 are unchanged.
> > > > >
> > > > > I would like to commit these sometime this week and would appreciate a review.
> > > >
> > > > I won't be able to review this. The list should consider it up for grabs.
> > >
> > > Attached is v3, rebased over latest master.
> > >
> > > I reorganized the series to separate firing-state restoration (0001),
> > > per-firing-cycle batch and callback scoping that fixes Peter's report
> > > [1] (0002), and per-subtransaction batch tracking as suggested by Noah
> > > (0003). 0003 also folds in the invariant assertion previously sent
> > > separately. The combined code is otherwise unchanged from v2.
> > >
> > > [1] https://postgr.es/m/CAH2-Wz%3DD533JbF_ak_Pc8kP0FKse-ju8DnMxtjvY%3D%3DyHsP4xgw%40mail.gmail.com
> >
> > I've now pushed 0001 and 0002. Since they fix live bugs, I decided to
> > commit them sooner rather than later.
> >
> > I plan to commit the attached remaining patch tomorrow, barring
> > objections, and then close this item.
>
> Pushed and closed the item.

Ayush Tiwari reported another hole in the per-firing-cycle fix to me
off-list. Here is his reproducer:

CREATE TABLE pk (id int PRIMARY KEY);
INSERT INTO pk VALUES (1);

CREATE TABLE fk (
a int REFERENCES pk (id),
b int CONSTRAINT fk_deferred REFERENCES pk (id)
DEFERRABLE INITIALLY DEFERRED);

CREATE FUNCTION check_now() RETURNS trigger LANGUAGE plpgsql AS $$
BEGIN
BEGIN
SET CONSTRAINTS fk_deferred IMMEDIATE;
EXCEPTION WHEN foreign_key_violation THEN
RAISE NOTICE 'caught by SET CONSTRAINTS';
END;
RETURN NEW;
END$$;

-- Name sorts after the RI trigger, so column a is already batched.
CREATE TRIGGER zz_check_now AFTER INSERT ON fk
FOR EACH ROW EXECUTE FUNCTION check_now();

BEGIN;
INSERT INTO fk VALUES (1, 999);

On the current master with my fixes from last week, an
assertion-enabled build produces:

CREATE TABLE
INSERT 0 1
CREATE TABLE
CREATE FUNCTION
CREATE TRIGGER
BEGIN
WARNING: resource was not closed: relation "pk_pkey"
WARNING: resource was not closed: relation "pk"
WARNING: resource was not closed: TupleDesc 0xffff812677f0 (16392,-1)
WARNING: resource was not closed: TupleDesc 0xffff81270500 (16386,-1)
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost.

The relevant part of the backtrace is:

#3 ExceptionalCondition (conditionName="false",
fileName="../src/backend/utils/adt/ri_triggers.c", lineNumber=4543)
#4 AtEOSubXact_RI (isCommit=true, mySubid=2, parentSubid=1)
at ../src/backend/utils/adt/ri_triggers.c:4543
#5 CommitSubTransaction ()
at ../src/backend/access/transam/xact.c:5247
#6 ReleaseCurrentSubTransaction ()
at ../src/backend/access/transam/xact.c:4836
#7 exec_stmt_block (...)
at ../src/pl/plpgsql/src/pl_exec.c:1859

SET CONSTRAINTS ... IMMEDIATE starts a nested firing cycle without
opening a new query level, so keying the cache by constraint OID and
query depth does not distinguish this cycle from the enclosing one.

I am preparing a patch that uses firing depth, gives the SET
CONSTRAINTS cycle its own callback list, and removes stale entries on
either subtransaction commit or abort. I will post it shortly.

--
Thanks, Amit Langote