Joe: I don't seem to find attachment. Maybe attach again ?
Thanks On Sun, Mar 7, 2021 at 11:12 AM Joe Conway <m...@joeconway.com> wrote: > On 3/3/21 9:43 AM, Joe Conway wrote: > > On 3/3/21 8:50 AM, David Steele wrote: > >> On 1/29/21 4:56 AM, Joe Conway wrote: > >>> On 1/29/21 12:13 AM, Ian Lawrence Barwick wrote: > >>>> 2021年1月28日(木) 17:18 Peter Eisentraut: > >>>> I'm not convinced the current behavior is wrong. Is there some > >>>> practical use case that is affected by this behavior? > >>>> > >>>> I was poking around at the function with a view to using it for > something and was > >>>> curious what it did with bad input. > >>>> > >>>> Providing the column name of a dropped column: > >>>> > >>>> Me: "Hey Postgres, do I have privileges on the dropped column > 'bar' of my > >>>> table 'foo'?" > >>>> Pg: "That column doesn't even exist - here, have an error > instead." > >>>> Me: "Hey Postgres, does some other less-privileged user have > privileges on the > >>>> dropped column 'bar' of my table 'foo'? > >>>> Pg: "That column doesn't even exist - here, have an error > instead." > >>>> > >>>> Providing the attnum of a dropped column: > >>>> > >>>> Me: "Hey Postgres, here's the attnum of the dropped column > 'bar', does some > >>>> other less-privileged user have privileges on that column?" > >>>> Pg: "That column doesn't even exist - here, have a NULL". > >>>> Me: "Hey Postgres, here's the attnum of the dropped column 'bar' > on this table > >>>> I own, do I have privileges on that column?" > >>>> Pg: "Yup. And feel free to throw any other smallint at me, I'll > pretend that > >>>> represents a column too even if it never existed.". > >>>> > >>>> Looking at the code, particularly the cited comment, it does seems > the intent was > >>>> to return NULL in all cases where an invalid attnum was provided, but > that gets > >>>> short-circuited by the assumption table owner = has privilege on any > column. > >>> > >>> Nicely illustrated :-) > >>> > >>>> Not the most urgent or exciting of issues, but seems inconsistent to > me. > >>> > >>> +1 > >> > >> Peter, did Ian's explanation answer your concerns? > >> > >> Joe (or Peter), any interest in reviewing/committing this patch? > > > > Sure, I'll take a look > > Based on Tom's commit here: > > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=3d0f68dd3 > > it appears to me that the intent is to return NULL. > > However I was not to happy with the way the submitted patch added an > argument to > column_privilege_check(). It seems to me that it is better to just reorder > the > checks in column_privilege_check() a bit, as in the attached. > > I wasn't entirely sure it was ok to split up the check for dropped > attribute and > pg_attribute_aclcheck like I did, but when I tested the race condition (by > pausing at pg_attribute_aclcheck and dropping the column in another > session) > everything seemed to work fine. > > Any comments? > > Thanks, > > Joe > > -- > Crunchy Data - http://crunchydata.com > PostgreSQL Support for Secure Enterprises > Consulting, Training, & Open Source Development > > >
