On Monday, July 27, 2015, Dean Rasheed <dean.a.rash...@gmail.com> wrote:
> On 27 July 2015 at 22:36, Joe Conway <m...@joeconway.com <javascript:;>> > wrote: > > -----BEGIN PGP SIGNED MESSAGE----- > > Hash: SHA1 > > > > On 07/27/2015 01:25 PM, Dean Rasheed wrote: > >> Looks good to me, except I'm not sure about those latest changes > >> because I don't understand the reasoning behind the logic in > >> check_enable_rls() when row_security is set to OFF. > >> > >> I would expect that if the current user has permission to bypass > >> RLS, and they have set row_security to OFF, then it should be off > >> for all tables that they have access to, regardless of how they > >> access those tables (directly or through a view). If it really is > >> intentional that RLS remains active when querying through a view > >> not owned by the table's owner, then the other calls to > >> check_enable_rls() should probably be left as they were, since the > >> table might have been updated through such a view and that code > >> can't really tell at that point. > > > > After looking again at those three call sites, I'm now on the fence. > > All three are in functions which are trying to avoid leaking info in > > error messages. If we leave the original coding, then the messages > > will remain the same for a given user regardless of the row_security > > setting, whereas if we change them as in my latest patch, the messages > > will be different depending on row_security for users with BYPASSRLS. > > > > I think if we do the former (original coding) there should probably be > > a comment added to indicate we are doing that intentionally. > > > > Thoughts? > > > > I could go either way on that, but I don't think it's too serious to > worry about leaking information to users with BYPASSRLS. > AFK at the moment, but my thinking was that we should avoid having the error message change based on what a GUC is set to. I agree that there should be comments which explain that. Thanks! Stephen
