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:
parent
ae99b3cca5
commit
1de8584fb1
@ -17,6 +17,7 @@
|
||||
#include "optimizer/joininfo.h"
|
||||
#include "optimizer/pathnode.h"
|
||||
#include "optimizer/paths.h"
|
||||
#include "utils/memutils.h"
|
||||
|
||||
|
||||
static List *make_rels_by_clause_joins(PlannerInfo *root,
|
||||
@ -947,11 +948,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;
|
||||
|
||||
@ -963,6 +985,8 @@ mark_dummy_rel(RelOptInfo *rel)
|
||||
|
||||
/* Set or update cheapest_total_path */
|
||||
set_cheapest(rel);
|
||||
|
||||
MemoryContextSwitchTo(oldcontext);
|
||||
}
|
||||
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user