* Simon Riggs (simon@2ndQuadrant.com) wrote:
> The loadable module doesn't "gain control" here it simplify kicks-in
> after, and in addition to, normal checking. That just means you have the
> option of failing for additional reasons.
Right, additional checks (such as the label) can be done.
> We're not passing in any form of context other than the rangetable so
> what additional reasons could there be? This is of no use to anything
> that uses object labelling. We're not even at the part of the executor
> where we would be able to identify objects yet, so I can't see what
> value this brings.
I'm a bit confused by this. By this point, we've fully planned out the
query, looked up info about all the objects involved, and the module can
now go look up any other information about them that it needs. It can
also use info like what the current user is and information about the
connection.
There was actually a proof-of-concept module created by KaiGai to do all
of this with SELinux using the existing COMMENT tables, I'm pretty sure
we would have heard a bit earlier if it was useless.
> Strangely, I was looking into removing the ExecCheckRTPerms check
> altogether by forcing plan invalidation when permissions are updated.
> That would be a performance tweak that would render this change useless.
I don't see how you could remove ExecCheckRTPerms..? It's what handles
all permissions checking for DML (like, making sure you have SELECT
rights on the table you're trying to query). I could see forcing plan
invalidation when permissions are updated, sure, but that doesn't mean
you can stop doing them altogether anywhere. Where would you move the
permissions checking to? Wherever it is, this hook would just need to
follow.
I don't know that you could (or that I'd be comfortable with) move the
permissions checking to the planner and then rely on plan invalidation
on permission changes, but if you did, just make sure the hook is
included in the decision about allowing the query.
Thanks,
Stephen