Re: docs: note ownership requirement for refreshing materialized views - Mailing list pgsql-hackers

From Dave Cramer
Subject Re: docs: note ownership requirement for refreshing materialized views
Date
Msg-id CADK3HHL49W2wkMyb4A19-2__s2cCbvaAzUdZoqa6VOymhtPzow@mail.gmail.com
Whole thread Raw
In response to Re: docs: note ownership requirement for refreshing materializedviews  ("Jonathan S. Katz" <jonathan.katz@excoventures.com>)
Responses Re: docs: note ownership requirement for refreshing materializedviews
Re: docs: note ownership requirement for refreshing materialized views
List pgsql-hackers

Dave Cramer


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

On Aug 16, 2018, at 1:05 AM, Jonathan S. Katz <jonathan.katz@excoventures.com> wrote:


On Aug 15, 2018, at 9:15 PM, Michael Paquier <michael@paquier.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

pgsql-hackers by date:

Previous
From: Marina Polyakova
Date:
Subject: Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors
Next
From: Dian Fay
Date:
Subject: Re: docs: note ownership requirement for refreshing materializedviews