On 5/14/2007 1:29 PM, Tom Lane wrote:
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.
The only problem with that is that there are code paths that set
ActiveSnapshot to palloc()'d memory that is released due to a
MemoryContextDelete() without resetting ActiveSnapshot to NULL. So it
might be possible for RevalidateCachedPlan to go ahead with an
ActiveSnapshot pointing to garbage.
I think it would be cleaner if RevalidateCachedPlan()'s API would have a
Snapshot argument. If it needs a snapshot and the argument is NULL, it
can create (and free) one itself, otherwise it'd use the one given.
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== [EMAIL PROTECTED] #
---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings
