Re: Flexible permissions for REFRESH MATERIALIZED VIEW - Mailing list pgsql-hackers

Isaac Morland <isaac.morland@gmail.com> writes:
> The original idea was to allow access to REFRESH MATERIALIZED VIEW to be a
> grantable permission, rather than being reserved to the table owner.

I'm not really on board with making that a separately grantable
permission.  You can do what you need today by having the matview be owned
by a single-purpose group role and granting membership in that role as
needed.  We do not have an infinite supply of available privilege type
bits --- in fact, without breaking on-disk compatibility, there are only
four free bits in AclMode.  So I can't see using one of them up for
REFRESH, let alone using three of them up for REFRESH, CLUSTER and
REINDEX.

> The patch so far uses TRUNCATE permission to control access as a
> proof-of-concept.

I can't see doing that either, TBH.  It's just ugly, and it surely doesn't
scale to cases where the conflated operations all apply to the same kind
of object.  (For instance, including CLUSTER permissions in TRUNCATE
wouldn't be very sane.)

It's conceivable that we could somehow allow new bits to get overlaid
onto bits currently used only for other object types, without that being
user-visible.  But I believe there are significant implementation issues
that'd have to be surmounted; for instance aclitemout has no idea what
sort of object the ACL it's printing is for.  Also, the ACL machinery
doesn't currently think that "matview" is a distinct type of object
from "table" anyway; it just sees 'em all as "relation".

Or, maybe I'm wrong and people feel that REFRESH-by-non-owner is important
enough to be worth consuming an AclMode bit for.  But I'm dubious.

> I think backward compatibility is pretty good.

No, actually, backwards compatibility would be bad.  If someone
had granted any privileges on their matview, which they almost certainly
would have if they cared about this scenario at all, then the owner's
privileges would have been instantiated in pg_class.relacl as
ACL_ALL_RIGHTS_RELATION, which doesn't (currently) include the
hypothetical new ACL_REFRESH bit.  So after upgrading, they'd have to
explicitly grant themselves the new REFRESH right before they could
refresh.  (I think pg_dump makes some attempt to cater for this case
by printing "GRANT ALL PRIVILEGES" in cases where all currently-extant
privileges are granted, but that's not very bulletproof; especially not
when pg_dump and server versions don't match, as they would not in an
upgrade situation.)

We could avoid that set of problems if we just redefined TRUNCATE as
meaning "TRUNCATE or REFRESH", but then you find out that people who
didn't have REFRESH ability before suddenly do.  Admittedly there was
no reason to grant such a privilege on a matview before, so maybe
people didn't; but by the same token there was no harm in granting
such a privilege, so maybe people did.  It's certainly not free from
the backwards-compatibility standpoint.

Now, none of these things are particularly the fault of this patch;
the bottom line is we don't have a good design for adding privilege
types to existing object kinds.  But nonetheless these are problems.

            regards, tom lane