Thread: Shared dependency patch
Patchers, Here is the latest installation of my shared dependency patch. As some of you may remember, the purpose of this patch is to record dependencies on shared objects, such as users, groups and tablespaces, from regular database objects. This is done on a new shared system catalog called pg_shdepend, so that when a backend wants to drop any shared object, it can easily verify whether it is referenced in other database. I have upgraded the patch to include references present in ACLs, and to lock the objects appropiately before checking. To do this I had to change the LOCKTAG struct somewhat, using a previous patch by Rod Taylor (thanks Rod!); and add a LockSharedObject() function. I believe that with these changes, we no longer require to be able to specify SysIds at user or group creation, because this necessity was driven by the fact that it was very easy to drop users mentioned in ACLs. Althought I haven't done it in the patch I present, it is now possible to get rid of that part of the grammar and also of the AclId type. This patch includes the required documentation additions, but I haven't checked whether existing documentation needs to be updated. Also, I'm not including any regression tests. Comments? My idea would be to apply this to HEAD, but giving the noise about a non-initdb-forcing 8.1, it would have to wait. (But then we could provide an upgrading method; I think this patch only needs pg_shdepend to be created and populated.) -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "Some men are heterosexual, and some are bisexual, and some men don't think about sex at all... they become lawyers" (Woody Allen)
Attachment
Hackers, Here is a newer version of the shared dependency patch. I keep the previous description below. Changes from the previous version are: - Removed several bogus elog(NOTICE) - Groups are checked at deletion for dependencies - I noticed a couple of tests (cluster and privileges) were leaving orphaned objects, so with this patch they would leave users behind. (consequently, future runs of the tests would fail). - Added simple regression testing. New files are src/backend/catalog/pg_shdepend.c src/include/catalog/pg_shdepend.h src/test/regress/sql/dependency.sql src/test/regress/expected/dependency.sql Comments are welcome. On Wed, Jan 26, 2005 at 09:04:53PM -0300, I wrote: > Patchers, > > Here is the latest installation of my shared dependency patch. > As some of you may remember, the purpose of this patch is to record > dependencies on shared objects, such as users, groups and tablespaces, > from regular database objects. This is done on a new shared system > catalog called pg_shdepend, so that when a backend wants to drop any > shared object, it can easily verify whether it is referenced in other > database. > > I have upgraded the patch to include references present in ACLs, and to > lock the objects appropiately before checking. To do this I had to > change the LOCKTAG struct somewhat, using a previous patch by Rod Taylor > (thanks Rod!); and add a LockSharedObject() function. -- Alvaro Herrera (<alvherre[@]dcc.uchile.cl>) Major Fambrough: You wish to see the frontier? John Dunbar: Yes sir, before it's gone.
Attachment
With us moving to a normal 8.1 release do you want to submit submit to be applied to CVS? --------------------------------------------------------------------------- Alvaro Herrera wrote: > Hackers, > > Here is a newer version of the shared dependency patch. I keep the > previous description below. Changes from the previous version are: > > - Removed several bogus elog(NOTICE) > - Groups are checked at deletion for dependencies > - I noticed a couple of tests (cluster and privileges) were leaving > orphaned objects, so with this patch they would leave users behind. > (consequently, future runs of the tests would fail). > - Added simple regression testing. > > New files are > > src/backend/catalog/pg_shdepend.c > src/include/catalog/pg_shdepend.h > src/test/regress/sql/dependency.sql > src/test/regress/expected/dependency.sql > > Comments are welcome. > > > On Wed, Jan 26, 2005 at 09:04:53PM -0300, I wrote: > > > Patchers, > > > > Here is the latest installation of my shared dependency patch. > > As some of you may remember, the purpose of this patch is to record > > dependencies on shared objects, such as users, groups and tablespaces, > > from regular database objects. This is done on a new shared system > > catalog called pg_shdepend, so that when a backend wants to drop any > > shared object, it can easily verify whether it is referenced in other > > database. > > > > I have upgraded the patch to include references present in ACLs, and to > > lock the objects appropiately before checking. To do this I had to > > change the LOCKTAG struct somewhat, using a previous patch by Rod Taylor > > (thanks Rod!); and add a LockSharedObject() function. > > -- > Alvaro Herrera (<alvherre[@]dcc.uchile.cl>) > Major Fambrough: You wish to see the frontier? > John Dunbar: Yes sir, before it's gone. [ Attachment, skipping... ] [ Attachment, skipping... ] [ Attachment, skipping... ] [ Attachment, skipping... ] [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 6: Have you searched our list archives? > > http://archives.postgresql.org -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
On Mon, Feb 14, 2005 at 10:09:55PM -0500, Bruce Momjian wrote: > > With us moving to a normal 8.1 release do you want to submit submit to > be applied to CVS? Yes; I have since updated this patch in several areas. I will submit it as soon as I'm back from my honeymoon. (I'm getting married on friday.) Thanks, -- Alvaro Herrera (<alvherre[@]dcc.uchile.cl>) "Pensar que el espectro que vemos es ilusorio no lo despoja de espanto, sólo le suma el nuevo terror de la locura" (Perelandra, CSLewis)
Alvaro Herrera wrote: > On Mon, Feb 14, 2005 at 10:09:55PM -0500, Bruce Momjian wrote: > >>With us moving to a normal 8.1 release do you want to submit submit to >>be applied to CVS? > > > Yes; I have since updated this patch in several areas. I will submit it > as soon as I'm back from my honeymoon. (I'm getting married on friday.) Congratulations. Sincerely, Joshua D. Drake > > Thanks, > -- Command Prompt, Inc., your source for PostgreSQL replication, professional support, programming, managed services, shared and dedicated hosting. Home of the Open Source Projects plPHP, plPerlNG, pgManage, and pgPHPtoolkit. Contact us now at: +1-503-667-4564 - http://www.commandprompt.com
Attachment
On Mon, Feb 14, 2005 at 10:09:55PM -0500, Bruce Momjian wrote: > With us moving to a normal 8.1 release do you want to submit submit to > be applied to CVS? Here it is in a single tarball. I keep the previous description below. Thanks, Here is the latest installation of my shared dependency patch. As some of you may remember, the purpose of this patch is to record dependencies on shared objects, such as users, groups and tablespaces, from regular database objects. This is done on a new shared system catalog called pg_shdepend, so that when a backend wants to drop any shared object, it can easily verify whether it is referenced in other database. I have upgraded the patch to include references present in ACLs, and to lock the objects appropiately before checking. To do this I had to change the LOCKTAG struct somewhat, using a previous patch by Rod Taylor (thanks Rod!); and add a LockSharedObject() function. -- Alvaro Herrera (<alvherre[@]dcc.uchile.cl>) "La tristeza es un muro entre dos jardines" (Khalil Gibran)
Attachment
On Wed, Mar 09, 2005 at 01:05:31AM -0300, Alvaro Herrera wrote: Hackers, I have updated this patch to the current CVS HEAD. If somebody would be so kind to review this for applying at his earliest convenience, I'd appreciate it. Previous description below: > Here is the latest installation of my shared dependency patch. > As some of you may remember, the purpose of this patch is to record > dependencies on shared objects, such as users, groups and tablespaces, > from regular database objects. This is done on a new shared system > catalog called pg_shdepend, so that when a backend wants to drop any > shared object, it can easily verify whether it is referenced in other > database. > > I have upgraded the patch to include references present in ACLs, and to > lock the objects appropiately before checking. To do this I had to > change the LOCKTAG struct somewhat, using a previous patch by Rod Taylor > (thanks Rod!); and add a LockSharedObject() function. -- Alvaro Herrera (<alvherre[@]dcc.uchile.cl>) "Investigación es lo que hago cuando no sé lo que estoy haciendo" (Wernher von Braun)
Attachment
Alvaro Herrera <alvherre@dcc.uchile.cl> writes: > I have updated this patch to the current CVS HEAD. If somebody would be > so kind to review this for applying at his earliest convenience, I'd > appreciate it. It's not really ready to apply yet, because it's a bit schizophrenic about the users-vs-groups business. You are treating groups as a distinct object class in shdependUpdateAclInfo, but I don't see an OCLASS_GROUP ... isn't this going to fail as soon as someone tries to display a dependency on a group? But I'm not sure it's worth going to the trouble of fixing, seeing that we intend to remove groups soon in favor of roles. Also, you seem to have decided that we don't need dependency types for shared dependencies, which I think is a bad idea. In particular we should have at least DEPENDENCY_PIN, whereupon we can pin the original superuser, whereupon most of the initdb-time dependencies you are currently installing needn't exist at all. That's not just a space savings but a considerable time savings during searches. (Imagine how much slower the regular dependency stuff would be if we hadn't invented DEPENDENCY_PIN and therefore had to explicitly record all dependencies on every system object.) I'm also unconvinced that we would never find a use for DEPENDENCY_AUTO or DEPENDENCY_INTERNAL. I'm inclined to think it would be best to put this on the back burner until after the pg_role catalog changes are finished. Otherwise you'll have to do a fair amount of ultimately-useless work to make the group handling realistic. As far as OCLASS_AM goes, wouldn't it be simpler just to remove the owner column from pg_am? I can't imagine that there will ever be runtime commands to add and remove index access methods, much less such commands that are allowed to non-superusers. So the notion of an owner for an index AM seems like dead weight. (Compare the lack of an owner for languages.) I didn't have a problem with carrying a useless column, but adding infrastructure to support a useless column is a bit much. regards, tom lane
Alvaro, did you update your patch to address the concerns mentioned below? --------------------------------------------------------------------------- Tom Lane wrote: > Alvaro Herrera <alvherre@dcc.uchile.cl> writes: > > I have updated this patch to the current CVS HEAD. If somebody would be > > so kind to review this for applying at his earliest convenience, I'd > > appreciate it. > > It's not really ready to apply yet, because it's a bit schizophrenic > about the users-vs-groups business. You are treating groups as a > distinct object class in shdependUpdateAclInfo, but I don't see an > OCLASS_GROUP ... isn't this going to fail as soon as someone tries > to display a dependency on a group? But I'm not sure it's worth > going to the trouble of fixing, seeing that we intend to remove > groups soon in favor of roles. > > Also, you seem to have decided that we don't need dependency types > for shared dependencies, which I think is a bad idea. In particular > we should have at least DEPENDENCY_PIN, whereupon we can pin the > original superuser, whereupon most of the initdb-time dependencies you > are currently installing needn't exist at all. That's not just a space > savings but a considerable time savings during searches. (Imagine > how much slower the regular dependency stuff would be if we hadn't > invented DEPENDENCY_PIN and therefore had to explicitly record all > dependencies on every system object.) I'm also unconvinced that > we would never find a use for DEPENDENCY_AUTO or DEPENDENCY_INTERNAL. > > I'm inclined to think it would be best to put this on the back burner > until after the pg_role catalog changes are finished. Otherwise > you'll have to do a fair amount of ultimately-useless work to make > the group handling realistic. > > As far as OCLASS_AM goes, wouldn't it be simpler just to remove the > owner column from pg_am? I can't imagine that there will ever be > runtime commands to add and remove index access methods, much less such > commands that are allowed to non-superusers. So the notion of an owner > for an index AM seems like dead weight. (Compare the lack of an owner > for languages.) I didn't have a problem with carrying a useless column, > but adding infrastructure to support a useless column is a bit much. > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 8: explain analyze is your friend > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
On Mon, Apr 25, 2005 at 10:14:05PM -0400, Bruce Momjian wrote: > Alvaro, did you update your patch to address the concerns mentioned below? I'm on it right now. I wanted to finish the shared row locking patch first, and now that I'm waiting on someone to review it, I'll give some time to this. > Tom Lane wrote: > > Alvaro Herrera <alvherre@dcc.uchile.cl> writes: > > > I have updated this patch to the current CVS HEAD. If somebody would be > > > so kind to review this for applying at his earliest convenience, I'd > > > appreciate it. > > > > It's not really ready to apply yet, because it's a bit schizophrenic > > about the users-vs-groups business. You are treating groups as a > > distinct object class in shdependUpdateAclInfo, but I don't see an > > OCLASS_GROUP ... isn't this going to fail as soon as someone tries > > to display a dependency on a group? But I'm not sure it's worth > > going to the trouble of fixing, seeing that we intend to remove > > groups soon in favor of roles. > > > > Also, you seem to have decided that we don't need dependency types > > for shared dependencies, which I think is a bad idea. In particular > > we should have at least DEPENDENCY_PIN, whereupon we can pin the > > original superuser, whereupon most of the initdb-time dependencies you > > are currently installing needn't exist at all. That's not just a space > > savings but a considerable time savings during searches. (Imagine > > how much slower the regular dependency stuff would be if we hadn't > > invented DEPENDENCY_PIN and therefore had to explicitly record all > > dependencies on every system object.) I'm also unconvinced that > > we would never find a use for DEPENDENCY_AUTO or DEPENDENCY_INTERNAL. > > > > I'm inclined to think it would be best to put this on the back burner > > until after the pg_role catalog changes are finished. Otherwise > > you'll have to do a fair amount of ultimately-useless work to make > > the group handling realistic. > > > > As far as OCLASS_AM goes, wouldn't it be simpler just to remove the > > owner column from pg_am? I can't imagine that there will ever be > > runtime commands to add and remove index access methods, much less such > > commands that are allowed to non-superusers. So the notion of an owner > > for an index AM seems like dead weight. (Compare the lack of an owner > > for languages.) I didn't have a problem with carrying a useless column, > > but adding infrastructure to support a useless column is a bit much. -- Alvaro Herrera (<alvherre[@]dcc.uchile.cl>) "No single strategy is always right (Unless the boss says so)" (Larry Wall)