| From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
|---|---|
| To: | Matheus Alcantara <matheusssilv97(at)gmail(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, rmt(at)lists(dot)postgresql(dot)org |
| Subject: | Re: RI fastpath misses checking EXECUTE on functions |
| Date: | 2026-10-04 23:37:54 |
| Message-ID: | CA+HiwqFaipkMsZ8XP-9sMk21h0Q7rih=NpJQXsBAaD39VtnrOA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Sat, Sep 26, 2026 at 11:15 AM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Fri, Sep 25, 2026 at 8:45 PM Matheus Alcantara
> <matheusssilv97(at)gmail(dot)com> wrote:
> > On 25/09/26 05:16, Amit Langote wrote:
> > >> ri_CheckFunctionPermissions(riinfo, fpmeta) passes both when fpmeta
> > >> == riinfo->fpmeta. I'm wondering if we could just pass riinfo?
> > >
> > > That's just for consistency with build_index_scankeys(); it isn't
> > > needed, so I don't feel strongly either way.
> > >
> >
> > Ok, make sense.
> >
> > >> IIUC this patch only fix the case for FastPath without batching right?
> > >> Since batching is still on master, I'm wondering if we could also fix
> > >> it. See attached patch (v2-0001 is your v1-0001).
> > >
> > > I've left the batch code alone because I intend to revert it from
> > > master too sometime next week. Thanks for the patch, though.
> > >
> >
> > Ok, thanks for letting me know.
> >
> > > I have attached a new version where I polished
> > > ri_CheckFunctionPermissions()'s comment and the commit message. I
> > > would like to commit it tomorrow if there are no more comments.
> > >
> >
> > Looks good to me.
>
> Thanks, pushed.
A review of the fast-path code using Opus 5.5 found a potential NULL
de-reference after this commit. ri_FastPathCheck() reads
riinfo->fpmeta again after ri_CheckFunctionPermissions(), whose
catalog lookups can process an invalidation that detaches the metadata
and sets riinfo->fpmeta to NULL.
The attached patch keeps a local pointer, as
InvalidateConstraintCacheCallBack() already assumes callers do. It
applies to both master and REL_19_STABLE. Also, there's no batching
code anymore to keep in sync as of last Saturday [1].
--
Thanks, Amit Langote
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Fix-possible-crash-when-RI-fast-path-metadata-is-.patch | application/octet-stream | 2.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Manu | 2026-10-04 23:42:28 | Re: doc: Document Linux cgroup memory limits |
| Previous Message | Manu | 2026-10-04 23:27:26 | Re: Planning time quadratic in the IN-list length for "c = X AND (a, b) IN (...)" with BitmapOr |