Re: [HACKERS] REFERENCES privilege should not be symmetric (was Re: [GENERAL] Postgres Permissions Article)
"David G. Johnston" writes: > On Fri, Mar 31, 2017 at 10:40 AM, Tom Lane wrote: >> I think the benefit is reduction of user confusion. Admittedly, since >> Paul is the first person I can remember ever having complained about it, >> maybe nobody else is confused. > After going back-and-forth on this (and not being able to independently > come to the conclusion that what we are adhering to is actually a typo) I'm > going to toss my +1 in with Robert's. OK, the consensus seems clearly against changing this in the back branches, so I'll just fix it in HEAD. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REFERENCES privilege should not be symmetric (was Re: [GENERAL] Postgres Permissions Article)
On Fri, Mar 31, 2017 at 7:40 PM, Tom Lane wrote: > Robert Haas writes: > > On Fri, Mar 31, 2017 at 11:29 AM, Tom Lane wrote: > >> The argument for not back-patching a bug fix usually boils down to > >> fear of breaking existing applications, but it's hard to see how > >> removal of a permission check could break a working application --- > >> especially when the permission check is as hard to trigger as this one. > >> How many table owners ever revoke their own REFERENCES permission? > > > Sure, but that argument cuts both ways. If nobody ever does that, who > > will be helped by back-patching this? > > I certainly agree that back-patching this change is pretty low risk. > > I just don't think it has any real benefits. > > I think the benefit is reduction of user confusion. Admittedly, since > Paul is the first person I can remember ever having complained about it, > maybe nobody else is confused. > I think we also need to be extra careful about changing *security related* behavior in back branches, even more so than other behavior. In this case I think it's quite unlikely that it would hit somebody, but the risk is there. And people generally auto-upgrade to the latest minor releases, whereas they at least in theory read the top of the release notes when doing a major upgrade (ok, most people probably don't, but at least some do). -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] REFERENCES privilege should not be symmetric (was Re: [GENERAL] Postgres Permissions Article)
On Fri, Mar 31, 2017 at 10:40 AM, Tom Lane wrote: > Robert Haas writes: > > On Fri, Mar 31, 2017 at 11:29 AM, Tom Lane wrote: > >> The argument for not back-patching a bug fix usually boils down to > >> fear of breaking existing applications, but it's hard to see how > >> removal of a permission check could break a working application --- > >> especially when the permission check is as hard to trigger as this one. > >> How many table owners ever revoke their own REFERENCES permission? > > > Sure, but that argument cuts both ways. If nobody ever does that, who > > will be helped by back-patching this? > > I certainly agree that back-patching this change is pretty low risk. > > I just don't think it has any real benefits. > > I think the benefit is reduction of user confusion. Admittedly, since > Paul is the first person I can remember ever having complained about it, > maybe nobody else is confused. > After going back-and-forth on this (and not being able to independently come to the conclusion that what we are adhering to is actually a typo) I'm going to toss my +1 in with Robert's. If anyone actually complains about the behavior and not just the documentation we could consider back-patching if any release before 10.0 is still under support. There have been non-bug fix improvements to the docs that didn't get back-patched covering topics more confusing than this. Expecting those learning the system to consult the most recent version of the docs is standard fare here. From a practical perspective the revised current docs will be applicable for past versions as long as one doesn't go a get their REFERENCES permission revoked somehow. If they do, and wonder why, the docs and these list will be able to explain it reasonably well. David J.
Re: [HACKERS] REFERENCES privilege should not be symmetric (was Re: [GENERAL] Postgres Permissions Article)
Robert Haas writes: > On Fri, Mar 31, 2017 at 11:29 AM, Tom Lane wrote: >> The argument for not back-patching a bug fix usually boils down to >> fear of breaking existing applications, but it's hard to see how >> removal of a permission check could break a working application --- >> especially when the permission check is as hard to trigger as this one. >> How many table owners ever revoke their own REFERENCES permission? > Sure, but that argument cuts both ways. If nobody ever does that, who > will be helped by back-patching this? > I certainly agree that back-patching this change is pretty low risk. > I just don't think it has any real benefits. I think the benefit is reduction of user confusion. Admittedly, since Paul is the first person I can remember ever having complained about it, maybe nobody else is confused. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REFERENCES privilege should not be symmetric (was Re: [GENERAL] Postgres Permissions Article)
On Fri, Mar 31, 2017 at 11:29 AM, Tom Lane wrote: > Robert Haas writes: >> On Thu, Mar 30, 2017 at 4:45 PM, Tom Lane wrote: >>> In short, it seems like this statement in the docs is correctly describing >>> our code's behavior, but said behavior is wrong and should be changed. >>> I'd propose fixing it like that in HEAD; I'm not sure if the back branches >>> should also be changed. > >> Sounds reasonable, but I don't see much advantage to changing it in >> the back-branches. > > Well, it's a SQL-compliance bug, and we often back-patch bug fixes. Personally, I'd be more inclined to view it as a definitional change. Maybe we picked the wrong definition before, but it's well-established behavior at this point. The SQL standard also says that identifiers are supposed to be folded to upper case, so I don't think the theory that every deviation from the standard should be fixed and back-patched holds a lot of water. > The argument for not back-patching a bug fix usually boils down to > fear of breaking existing applications, but it's hard to see how > removal of a permission check could break a working application --- > especially when the permission check is as hard to trigger as this one. > How many table owners ever revoke their own REFERENCES permission? Sure, but that argument cuts both ways. If nobody ever does that, who will be helped by back-patching this? I certainly agree that back-patching this change is pretty low risk. I just don't think it has any real benefits. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REFERENCES privilege should not be symmetric (was Re: [GENERAL] Postgres Permissions Article)
Robert Haas writes: > On Thu, Mar 30, 2017 at 4:45 PM, Tom Lane wrote: >> In short, it seems like this statement in the docs is correctly describing >> our code's behavior, but said behavior is wrong and should be changed. >> I'd propose fixing it like that in HEAD; I'm not sure if the back branches >> should also be changed. > Sounds reasonable, but I don't see much advantage to changing it in > the back-branches. Well, it's a SQL-compliance bug, and we often back-patch bug fixes. The argument for not back-patching a bug fix usually boils down to fear of breaking existing applications, but it's hard to see how removal of a permission check could break a working application --- especially when the permission check is as hard to trigger as this one. How many table owners ever revoke their own REFERENCES permission? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REFERENCES privilege should not be symmetric (was Re: [GENERAL] Postgres Permissions Article)
On Thu, Mar 30, 2017 at 4:45 PM, Tom Lane wrote: > In short, it seems like this statement in the docs is correctly describing > our code's behavior, but said behavior is wrong and should be changed. > I'd propose fixing it like that in HEAD; I'm not sure if the back branches > should also be changed. Sounds reasonable, but I don't see much advantage to changing it in the back-branches. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] REFERENCES privilege should not be symmetric (was Re: [GENERAL] Postgres Permissions Article)
Paul Jungwirth writes: >> Also I don't understand why you wrote “You need the permission on both >> tables”: Only the owner of a table can add constraints to it > Ah, this piece was really helpful for me in making it click. Thanks so > much! I added a couple new paragraphs to my post with a link back to > this thread. I feel like it all makes sense now! :-) > FYI "You need this permission on both tables" is what the docs say > (https://www.postgresql.org/docs/9.6/static/sql-grant.html): >>> To create a foreign key constraint, it is necessary to have this >>> privilege on both the referencing and referenced columns. > Maybe it would be worth clarifying there that you need to *own* the > referencing table, and you need REFERENCES on the referenced table? Hmm ... interesting. A bit of excavating in tablecmds.c shows that in order to do ADD FOREIGN KEY, you need to be owner of the table the constraint is being attached to (checked by ATSimplePermissions, which is used for AT_AddConstraint by ATPrepCmd), *and* you need REFERENCES on both tables, or at least on the columns involved in the proposed FK constraint (checked by checkFkeyPermissions, which is invoked against each of the tables by ATAddForeignKeyConstraint). So yeah, this seems a little bit redundant. In principle, a table owner could revoke her own REFERENCES permissions on the table and thereby disable creation of FKs leading out of it, but it'd be pretty unusual to want to do so. Moreover, this definition seems neither intuitive (REFERENCES doesn't seem like it should be symmetric) nor compliant with the SQL standard. In SQL:2011 section 11.8 I read Access Rules 1) The applicable privileges for the owner of T shall include REFERENCES for each referenced column. (T is the referenced table.) I see nothing suggesting that the command requires REFERENCES privilege on the referencing table. Now this is a little garbled, because surely they meant the owner of the referencing table (who is issuing the command) not the owner of the referenced table, but I think the intent is clear enough. In short, it seems like this statement in the docs is correctly describing our code's behavior, but said behavior is wrong and should be changed. I'd propose fixing it like that in HEAD; I'm not sure if the back branches should also be changed. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers