"Kevin Grittner" <kevin.gritt...@wicourts.gov> writes:
> *** a/src/backend/storage/lmgr/predicate.c
> --- b/src/backend/storage/lmgr/predicate.c
> ***************
> *** 274,279 ****
> --- 274,280 ----
>   #define SkipSerialization(relation) \
>       ((!IsolationIsSerializable()) \
>       || ((MySerializableXact == InvalidSerializableXact)) \
> +     || (!IsMVCCSnapshot(GetActiveSnapshot())) \
>       || ReleasePredicateLocksIfROSafe() \
>       || SkipPredicateLocksForRelation(relation))
  

While I agree with the goal here, this implementation seems fairly
dangerous.  The recommendation was to check *the snapshot being used in
the scan*, and I think you have to do it that way.  This macro isn't
necessarily checking the right snapshot.  What's more, if it's ever used
in a place where there is no "active" snapshot, it'd dump core outright.

I think you probably need to add the snapshot as an explicit parameter
to the macro if you're going to do this.

BTW, am I reading the function names right to suspect that
ReleasePredicateLocksIfROSafe might be something with side-effects?
Yuck.

                        regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to