Thread: Shared dependency patch

Shared dependency patch

From
Alvaro Herrera
Date:
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

Re: Shared dependency patch

From
Alvaro Herrera
Date:
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

Re: Shared dependency patch

From
Bruce Momjian
Date:
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

Re: Shared dependency patch

From
Alvaro Herrera
Date:
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)

Re: Shared dependency patch

From
"Joshua D. Drake"
Date:
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

Re: Shared dependency patch

From
Alvaro Herrera
Date:
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

Re: Shared dependency patch

From
Alvaro Herrera
Date:
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

Re: Shared dependency patch

From
Tom Lane
Date:
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

Re: Shared dependency patch

From
Bruce Momjian
Date:
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

Re: Shared dependency patch

From
Alvaro Herrera
Date:
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)