Re: Use of ActiveSnapshot

Jan Wieck <JanWieck(at)Yahoo(dot)com> writes:
> The comment for the call of pg_plan_queries in util/cache/plancache.c
> line 469 for example is fatally wrong. Not only should the snapshot be
> set by all callers at this point, but if the call actually does replan
> the queries, the existing ActiveSnapshot is replaced with one allocated
> on the current memory context. If this happens to be inside of a nested
> SPI call sequence, the innermost SPI stack frame will free the snapshot
> data without restoring ActiveSnapshot to the one from the caller.

Yeah, I'd been meaning to go back and recheck that point after the code
settled down, but forgot :-(.

It is possible for RevalidateCachedPlan to be called with no snapshot
yet set --- at least the protocol Describe messages can do that. I
don't want Describe to force a snapshot because that would be bad for
cases like LOCK TABLE at the start of a serializable transaction, so
RevalidateCachedPlan had better be able to cope with this case.

Since the "typical" case in which no replan is necessary won't touch
the snapshot, I think we'd better adopt the rule that
RevalidateCachedPlan never causes any caller-visible change in
ActiveSnapshot, else we'll be risking very-hard-to-reproduce bugs.
So my proposal is that RevalidateCachedPlan should set a snapshot for
itself if it needs to replan and ActiveSnapshot is NULL (else it might
as well just use the existing snap); and that it should save and restore
ActiveSnapshot when it does this.

regards, tom lane