On Tue, Sep 22, 2026 at 8:26 PM Amit Langote <[email protected]> wrote: > On Thu, Jul 16, 2026 at 8:32 PM Amit Langote <[email protected]> wrote: > > On Mon, Jul 6, 2026 at 6:05 AM Noah Misch <[email protected]> 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
