Re: Shared dependency patch

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(at)dcc(dot)uchile(dot)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[(at)]dcc(dot)uchile(dot)cl>)
"No single strategy is always right (Unless the boss says so)"
(Larry Wall)