Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit d1c23d7

Browse files
committed
Fix two latent(?) bugs in equivclass.c.
get_eclass_for_sort_expr() computes expr_relids and nullable_relids early on, even though they won't be needed unless we make a new EquivalenceClass, which we often don't. Aside from the probably-minor inefficiency, there's a memory management problem: these bitmapsets will be built in the caller's context, leading to dangling pointers if that is shorter-lived than root->planner_cxt. This would be a live bug if get_eclass_for_sort_expr() could be called with create_it = true during GEQO join planning. So far as I can find, the core code never does that, but it's hard to be sure that no extensions do, especially since the comments make it clear that that's supposed to be a supported case. Fix by not computing these values until we've switched into planner_cxt to build the new EquivalenceClass. generate_join_implied_equalities() uses inner_rel->relids to look up relevant eclasses, but it ought to be using nominal_inner_relids. This is presently harmless because a child RelOptInfo will always have exactly the same eclass_indexes as its topmost parent; but that might not be true forever, and anyway it makes the code confusing. The first of these is old (introduced by me in f3b3b8d), so back-patch to all supported branches. The second only dates to v13, but we might as well back-patch it to keep the code looking similar across branches. Discussion: https://postgr.es/m/1508010.1601832581@sss.pgh.pa.us
1 parent 019eb96 commit d1c23d7

File tree

1 file changed

+8
-8
lines changed

1 file changed

+8
-8
lines changed

src/backend/optimizer/path/equivclass.c

+8-8
Original file line numberDiff line numberDiff line change
@@ -634,12 +634,6 @@ get_eclass_for_sort_expr(PlannerInfo *root,
634634
*/
635635
expr = canonicalize_ec_expression(expr, opcintype, collation);
636636

637-
/*
638-
* Get the precise set of nullable relids appearing in the expression.
639-
*/
640-
expr_relids = pull_varnos((Node *) expr);
641-
nullable_relids = bms_intersect(nullable_relids, expr_relids);
642-
643637
/*
644638
* Scan through the existing EquivalenceClasses for a match
645639
*/
@@ -716,6 +710,12 @@ get_eclass_for_sort_expr(PlannerInfo *root,
716710
if (newec->ec_has_volatile && sortref == 0) /* should not happen */
717711
elog(ERROR, "volatile EquivalenceClass has no sortref");
718712

713+
/*
714+
* Get the precise set of nullable relids appearing in the expression.
715+
*/
716+
expr_relids = pull_varnos((Node *) expr);
717+
nullable_relids = bms_intersect(nullable_relids, expr_relids);
718+
719719
newem = add_eq_member(newec, copyObject(expr), expr_relids,
720720
nullable_relids, false, opcintype);
721721

@@ -1171,9 +1171,9 @@ generate_join_implied_equalities(PlannerInfo *root,
11711171
}
11721172

11731173
/*
1174-
* Get all eclasses in common between inner_rel's relids and outer_relids
1174+
* Get all eclasses that mention both inner and outer sides of the join
11751175
*/
1176-
matching_ecs = get_common_eclass_indexes(root, inner_rel->relids,
1176+
matching_ecs = get_common_eclass_indexes(root, nominal_inner_relids,
11771177
outer_relids);
11781178

11791179
i = -1;

0 commit comments

Comments
 (0)