Shut down EvalPlanQual machinery when LockRows node reaches the end.

Previously, we left the EPQ sub-executor alone until ExecEndLockRows.
This caused any buffer pins or other resources that it might hold to
remain held until ExecutorEnd, which in some code paths means that
they are held till the Portal is closed.  That can cause user-visible
problems, such as blocking VACUUM; and it's unlike the behavior of
ordinary table-scanning nodes, which will have released all buffer
pins by the time they return an EOF indication.

We can make LockRows work more like other plan nodes by calling
EvalPlanQualEnd just before returning NULL.  We still need to call it
in ExecEndLockRows in case the node was not run to completion, but in
the normal case the second call does nothing and costs little.

Per report from Yura Sokolov.  In principle this is a longstanding
bug, but in view of the lack of other complaints and the low severity
of the consequences, I chose not to back-patch.

Discussion: https://postgr.es/m/4aa370cb91ecf2f9885d98b80ad1109c@postgrespro.ru

diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c
index b2e5c30079e58760b3eb54781a6328f22e8e5510..7583973f4ae069e145d61540a57041729074ce24 100644 (file)
--- a/src/backend/executor/nodeLockRows.c
+++ b/src/backend/executor/nodeLockRows.c
@@ -59,7 +59,11 @@ lnext:
    slot = ExecProcNode(outerPlan);
 
    if (TupIsNull(slot))
+   {
+       /* Release any resources held by EPQ mechanism before exiting */
+       EvalPlanQualEnd(&node->lr_epqstate);
        return NULL;
+   }
 
    /* We don't need EvalPlanQual unless we get updated tuple version(s) */
    epq_needed = false;
@@ -381,6 +385,7 @@ ExecInitLockRows(LockRows *node, EState *estate, int eflags)
 void
 ExecEndLockRows(LockRowsState *node)
 {
+   /* We may have shut down EPQ already, but no harm in another call */
    EvalPlanQualEnd(&node->lr_epqstate);
    ExecEndNode(outerPlanState(node));
 }