* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > * Tom Lane (tgl@sss.pgh.pa.us) wrote:
> >> To make this work, you'd need a way to distinguish privileges installed
> >> by initdb from those changed later.
>
> > To replicate whatever the current ACL is, we don't actually need to
> > make such a differentiation. I'm not against doing so, but the only
> > point of it would be to eliminate a few extra lines being dumped out
> > which re-run those commands that initdb runs on restore.
>
> No, the point of it would be to not have pg_dump scripts overriding
> installed-by-default ACLs. A newer PG version might have different
> ideas about what those should be. I don't think this is exactly an
> academic concern, either: wouldn't a likely outcome of your default-roles
> work be that some built-in functions have different initial ACLs than
> they do today? Good luck with that, if pg_upgrade overwrites those
> ACLs with the previous-version values.
As it turns out, there isn't such an issue as the default for functions
is to allow PUBLIC to EXECUTE and therefore we don't dump out ACLs for
most functions. The follow-on change to this patch is to modify those
functions to *not* have the default/NULL ACL (and also drop the explicit
if (!superuser()) ereport() checks in those functions), which will work
just fine and won't be overwritten during pg_upgrade because those
functions currently just have the default ACL, which we don't dump out.
Of course, it's a different story if the user changes the ACL on objects
in pg_catalog and then we change what we think the default ACL should
be, but in such a case, I'm guessing we should probably go with what the
user explicitly asked for anyway and if there's a serious enough change
in the permissions of the function then perhaps we should have a
different function instead of re-defining the existing one.
We do have some fun issues with pg_upgrade by going with this approach
of having pg_dump dump out ACLs- what happens when there's a function or
column which goes away? If there's a non-NULL ACL on them, the restore
will just outright fail, if we don't do something more. I'm not a huge
fan of coding into pg_dump the knowledge of every object which exists
for every version of PG we support for pg_dump though.
Regarding the default roles patch, now that it's down to only one
default role, based on the assumption that this approach with pg_dump
will solve all the other concerns, there isn't really much overlap
between the default roles and the function ACLs. Those functions which
will be able to work with just ACLs won't have a default role and the
functions which we need a default role for will have the default NULL
ACL.
Thanks!
Stephen