Ensure mark_dummy_rel doesn't create dangling pointers in RelOptInfos.

When we are doing GEQO join planning, the current memory context is a
short-lived context that will be reset at the end of geqo_eval().  However,
the RelOptInfos for base relations are set up before that and then re-used
across many GEQO cycles.  Hence, any code that modifies a baserel during
join planning has to be careful not to put pointers to the short-lived
context into the baserel struct.  mark_dummy_rel got this wrong, leading to
easy-to-reproduce-once-you-know-how crashes in 8.4, as reported off-list by
Leo Carson of SDSC.  Some improvements made in 9.0 make it difficult to
demonstrate the crash in 9.0 or HEAD; but there's no doubt that there's
still a risk factor here, so patch all branches that have the function.
(Note: 8.3 has a similar function, but it's only applied to joinrels and
thus is not a hazard.)
This commit is contained in:
Tom Lane 2011-04-13 18:56:40 -04:00
parent 170aeb5407
commit eca75a12a2
1 changed files with 25 additions and 1 deletions

View File

@ -17,6 +17,7 @@
#include "optimizer/joininfo.h"
#include "optimizer/pathnode.h"
#include "optimizer/paths.h"
#include "utils/memutils.h"
static void make_rels_by_clause_joins(PlannerInfo *root,
@ -945,11 +946,32 @@ is_dummy_rel(RelOptInfo *rel)
}
/*
* Mark a rel as proven empty.
* Mark a relation as proven empty.
*
* During GEQO planning, this can get invoked more than once on the same
* baserel struct, so it's worth checking to see if the rel is already marked
* dummy.
*
* Also, when called during GEQO join planning, we are in a short-lived
* memory context. We must make sure that the dummy path attached to a
* baserel survives the GEQO cycle, else the baserel is trashed for future
* GEQO cycles. On the other hand, when we are marking a joinrel during GEQO,
* we don't want the dummy path to clutter the main planning context. Upshot
* is that the best solution is to explicitly make the dummy path in the same
* context the given RelOptInfo is in.
*/
static void
mark_dummy_rel(RelOptInfo *rel)
{
MemoryContext oldcontext;
/* Already marked? */
if (is_dummy_rel(rel))
return;
/* No, so choose correct context to make the dummy path in */
oldcontext = MemoryContextSwitchTo(GetMemoryChunkContext(rel));
/* Set dummy size estimate */
rel->rows = 0;
@ -961,6 +983,8 @@ mark_dummy_rel(RelOptInfo *rel)
/* Set or update cheapest_total_path */
set_cheapest(rel);
MemoryContextSwitchTo(oldcontext);
}