Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit f6a2334

Browse filesBrowse 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 cf42550 commit f6a2334
Copy full SHA for f6a2334

File tree

Expand file treeCollapse file tree

1 file changed

+6
-6
lines changed
Filter options
Expand file treeCollapse file tree

1 file changed

+6
-6
lines changed

‎src/backend/optimizer/path/equivclass.c

Copy file name to clipboardExpand all lines: src/backend/optimizer/path/equivclass.c
+6-6Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -565,12 +565,6 @@ get_eclass_for_sort_expr(PlannerInfo *root,
565565
*/
566566
expr = canonicalize_ec_expression(expr, opcintype, collation);
567567

568-
/*
569-
* Get the precise set of nullable relids appearing in the expression.
570-
*/
571-
expr_relids = pull_varnos((Node *) expr);
572-
nullable_relids = bms_intersect(nullable_relids, expr_relids);
573-
574568
/*
575569
* Scan through the existing EquivalenceClasses for a match
576570
*/
@@ -645,6 +639,12 @@ get_eclass_for_sort_expr(PlannerInfo *root,
645639
if (newec->ec_has_volatile && sortref == 0) /* should not happen */
646640
elog(ERROR, "volatile EquivalenceClass has no sortref");
647641

642+
/*
643+
* Get the precise set of nullable relids appearing in the expression.
644+
*/
645+
expr_relids = pull_varnos((Node *) expr);
646+
nullable_relids = bms_intersect(nullable_relids, expr_relids);
647+
648648
newem = add_eq_member(newec, copyObject(expr), expr_relids,
649649
nullable_relids, false, opcintype);
650650

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.