Hi Matheus, On Thu, Sep 24, 2026 at 11:45 PM Matheus Alcantara <[email protected]> wrote: > > On 24/09/26 00:42, Amit Langote wrote: > > Hi, > > > > It was pointed out to me offlist by Nikolay Samokhvalov that the RI > > fast path (ri_FastPathCheck) doesn't check EXECUTE privilege on the > > functions it invokes on the FK values. There are two such functions: > > the equality operator's function, which goes into the ScanKey the > > index AM evaluates, and the implicit cast function, which is used when > > the FK value must be converted to the PK column's type. In the SPI > > path's case, because the operator appears in the generated query's > > WHERE clause and the cast as $1::pktype, ExecutorStart checked EXECUTE > > on both as part of initializing the query, as the referenced table's > > owner. So while I added ri_CheckPermissions() to parallel the > > executor's ExecCheckPermissions(), I missed the non-relation > > permission checks that occur as part of expression initialization. > > > > The attached patch adds ri_CheckFunctionPermissions(), which does > > object_aclcheck(ProcedureRelationId, ..., ACL_EXECUTE) plus > > InvokeFunctionExecuteHook() for each of the two functions, and calls > > it once the fast-path metadata is populated. That puts the checks at > > the same point as the existing relation permission check. > > > > Regression tests are included for both functions. They reach the fast > > path through per-row validation of a new constraint, and repeat each > > case against a partitioned referenced table, which takes the SPI path, > > to establish that the two behave identically. > > > > I'll add an open item. I would like to get this committed by EOW if > > there are no comments. > > I take a look on the patch, it looks good to me. Just a minor > comment:
Thanks for looking. > 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. > 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. 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. -- Thanks, Amit Langote
v2-0001-Check-EXECUTE-privilege-on-functions-invoked-by-t.patch
Description: Binary data
