Re: ri_Fast* crash w/ nullable UNIQUE constraint

On Tue, Sep 22, 2026 at 8:26 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Thu, Jul 16, 2026 at 8:32 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > On Mon, Jul 6, 2026 at 6:05 AM Noah Misch <noah(at)leadboat(dot)com> wrote:
> > > I reviewed the ri_Fast* family of commits. This thread covers $SUBJECT and
> > > some other findings. Feel free to fork more threads as needed.
> > > ==== ri_CheckPermissions() does not cover hooks / sepgsql
> > >
> > > > The ri_CheckPermissions() function performs schema USAGE and table
> > > > SELECT checks, matching what the SPI path gets implicitly through
> > > > the executor's permission checks.
> > >
> > > It doesn't call ExecutorCheckPerms_hook or object_access_hook (via
> > > e.g. InvokeFunctionExecuteHook), so sepgsql doesn't get control. That might
> > > be okay if called out in the sepgsql documentation.
> >
> > You're right that the fast path doesn't reach ExecutorCheckPerms_hook
> > or the object access hooks, so sepgsql doesn't get control where it
> > would on the SPI path. Let me think through what restoring that would
> > mean, because I'm not sure it's the right goal.
> >
> > On the SPI path these hooks fired as a consequence of the check
> > running through the executor. For the per-row validation path in
> > particular, that meant a hook invocation per row checked, so a foreign
> > key validation over a large table would have produced an audit record
> > per row. I don't think that was ever an intended sepgsql behavior; it
> > seems more like a side effect of the execution path. Reproducing it
> > deliberately on the fast path doesn't seem necessary IMHO.
> >
> > Invoking the hooks would also mean synthesizing an RTEPermissionInfo
> > list outside any planned query, which is the kind of executor
> > scaffolding the fast path is trying to avoid.
> >
> > Given that, my inclination is to leave it as is rather than wire up
> > the hooks. I'm also unsure a sepgsql doc note is the right place,
> > since it might read as a limitation we intend to close rather than an
> > implementation detail. But I'd rather get your read before deciding.
> > If you or anyone else thinks the bypass is worth addressing or noting
> > somewhere, I'm happy to work out how.
>
> On thinking about this more, I think it was wrong to say that RI
> checks going through ExecutorCheckPerms_hook is an accidental detail.
> These checks do access relations, and anyone who relies on the hook to
> track every relation access, for example, won't see these, because the
> fast path doesn't call it and so diverges from the SPI path there. In
> light of the various recent fixes whose point was to bring the fast
> path's behavior in line with the SPI path's, I'd like to propose
> changing it to call the hook as well. Patch attached. I'll add an open
> item.

Added:

RI fastpath doesn't call ExecutorCheckPerms_hook
Commit: 2da86c1ef9b
Owner: Amit Langote

--
Thanks, Amit Langote