Fix planner's use of Result Cache with unique joins

david-rowley · david-rowley · commit 9e215378d7fb · 2021-05-22T16:22:27.000+12:00
When the planner considered using a Result Cache node to cache results from the inner side of a Nested Loop Join, it failed to consider that the inner path's parameterization may not be the entire join condition. If the join was marked as inner_unique then we may accidentally put the cache in singlerow mode. This meant that entries would be marked as complete after caching the first row. That was wrong as if only part of the join condition was parameterized then the uniqueness of the unique join was not guaranteed at the Result Cache's level. The uniqueness is only guaranteed after Nested Loop applies the join filter. If subsequent rows were found, this would lead to: ERROR: cache entry already complete This could have been fixed by only putting the cache in singlerow mode if the entire join condition was parameterized. However, Nested Loop will only read its inner side so far as the first matching row when the join is unique, so that might mean we never get an opportunity to mark cache entries as complete. Since non-complete cache entries are useless for subsequent lookups, we just don't bother considering a Result Cache path in this case. In passing, remove the XXX comment that claimed the above ERROR might be better suited to be an Assert. After there being an actual case which triggered it, it seems better to keep it an ERROR. Reported-by: David Christensen Discussion: https://postgr.es/m/CAOxo6X+dy-V58iEPFgst8ahPKEU+38NZzUuc+a7wDBZd4TrHMQ@mail.gmail.com
diff --git a/src/backend/executor/nodeResultCache.c b/src/backend/executor/nodeResultCache.c
@@ -760,7 +760,7 @@ ExecResultCache(PlanState *pstate)
 				/*
 				 * Validate if the planner properly set the singlerow flag. It
 				 * should only set that if each cache entry can, at most,
-				 * return 1 row.  XXX maybe this should be an Assert?
+				 * return 1 row.
 				 */
 				if (unlikely(entry->complete))
 					elog(ERROR, "cache entry already complete");
diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
@@ -503,6 +503,37 @@ get_resultcache_path(PlannerInfo *root, RelOptInfo *innerrel,
 								 jointype == JOIN_ANTI))
 		return NULL;
 
+	/*
+	 * Result Cache normally marks cache entries as complete when it runs out
+	 * of tuples to read from its subplan.  However, with unique joins, Nested
+	 * Loop will skip to the next outer tuple after finding the first matching
+	 * inner tuple.  This means that we may not read the inner side of the
+	 * join to completion which leaves no opportunity to mark the cache entry
+	 * as complete.  To work around that, when the join is unique we
+	 * automatically mark cache entries as complete after fetching the first
+	 * tuple.  This works when the entire join condition is parameterized.
+	 * Otherwise, when the parameterization is only a subset of the join
+	 * condition, we can't be sure which part of it causes the join to be
+	 * unique.  This means there are no guarantees that only 1 tuple will be
+	 * read.  We cannot mark the cache entry as complete after reading the
+	 * first tuple without that guarantee.  This means the scope of Result
+	 * Cache's usefulness is limited to only outer rows that have no join
+	 * partner as this is the only case where Nested Loop would exhaust the
+	 * inner scan of a unique join.  Since the scope is limited to that, we
+	 * just don't bother making a result cache path in this case.
+	 *
+	 * Lateral vars needn't be considered here as they're not considered when
+	 * determining if the join is unique.
+	 *
+	 * XXX this could be enabled if the remaining join quals were made part of
+	 * the inner scan's filter instead of the join filter.  Maybe it's worth
+	 * considering doing that?
+	 */
+	if (extra->inner_unique &&
+		list_length(inner_path->param_info->ppi_clauses) <
+		list_length(extra->restrictlist))
+		return NULL;
+
 	/*
 	 * We can't use a result cache if there are volatile functions in the
 	 * inner rel's target list or restrict list.  A cache hit could reduce the
