| From: | Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> |
|---|---|
| To: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
| Cc: | Álvaro Herrera <alvherre(at)kurilemu(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: [PATCH] refint: Avoid reusing cascade UPDATE plans. |
| Date: | 2026-05-15 18:33:35 |
| Message-ID: | CAJTYsWWx9h7OyzakZVyCCQG0xBW6iRyYG=YV0ADnOWg4W-FV+A@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Fri, 15 May 2026 at 22:57, Nathan Bossart <nathandbossart(at)gmail(dot)com>
wrote:
> On Fri, May 15, 2026 at 08:34:55PM +0530, Ayush Tiwari wrote:
> > On Fri, 15 May 2026 at 20:16, Nathan Bossart <nathandbossart(at)gmail(dot)com>
> > wrote:
> >> Big +1 for removing it in v20. Or maybe even v19. I do think it is
> worth
> >> trying to fix the more egregious bugs, though, as the code will still be
> >> supported for a few years.
> >
> > I agree on the removal part.
>
> Cool. Do you want to write the patch for that, too? I think we should aim
> for removing it shortly after v20 opens for development. It might be best
> to move that to its own thread so that folks are aware and have a chance to
> object.
>
Yes, I would love to work on it. I'll start a new thread in a few days for
v20.
>
> > In the meantime, since the module will still be
> > supported for a while, this seems like a focused fix for the more
> > egregious cache behavior.
> >
> > Attached v3 removes the private SPI plan cache from refint entirely.
> > Both check_primary_key() and check_foreign_key() now prepare their SPI
> > plans per trigger invocation and let SPI_finish() release them.
>
> I'm not sure I'd bother with the tests. At a glance the rest looks
> generally reasonable. I realize that leaving the braces around the plan
> preparation logic keeps the patch small, but we probably wouldn't write it
> that way today. I'd suggest moving the local variable declarations to the
> top of the function in your patch, and then doing a follow-up patch to
> re-pgindent the file. Also, I don't think we need any of the comments
> about the historical usage of a cache; let's just clean house.
>
>
Thanks for looking. Attached v4 drops the regression-test additions,
removes the historical cache comments, and moves the new local variable
declarations to the tops of the functions.
I've split the pgindent result into a second patch.
Regards,
Ayush
| Attachment | Content-Type | Size |
|---|---|---|
| v4-0001-refint-Remove-private-SPI-plan-cache.patch | application/octet-stream | 9.1 KB |
| v4-0002-refint-Reindent-after-removing-plan-cache.patch | application/octet-stream | 7.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Isaac Morland | 2026-05-15 18:36:58 | Order of tables dumped by pg_dump |
| Previous Message | Mohamed ALi | 2026-05-15 18:30:24 | [PATCH] doc: Document that invalid indexes are skipped during ATTACH PARTITION |