Re: Oddity in handling of cached plans for FDW queries

On Wed, Jul 20, 2016 at 10:15 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> Mumble. Why, exactly, was this a good idea? The upside of commit
>> 45639a0525a58a2700cf46d4c934d6de78349dac is only that you do fewer
>> plan invalidations, but surely that's not a significant benefit for
>> most people: user mappings don't change that often. On the downside,
>> there are now cases where joins would have gotten pushed down
>> previously and now they won't. In other words, you've saved some
>> replanning activity at the cost of delivering worse plans. That seems
>> pretty suspect to me, although I grant that the scenarios where either
>> the old or the new behavior is actually a problem are all somewhat off
>> the beaten path.
>
> I think that you are undervaluing the removal of user-mapping-based plan
> invalidation. That was never more than a kluge, and here is the reason:
> we have no way to lock user mappings. The system whereby we invalidate
> plans as a consequence of table DDL changes is bulletproof, because we
> (re) acquire locks on the tables used in the plan, then check for
> invalidation signals, before deciding whether the plan is stale. The
> corresponding scenario where a user mapping changes between that check
> and execution time is unprotected, so that we could end up using a plan
> that is insecure for the mappings selected at execution.

OK, that's a fair point. Thanks for explaining.

> Another way we could have removed the race condition is the suggestion
> made upthread of embedding the user mapping details right into the plan
> instead of looking them up afresh at execution. But I didn't much like
> that approach, per upthread discussion.

OK.

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