Re: reindex creates predicate lock on index root

"Kevin Grittner" <Kevin.Grittner@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