Re: Shared dependency patch

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