Re: RI fastpath misses checking EXECUTE on functions

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-30 00:30:37
Message-ID: CA+HiwqEYn8rkB9XgBB5qDNdx1kEuxJ2cAgd1HH=BTy_PmcMEQQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Oct 5, 2026 at 8:37 AM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> 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].

FTR, I pushed the patch on Oct 6. e73e0196d301 is the commit hash.

--
Thanks, Amit Langote

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Takeshi Ideriha 2026-10-30 01:02:52 Re: Return INT_MAX or INT_MIN from strtoint() on overflow
Previous Message Sami Imseih 2026-10-30 00:05:14 Re: [PATCH] pgstat: Add helper to drop entries from a stats kind's hash