public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: Amit Langote <[email protected]>
Cc: Junwang Zhao <[email protected]>
Cc: Haibo Yan <[email protected]>
Cc: Pavel Stehule <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Cc: Tomas Vondra <[email protected]>
Subject: Re: Eliminating SPI / SQL from some RI triggers - take 3
Date: Tue, 31 Mar 2026 17:09:19 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+HiwqHCB7kcbspkhaLN9enoj5x=ehzhFM4PXDgWUUP8Px41GA@mail.gmail.com>
References: <CA+HiwqF4C0ws3cO+z5cLkPuvwnAwkSp7sfvgGj3yQ=Li6KNMqA@mail.gmail.com>
<CA+HiwqGM6nvAV5O+=Nr+BXMPWOma0oeCRVzVP0XiLE8zX5TVAg@mail.gmail.com>
<CA+HiwqGMaovCUgDbGxVGnK0Mrivr+ph3YE2Ws+47-ugyPb4f7g@mail.gmail.com>
<CAFj8pRDaiBe_GOLk_yyYHTtPiDAAaLOM8u1-=Q3ZgXBTH+1igg@mail.gmail.com>
<CA+HiwqGA5Ay_MR0eJEEbt4j6WrVh4F+AasTp8yCbs5aJLOJn6Q@mail.gmail.com>
<CAEG8a3JM=NoqiTK0V6S9FNxZPvy1+C5F7rfafTtPKBVWnunL-g@mail.gmail.com>
<CA+HiwqEyiLCY6MTLbOJXDdLNNQLaURYHvdW797MQgbjEK9od4Q@mail.gmail.com>
<CAEG8a3+VBpwPf1Rm-ECD90whM9b3YnGhux5CVXdsL6khiBfzRQ@mail.gmail.com>
<CA+HiwqF2UHzF0sKCp-F2a-U29rqh_9ZPy=f1h+Fh_=M8efj3pg@mail.gmail.com>
<CAEG8a3L9Ew-WL8sxLROVOcypeaENPmd8qCmMvz4geoGL1TDGCA@mail.gmail.com>
<CAEG8a3+nUFQo4sdPQF9xy0J73J8RFJ5U9A5+_kMosGDaZ+1sXA@mail.gmail.com>
<[email protected]>
<CAEG8a3JyKdizWvYsF+z_mA1BKy=dpW11iKVMOG=bk6Tbz6M1Bw@mail.gmail.com>
<CAEG8a3+Hf4tvvbts29_k_AFhWQmRYfEo_SW4C5FY_140iKghBw@mail.gmail.com>
<CA+HiwqFV-PY-3BxM6j5TaAiC3AwedDxo-6vwRSbvygg3zF+xAQ@mail.gmail.com>
<CA+HiwqHpaisS-e+0gAgzh92qZAFxncAJMmmTRZZN=efoeTPgOg@mail.gmail.com>
<CA+HiwqFwZ6WLRbY8R7VC7JVp5Jot6ktZOkr9wDxTjoK=W1SgdQ@mail.gmail.com>
<CA+HiwqF_Kz=R8juHJBiOATvabWSOugK4VaGOxoJ_n=E2c7UM9w@mail.gmail.com>
<CA+HiwqHCB7kcbspkhaLN9enoj5x=ehzhFM4PXDgWUUP8Px41GA@mail.gmail.com>
> On Mar 30, 2026, at 19:15, Amit Langote <[email protected]> wrote:
>
> On Mon, Mar 30, 2026 at 1:55 PM Amit Langote <[email protected]> wrote:
>> Junwang pointed out off-list that FK tuples added to
>> RI_FastPathEntry.batch[] were being copied into TopTransactionContext
>> rather than flush_cxt, so they would accumulate until the batch was
>> exhausted rather than being reclaimed per flush. Fixed in
>> ri_FastPathBatchAdd() in 0002.
>>
>> Also added a couple of comments in trigger.c that were missing: an
>> Assert and explanation in RegisterAfterTriggerBatchCallback()
>> clarifying the query_depth >= 0 precondition, a comment at the
>> AfterTriggerEndQuery call site explaining why
>> FireAfterTriggerBatchCallbacks() must precede the query_depth
>> decrement and AfterTriggerFreeQuery, and brief intent comments at the
>> AfterTriggerFireDeferred and AfterTriggerSetState call sites.
>>
>> Plan is to commit 0001 tomorrow barring objections and let it sit for
>> a bit before committing 0002. Feedback on 0002, particularly on the
>> AfterTriggerBatchCallback mechanism in trigger.c, welcome in the
>> meantime.
>
> Kept looking at 0002 and found a couple of things to improve or change
> my thoughts about. I decided to move the permission check from fast
> path cache entry creation into ri_FastPathBatchFlush(), alongside the
> snapshot, so that permission changes between flushes are respected
> rather than checked once at batch start; the check happens for every
> row in the SPI and non-batched fast path. Also, improved comments in
> a few places to mention design decisions better.
>
> 0001 is mostly unchanged from v11 except I updated its commit message
> to explain why only RI_FKey_check is covered and not the action
> triggers as the topic has come up in previous threads about this
> topic.
>
> Still planning to commit 0001 tomorrow.
>
> --
> Thanks, Amit Langote
> <v12-0001-Add-fast-path-for-foreign-key-constraint-checks.patch><v12-0002-Batch-FK-rows-and-use-SK_SEARCHARRAY-for-fast-pa.patch>
Hi Amit,
While reading the recent commits, I saw that 0001 has been pushed as 2da86c1ef9b5446e0e22c0b6a5846293e58d98e3. However, I also just noticed a use-after-free issue in ri_LoadConstraintInfo(). It dereferences conForm after ReleaseSysCache(tup), which is unsafe. I am attaching a tiny patch to fix that.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v1-0001-Fix-a-use-after-problem-in-ri_LoadConstraintInfo.patch (1007B, 2-v1-0001-Fix-a-use-after-problem-in-ri_LoadConstraintInfo.patch)
download | inline diff:
From 85898f7825631f9a46b057ea486a766484c77f9b Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Tue, 31 Mar 2026 17:06:12 +0800
Subject: [PATCH v1] Fix a use-after-problem in ri_LoadConstraintInfo()
Author: Chao Li <[email protected]>
---
src/backend/utils/adt/ri_triggers.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index da7640a8005..94bb180325b 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -2396,8 +2396,6 @@ ri_LoadConstraintInfo(Oid constraintOid)
&riinfo->period_intersect_oper);
}
- ReleaseSysCache(tup);
-
/*
* For efficient processing of invalidation messages below, we keep a
* doubly-linked count list of all currently valid entries.
@@ -2412,6 +2410,8 @@ ri_LoadConstraintInfo(Oid constraintOid)
riinfo->fpmeta = NULL;
+ ReleaseSysCache(tup);
+
return riinfo;
}
--
2.50.1 (Apple Git-155)
view thread (63+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Eliminating SPI / SQL from some RI triggers - take 3
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox