Use of ActiveSnapshot

The use of ActiveSnapshot throughout the code appears rather dangerous
to me. It is a global pointer, assumed not to be set yet in some places,
assumed to be saved and restored by the caller in others. The actual
(context) memory it points to is sometimes explicitly freed, sometimes
just left in the context and thrown away by MemoryContextDelete()
without resetting ActiveSnapshot to NULL.

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.

Either calling pg_plan_queries() with needSnapshot=false or saving and
restoring ActiveSnapshot will prevent the backend from dumping core in
the mentioned example, but I am not entirely sure as to which one is the
right solution.

Jan

--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck(at)Yahoo(dot)com #