Prohibit shutting down resources if there is a possibility of back up.

Amit Kapila · Amit Kapila · commit 2cd0acfdade8 · 2018-08-13T08:22:18.000+05:30
Currently, we release the asynchronous resources as soon as it is evident that no more rows will be needed e.g. when a Limit is filled. This can be problematic especially for custom and foreign scans where we can scan backward. Fix that by disallowing the shutting down of resources in such cases. Reported-by: Robert Haas Analysed-by: Robert Haas and Amit Kapila Author: Amit Kapila Reviewed-by: Robert Haas Backpatch-through: 9.6 where this code was introduced Discussion: https://postgr.es/m/86137f17-1dfb-42f9-7421-82fd786b04a1@anayrat.info
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
@@ -1726,8 +1726,12 @@ ExecutePlan(EState *estate,
 		 */
 		if (TupIsNull(slot))
 		{
-			/* Allow nodes to release or shut down resources. */
-			(void) ExecShutdownNode(planstate);
+			/*
+			 * If we know we won't need to back up, we can release
+			 * resources at this point.
+			 */
+			if (!(estate->es_top_eflags & EXEC_FLAG_BACKWARD))
+				(void) ExecShutdownNode(planstate);
 			break;
 		}
 
@@ -1773,8 +1777,12 @@ ExecutePlan(EState *estate,
 		current_tuple_count++;
 		if (numberTuples && numberTuples == current_tuple_count)
 		{
-			/* Allow nodes to release or shut down resources. */
-			(void) ExecShutdownNode(planstate);
+			/*
+			 * If we know we won't need to back up, we can release
+			 * resources at this point.
+			 */
+			if (!(estate->es_top_eflags & EXEC_FLAG_BACKWARD))
+				(void) ExecShutdownNode(planstate);
 			break;
 		}
 	}
diff --git a/src/backend/executor/nodeLimit.c b/src/backend/executor/nodeLimit.c
@@ -134,8 +134,14 @@ ExecLimit(PlanState *pstate)
 					node->position - node->offset >= node->count)
 				{
 					node->lstate = LIMIT_WINDOWEND;
-					/* Allow nodes to release or shut down resources. */
-					(void) ExecShutdownNode(outerPlan);
+
+					/*
+					 * If we know we won't need to back up, we can release
+					 * resources at this point.
+					 */
+					if (!(node->ps.state->es_top_eflags & EXEC_FLAG_BACKWARD))
+						(void) ExecShutdownNode(outerPlan);
+
 					return NULL;
 				}
 

Original file line number	Diff line number	Diff line change
`@@ -1726,8 +1726,12 @@ ExecutePlan(EState *estate,`
`1726`	`1726`	`*/`
`1727`	`1727`	`if (TupIsNull(slot))`
`1728`	`1728`	`{`
`1729`		`- /* Allow nodes to release or shut down resources. */`
`1730`		`- (void) ExecShutdownNode(planstate);`
	`1729`	`+ /*`
	`1730`	`+ * If we know we won't need to back up, we can release`
	`1731`	`+ * resources at this point.`
	`1732`	`+ */`
	`1733`	`+ if (!(estate->es_top_eflags & EXEC_FLAG_BACKWARD))`
	`1734`	`+ (void) ExecShutdownNode(planstate);`
`1731`	`1735`	`break;`
`1732`	`1736`	`}`
`1733`	`1737`
`@@ -1773,8 +1777,12 @@ ExecutePlan(EState *estate,`
`1773`	`1777`	`current_tuple_count++;`
`1774`	`1778`	`if (numberTuples && numberTuples == current_tuple_count)`
`1775`	`1779`	`{`
`1776`		`- /* Allow nodes to release or shut down resources. */`
`1777`		`- (void) ExecShutdownNode(planstate);`
	`1780`	`+ /*`
	`1781`	`+ * If we know we won't need to back up, we can release`
	`1782`	`+ * resources at this point.`
	`1783`	`+ */`
	`1784`	`+ if (!(estate->es_top_eflags & EXEC_FLAG_BACKWARD))`
	`1785`	`+ (void) ExecShutdownNode(planstate);`
`1778`	`1786`	`break;`
`1779`	`1787`	`}`
`1780`	`1788`	`}`