Re: perplexing error message

On Sat, Feb 7, 2015 at 4:35 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> What's going on here is that we're checking the SELECT that is added
> by the rule, and per TFM that executes before the UPDATE, so you get
> this error first rather than one complaining about UPDATE (which you
> would get later if this one hadn't interfered).
>
> It's visible at this level that this PlannedStmt isn't the main query
> (because it has canSetTag = 0), but that does not help much because
> we have no very convenient way to find the main querytree. Anyway
> you can construct related scenarios where the tag for the current
> PlannedStmt probably *is* the thing to use, so I'm disinclined to
> try to fix it by locating the main querytree to use the tag for that.
>
> I think a more realistic way to fix it is to adjust rule expansion so that
> the RTE generated for rules_src in the added query is marked with only the
> privileges required for that query --- I imagine right now, we're just
> copying the requiredPerms verbatim from the original query. There's
> certainly no other obvious reason why an RTE in a SELECT (VALUES)
> querytree would have the ACL_UPDATE bit set, as this one does.

That seems to be what's happening. Specifically, rewriteRuleAction() does this:

sub_action->rtable = list_concat((List *) copyObject(parsetree->rtable),
sub_action->rtable);

There's a long comment justifying that; the important point, as far as
I can see, is that when we expand a view we want to check permissions
on the view, not the expansion. In other words, this is fairly
intentional behavior.

Now, we could reduce the scope of the weirdness by clearing all the
required permission bits from the copied range table in the DO-ALSO
case, because we know there is a copy with the original permission
bits floating around someplace and so those bits will be checked. But
that's not a complete fix, because you'll still get an error
complaining about SELECT if you try to UPDATE a view with a DO INSTEAD
SELECT rule. In fact, I'm not really sure why that case should fail
at all. Despite the fact that we require UPDATE permission to perform
it in this instance, it's still a SELECT at heart, and that's a
read-only operation. Maybe we should be keying
ExecCheckXactReadOnly's decision off of the PlannedStmt's commandType;
if not CMD_SELECT, then iterate over resultRelations and check whether
each of those is an RTE_RELATION in a non-temp namespace.

I'm not sure that's correct when hasModifyingCTE is set, though. I
don't know what to do about that case.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company