Re: Oddity in handling of cached plans for FDW queries - Mailing list pgsql-hackers

I wrote:
> I concur with Etsuro-san's dislike for hasForeignJoin; that flag is
> underspecified and doesn't convey nearly enough information.  I do not
> think a uses_user_mapping flag is much better.  ISTM what should happen is
> that any time we decide to push down a foreign join, we should record the
> identity of the common user mapping that made that pushdown possible in
> the plan's invalItems list.  That would make it possible to invalidate
> only the relevant plans when a user mapping is changed.

I thought a bit more about this and realized that the above doesn't work
too well.  Initially, a join might have been pushed down on the strength
of both userids mapping to a PUBLIC user mapping for the server.  If now
someone does CREATE USER MAPPING to install a new mapping for one of
those userids, we should invalidate the plan --- but there is certainly
not going to be anything in the plan matching the new user mapping.

> Another way we could attack it would be to record the foreign server OID
> as an invalItem for any query that has more than one foreign table
> belonging to the same foreign server.  Then, invalidate whenever any user
> mapping for that server changes.

And that doesn't work so well either, because the most that the plan inval
code is going to have its hands on is (a hash of) the OID of the user
mapping that changed.  We can't tell which server that's for.

On reflection, it seems to me that we've gone wrong by tying planning to
equality of user mappings at all, and the best way to get out of this is
to not do that.  Instead, let's insist that a join can be pushed down only
if the checkAsUser fields of the relevant RTEs are equal.  If they are,
then the same user mapping must apply to both at runtime, whatever it is
--- and we don't need to predetermine that.  With this approach, the need
for plan invalidation due to user mapping changes goes away entirely.

This doesn't cost us anything at all in simple cases such as direct
execution of a query, because all the checkAsUser fields will be equal
(i.e., zero).  And it also doesn't hurt if the potential foreign join is
encapsulated in a view, where all the checkAsUser fields would contain
the OID of the view owner.

The situation where we potentially lose something is a case like
Etsuro-san's original example, where the query contains one foreign table
reference coming from a view and one coming from the outer query, or maybe
from a different view.  In the two-views case we would have to not push
down the join if the views have different owners, even though perhaps both
owners will use the PUBLIC mapping at runtime.  I think that's a narrow
enough case that we can just live with not optimizing it.  In the
view-and-outer-query case, the simplest answer is that we can't push down
because zero is not equal to the view owner's OID.  We could make that a
little better if we know that the query will be executed as the view
owner, so that the relevant user IDs will be the same at runtime.  There
is already some mechanism in the plan cache to track whether a plan
depends on the identity of the user running it (for RLS), so we could use
that to enforce that a plan containing such a pushed-down join is only run
by the same user that owns the view.

Comments?
        regards, tom lane