From eca75a12a27d28b972fc269c1c8813cd8eb15441 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 13 Apr 2011 18:56:40 -0400 Subject: [PATCH] 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.) --- src/backend/optimizer/path/joinrels.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c index bbb79c582d..42427d3aca 100644 --- a/src/backend/optimizer/path/joinrels.c +++ b/src/backend/optimizer/path/joinrels.c @@ -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); }