Re: Eliminating SPI / SQL from some RI triggers - take 3
| От | Chao Li |
|---|---|
| Тема | Re: Eliminating SPI / SQL from some RI triggers - take 3 |
| Дата | |
| Msg-id | 77FA04FE-1F84-4DA1-8855-8BBFD8CC889A@gmail.com обсуждение |
| Ответ на | Re: Eliminating SPI / SQL from some RI triggers - take 3 (Amit Langote <amitlangote09@gmail.com>) |
| Ответы |
Re: Eliminating SPI / SQL from some RI triggers - take 3
|
| Список | pgsql-hackers |
> On Mar 30, 2026, at 19:15, Amit Langote <amitlangote09@gmail.com> wrote: > > On Mon, Mar 30, 2026 at 1:55 PM Amit Langote <amitlangote09@gmail.com> 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, Ialso 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/
Вложения
В списке pgsql-hackers по дате отправления: