Alvaro, did you update your patch to address the concerns mentioned below?

---------------------------------------------------------------------------

Tom Lane wrote:
> Alvaro Herrera <[EMAIL PROTECTED]> 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

---------------------------(end of broadcast)---------------------------
TIP 8: explain analyze is your friend

Reply via email to