Re: Fwd: Problem with a "complex" upsert

I wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
>> I think we definitely should try to get this in.

> Well, if you're excited about it, help Dean review it.

So, in the spirit of "put your money where your mouth is", I've been
working off-list with Dean today to review this patch. We found a couple
additional minor issues:

* We noticed that the rewriter was expanding the EXCLUDED
pseudo-relation's RTE into an RTE_SUBQUERY RTE, rather than leaving it
in the intended form as an RTE_RELATION RTE with a nonstandard rtekind.
(This happens basically always with a view target rel in the existing
code, but only in corner cases after Dean's patch.) While this doesn't
seem to have obvious bad effects, it's both unintended and a waste of
cycles. Also, the fact that this area seems rather undertested leaves
me not wanting to have weird data structure differences that happen
only in corner cases. So we added a check to fireRIRrules to prevent
that from happening.

* We happened across some unexpected permissions failures while checking
the patch in cases where the calling user is not the view owner. This
turned out to be because of a pre-existing oversight: the replacement
EXCLUDED RTE is initially manufactured with requiredPerms = ACL_SELECT,
and nothing was getting done to change that, leading to failure if the
calling user doesn't have SELECT on the underlying table. (The desired
behavior is that the view owner needs permissions on the underlying table,
but the calling user only needs permissions on the view.) So that's
easily fixed by zeroing out requiredPerms in the EXCLUDED RTE; nothing is
lost because other code was already adding the required permission check
flags to the query's actual target relation. But out of paranoia we added
a bunch of permissions-checking test cases to updatable_views.sql.

Attached is our finished patch against HEAD. This is pretty much all
Dean's work, but I'm posting it on his behalf because it's late in the UK
and he's gone offline for the day. In the interests of getting a
full set of buildfarm testing on the patch before Monday's wrap deadline,
I'm going to finish up back-porting the patch and push it tonight.

regards, tom lane