Re: reindex creates predicate lock on index root

"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)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