Commit 9e215378 authored by David Rowley's avatar David Rowley

Fix planner's use of Result Cache with unique joins

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
parent 0cdaa05b
...@@ -760,7 +760,7 @@ ExecResultCache(PlanState *pstate) ...@@ -760,7 +760,7 @@ ExecResultCache(PlanState *pstate)
/* /*
* Validate if the planner properly set the singlerow flag. It * Validate if the planner properly set the singlerow flag. It
* should only set that if each cache entry can, at most, * 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)) if (unlikely(entry->complete))
elog(ERROR, "cache entry already complete"); elog(ERROR, "cache entry already complete");
......
...@@ -503,6 +503,37 @@ get_resultcache_path(PlannerInfo *root, RelOptInfo *innerrel, ...@@ -503,6 +503,37 @@ get_resultcache_path(PlannerInfo *root, RelOptInfo *innerrel,
jointype == JOIN_ANTI)) jointype == JOIN_ANTI))
return NULL; 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 * 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 * inner rel's target list or restrict list. A cache hit could reduce the
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment