OSDN Git Service

Ensure mark_dummy_rel doesn't create dangling pointers in RelOptInfos.
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 13 Apr 2011 22:56:47 +0000 (18:56 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 13 Apr 2011 22:56:47 +0000 (18:56 -0400)
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

index 3b65940..5375dd9 100644 (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,
@@ -933,11 +934,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;
 
@@ -949,6 +971,8 @@ mark_dummy_rel(RelOptInfo *rel)
 
        /* Set or update cheapest_total_path */
        set_cheapest(rel);
+
+       MemoryContextSwitchTo(oldcontext);
 }