Commit d7885b1f authored by Tom Lane's avatar Tom Lane

Build EC members for child join rels in the right memory context.

This patch prevents crashes or wrong plans when partition-wise joins
are considered during GEQO planning, as a consequence of the
EquivalenceClass data structures becoming corrupt after a GEQO
context reset.

A remaining problem is that successive GEQO cycles will make multiple
copies of the required EC members, since add_child_join_rel_equivalences
has no idea that such members might exist already.  For now we'll just
live with that.  The lack of field complaints of crashes suggests that
this is a mighty little-used situation.

Back-patch to v12 where this code was introduced.

Discussion: https://postgr.es/m/1683100.1601860653@sss.pgh.pa.us
parent 1b222249
...@@ -2380,6 +2380,7 @@ add_child_join_rel_equivalences(PlannerInfo *root, ...@@ -2380,6 +2380,7 @@ add_child_join_rel_equivalences(PlannerInfo *root,
Relids top_parent_relids = child_joinrel->top_parent_relids; Relids top_parent_relids = child_joinrel->top_parent_relids;
Relids child_relids = child_joinrel->relids; Relids child_relids = child_joinrel->relids;
Bitmapset *matching_ecs; Bitmapset *matching_ecs;
MemoryContext oldcontext;
int i; int i;
Assert(IS_JOIN_REL(child_joinrel) && IS_JOIN_REL(parent_joinrel)); Assert(IS_JOIN_REL(child_joinrel) && IS_JOIN_REL(parent_joinrel));
...@@ -2387,6 +2388,16 @@ add_child_join_rel_equivalences(PlannerInfo *root, ...@@ -2387,6 +2388,16 @@ add_child_join_rel_equivalences(PlannerInfo *root,
/* We need consider only ECs that mention the parent joinrel */ /* We need consider only ECs that mention the parent joinrel */
matching_ecs = get_eclass_indexes_for_relids(root, top_parent_relids); matching_ecs = get_eclass_indexes_for_relids(root, top_parent_relids);
/*
* If we're being called during GEQO join planning, we still have to
* create any new EC members in the main planner context, to avoid having
* a corrupt EC data structure after the GEQO context is reset. This is
* problematic since we'll leak memory across repeated GEQO cycles. For
* now, though, bloat is better than crash. If it becomes a real issue
* we'll have to do something to avoid generating duplicate EC members.
*/
oldcontext = MemoryContextSwitchTo(root->planner_cxt);
i = -1; i = -1;
while ((i = bms_next_member(matching_ecs, i)) >= 0) while ((i = bms_next_member(matching_ecs, i)) >= 0)
{ {
...@@ -2486,6 +2497,8 @@ add_child_join_rel_equivalences(PlannerInfo *root, ...@@ -2486,6 +2497,8 @@ add_child_join_rel_equivalences(PlannerInfo *root,
} }
} }
} }
MemoryContextSwitchTo(oldcontext);
} }
......
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