Jan Wieck <[EMAIL PROTECTED]> 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 ---------------------------(end of broadcast)--------------------------- TIP 2: Don't 'kill -9' the postmaster