Re: perplexing error message - Mailing list pgsql-hackers
From | Robert Haas |
---|---|
Subject | Re: perplexing error message |
Date | |
Msg-id | CA+TgmoY5XNTWMc65+hVL+sBNypX72ACjXuNn9k6W=iNNdbzLrA@mail.gmail.com Whole thread Raw |
In response to | Re: perplexing error message (Tom Lane <tgl@sss.pgh.pa.us>) |
List | pgsql-hackers |
On Sat, Feb 7, 2015 at 4:35 PM, Tom Lane <tgl@sss.pgh.pa.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
pgsql-hackers by date: