Question: Should we release the FK fast-path pk_slot's buffer pin promptly?

Hi all,

While debugging an issue in AfterTriggerEndQuery [1], I wondered about
the batched FK fast-path code added by commit b7b27eb41a5: should
ri_FastPathBatchFlush release pk_slot's buffer pin immediately after
index_endscan, instead of letting it live until either the next batch's
first probe or end-of-statement teardown?

My initial thought was YES -- a stale pin sitting around looks wrong
-- but the more I look at it, the more skeptical I become.

Currently, RI_FastPathEntry caches pk_rel, idx_rel, pk_slot,
fk_slot, and a per-batch scratch MemoryContext.  The cache lives until
AfterTriggerEndQuery, when ri_FastPathEndBatch flushes the remaining
buffered rows and ri_FastPathTeardown drops everything.

Inside ri_FastPathBatchFlush, index_getnext_slot stores each matched PK
heap tuple into pk_slot.  Because pk_slot is a buffer-heap-tuple slot,
that pins the buffer and registers the pin on CurrentResourceOwner.
After index_endscan() returns, the slot still holds the last-probed
tuple; the pin stays live until either:

1) the next probe's index_getnext_slot clears the slot before storing
a new tuple, or

2) end-of-statement teardown drops the slot, releasing the pin.

So the pin survives for a while but eventually gets cleaned up.
Question is whether the gap is worth tightening with an explicit
ExecClearTuple(fpentry->pk_slot) right after index_endscan.

Reasons to favor the explicit clear:

a) Buffer eviction.  A pinned buffer is unevictable.  Nothing in
ri_triggers.c references pk_slot again before teardown, so the pin is
functionally orphaned and slightly reduces the effective working set
for the remainder of the statement.

b)  Pin-lifetime discipline. ExecStoreBufferHeapTuple registers the
pin on CurrentResourceOwner; ExecDropSingleTupleTableSlot in teardown
asks the current CurrentResourceOwner to forget it. Today they
coincide, but the gap between probe and teardown spans arbitrary user
code. Clearing the slot right after the probe closes that gap for any
future or out-of-tree caller (extensions or forks) that ends up
invoking ri_FastPathTeardown from a different ResourceOwner context.

c) pinning_backends.  VACUUM truncation and a few buffer-related
operations slow down or skip work when buffers are pinned. Holding a
fast-path pin across unrelated execution burns that budget.

But none of those seem strictly concerning for today. However, the fix
is not problematic or very complicated and seems harmless to include
to address these trivial concerns; it would have two ExecClearTuple()
calls: one in ri_FastPathBatchFlush after index_endscan, and a
defensive pre-clear in ri_FastPathTeardown. Patch attached.

Thoughts?

1] http://postgr.es/m/CAAJ_b95p6-qiVpE2Gpr=bUsNAqTcejD_rPgLnfjx9m=fo3Rf3Q@mail.gmail.com

--
Regards,
Amul Sul
EDB: http://www.enterprisedb.com