Re: docs: note ownership requirement for refreshing materialized views

Dave Cramer

On Thu, 16 Aug 2018 at 18:27, Jonathan S. Katz <
jonathan(dot)katz(at)excoventures(dot)com> wrote:

>
> On Aug 16, 2018, at 1:05 AM, Jonathan S. Katz <
> jonathan(dot)katz(at)excoventures(dot)com> wrote:
>
>
> On Aug 15, 2018, at 9:15 PM, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Wed, Aug 15, 2018 at 09:06:34PM -0400, Jonathan S. Katz wrote:
>
> I played around with this feature a bit and did see this was the case.
> Also while playing around I noticed the error message was as such:
>
> test=> REFRESH MATERIALIZED VIEW blah;
> ERROR: must be owner of relation blah
>
> But it’s not a relation, it’s a materialized view. I attached a patch
> that I think should fix this. Kudos to Dave Cramer who was
> sitting next to me helping me to locate files and confirm assumptions.
>
>
> A relation may be a materialized view, no? The ACL check happens in
> RangeVarCallbackOwnsTable by the way (look at ExecRefreshMatView in
> matview.c).
>
>
> Comment on the RangeVarCallbackOwnsTable func (abbr):
>
> /*
> * This is intended as a callback for RangeVarGetRelidExtended(). It
> allows
> * the relation to be locked only if (1) it's a plain table,
> materialized
> * view, or TOAST table and (2) the current user is the owner (or the
> * superuser). This meets the permission-checking needs of CLUSTER,
> REINDEX
> * TABLE, and REFRESH MATERIALIZED VIEW; we expose it here so that it
> can be
> * used by all.
> */
>
> So it’s sharing the permission checking needs amongst all of those
> commands.
>
> As a user I could be confused if I saw the above error message, esp.
> because
> the behavior of REFRESH .. is specific to materialized views.
>
>
> With encouragement from Dave, let me demonstrate what the proposed patch
> does to fix the behavior. The steps, running from my “jkatz” user:
>
> CREATE ROLE bar LOGIN;
> CREATE TABLE a (x int);
> CREATE MATERIALIZED VIEW b AS SELECT * FROM a;
> \c - bar
> REFRESH MATERIALIZED VIEW b;
> ERROR: must be owner of materialized view b
>
> vs. the existing error message which I posted further upthread.
>
> Thanks,
>
> Jonathan
>

So it seems this patch is being ignored in this thread.
AFAICT, in aclcheck_error() the only way to get to the following sub case
in the case ACLCHECK_NOT_OWNER

case OBJECT_MATVIEW:
msg = gettext_noop("must be owner of materialized view %s");
break;

is if the patch is applied ?

Regards,

Dave