Thread
Commits
-
Fix bug in DROP OWNED BY.
- 7188b9b0fdbb 16.0 landed
-
Ensure that pg_auth_members.grantor is always valid.
- 6566133c5f52 16.0 cited
-
DROP OWNED BY is broken on master branch.
Rushabh Lathia <rushabh.lathia@gmail.com> — 2022-09-26T07:43:53Z
Hi All, Consider the below test: postgres@53130=#create role test WITH login createdb; CREATE ROLE postgres@53130=#\c - test You are now connected to database "postgres" as user "test". postgres@53150=#create database test; CREATE DATABASE postgres@53150=#\c - rushabh You are now connected to database "postgres" as user "rushabh". postgres@53162=# postgres@53162=# -- This was working before the below mentioned commit. postgres@53162=#drop owned by test; ERROR: global objects cannot be deleted by doDeletion Commit 6566133c5f52771198aca07ed18f84519fac1be7 ensure that pg_auth_members.grantor is always valid. This commit did changes into shdepDropOwned() function and combined the SHARED_DEPENDENCY_ACL and SHARED_DEPENDENCY_OWNER. In that process it removed condition for local object in owner dependency. case SHARED_DEPENDENCY_OWNER: - /* If a local object, save it for deletion below */ - if (sdepForm->dbid == MyDatabaseId) + /* Save it for deletion below */ Case ending up with above error because of the above removed condition. Please find the attached patch which fixes the case. Thanks, Rushabh Lathia www.EnterpriseDB.com -
Re: DROP OWNED BY is broken on master branch.
Michael Paquier <michael@paquier.xyz> — 2022-09-26T08:00:56Z
On Mon, Sep 26, 2022 at 01:13:53PM +0530, Rushabh Lathia wrote: > Please find the attached patch which fixes the case. Could it be possible to stress this stuff in the regression tests? There is a gap here. (I have not looked at what you are proposing.) -- Michael
-
Re: DROP OWNED BY is broken on master branch.
Robert Haas <robertmhaas@gmail.com> — 2022-09-26T18:16:45Z
On Mon, Sep 26, 2022 at 3:44 AM Rushabh Lathia <rushabh.lathia@gmail.com> wrote: > Commit 6566133c5f52771198aca07ed18f84519fac1be7 ensure that > pg_auth_members.grantor is always valid. This commit did changes > into shdepDropOwned() function and combined the SHARED_DEPENDENCY_ACL > and SHARED_DEPENDENCY_OWNER. In that process it removed condition for > local object in owner dependency. > > case SHARED_DEPENDENCY_OWNER: > - /* If a local object, save it for deletion below */ > - if (sdepForm->dbid == MyDatabaseId) > + /* Save it for deletion below */ > > Case ending up with above error because of the above removed condition. > > Please find the attached patch which fixes the case. Thanks for the report. I think it would be preferable not to duplicate the logic as your version does, though, so here's a slightly different version that avoids that. Per Michael's suggestion, I have also written a test case and included it in this version. Comments? -- Robert Haas EDB: http://www.enterprisedb.com
-
Re: DROP OWNED BY is broken on master branch.
Rushabh Lathia <rushabh.lathia@gmail.com> — 2022-09-27T06:52:50Z
On Mon, Sep 26, 2022 at 11:46 PM Robert Haas <robertmhaas@gmail.com> wrote: > On Mon, Sep 26, 2022 at 3:44 AM Rushabh Lathia <rushabh.lathia@gmail.com> > wrote: > > Commit 6566133c5f52771198aca07ed18f84519fac1be7 ensure that > > pg_auth_members.grantor is always valid. This commit did changes > > into shdepDropOwned() function and combined the SHARED_DEPENDENCY_ACL > > and SHARED_DEPENDENCY_OWNER. In that process it removed condition for > > local object in owner dependency. > > > > case SHARED_DEPENDENCY_OWNER: > > - /* If a local object, save it for deletion below */ > > - if (sdepForm->dbid == MyDatabaseId) > > + /* Save it for deletion below */ > > > > Case ending up with above error because of the above removed condition. > > > > Please find the attached patch which fixes the case. > > Thanks for the report. I think it would be preferable not to duplicate > the logic as your version does, though, so here's a slightly different > version that avoids that. > Yes, I was also thinking to avoid the duplicate logic but couldn't found a way. I did the quick testing with the patch, and reported test is working fine. But "make check" is failing with few failures. > Per Michael's suggestion, I have also written a test case and included > it in this version. > Thanks for this. > Comments? > > -- > Robert Haas > EDB: http://www.enterprisedb.com > -- Rushabh Lathia
-
Re: DROP OWNED BY is broken on master branch.
Robert Haas <robertmhaas@gmail.com> — 2022-09-27T14:04:14Z
On Tue, Sep 27, 2022 at 2:53 AM Rushabh Lathia <rushabh.lathia@gmail.com> wrote: > Yes, I was also thinking to avoid the duplicate logic but couldn't found > a way. I did the quick testing with the patch, and reported test is working > fine. But "make check" is failing with few failures. Oh, woops. There was a dumb mistake in that version -- it was testing sdepForm->dbid == SHARED_DEPENDENCY_OWNER, which is nonsense, instead of sdepForm->dbid == MyDatabaseId. Here's a fixed version. -- Robert Haas EDB: http://www.enterprisedb.com
-
Re: DROP OWNED BY is broken on master branch.
Rushabh Lathia <rushabh.lathia@gmail.com> — 2022-09-28T12:20:53Z
On Tue, Sep 27, 2022 at 7:34 PM Robert Haas <robertmhaas@gmail.com> wrote: > On Tue, Sep 27, 2022 at 2:53 AM Rushabh Lathia <rushabh.lathia@gmail.com> > wrote: > > Yes, I was also thinking to avoid the duplicate logic but couldn't found > > a way. I did the quick testing with the patch, and reported test is > working > > fine. But "make check" is failing with few failures. > > Oh, woops. There was a dumb mistake in that version -- it was testing > sdepForm->dbid == SHARED_DEPENDENCY_OWNER, which is nonsense, instead > of sdepForm->dbid == MyDatabaseId. Here's a fixed version. > This seems to fix the issue and in further testing I didn't find anything else. Thanks, > -- > Robert Haas > EDB: http://www.enterprisedb.com > -- Rushabh Lathia
-
Re: DROP OWNED BY is broken on master branch.
Robert Haas <robertmhaas@gmail.com> — 2022-09-28T15:02:37Z
On Wed, Sep 28, 2022 at 8:21 AM Rushabh Lathia <rushabh.lathia@gmail.com> wrote: > On Tue, Sep 27, 2022 at 7:34 PM Robert Haas <robertmhaas@gmail.com> wrote: >> On Tue, Sep 27, 2022 at 2:53 AM Rushabh Lathia <rushabh.lathia@gmail.com> wrote: >> > Yes, I was also thinking to avoid the duplicate logic but couldn't found >> > a way. I did the quick testing with the patch, and reported test is working >> > fine. But "make check" is failing with few failures. >> >> Oh, woops. There was a dumb mistake in that version -- it was testing >> sdepForm->dbid == SHARED_DEPENDENCY_OWNER, which is nonsense, instead >> of sdepForm->dbid == MyDatabaseId. Here's a fixed version. > > This seems to fix the issue and in further testing I didn't find anything else. OK, committed. -- Robert Haas EDB: http://www.enterprisedb.com