NotificationsYou must be signed in to change notification settings
Fork6
Star31

Commit1f94d76

committed

Fix two latent(?) bugs in equivclass.c.

get_eclass_for_sort_expr() computes expr_relids and nullable_relidsearly on, even though they won't be needed unless we make a newEquivalenceClass, which we often don't. Aside from the probably-minorinefficiency, there's a memory management problem: these bitmapsets willbe built in the caller's context, leading to dangling pointers if thatis shorter-lived than root->planner_cxt. This would be a live bug ifget_eclass_for_sort_expr() could be called with create_it = true duringGEQO join planning. So far as I can find, the core code never doesthat, but it's hard to be sure that no extensions do, especially sincethe 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_cxtto build the new EquivalenceClass.generate_join_implied_equalities() uses inner_rel->relids to look uprelevant eclasses, but it ought to be using nominal_inner_relids.This is presently harmless because a child RelOptInfo will always haveexactly the same eclass_indexes as its topmost parent; but that mightnot be true forever, and anyway it makes the code confusing.The first of these is old (introduced by me inf3b3b8d), so back-patchto all supported branches. The second only dates to v13, but we mightas well back-patch it to keep the code looking similar across branches.Discussion:https://postgr.es/m/1508010.1601832581@sss.pgh.pa.us

1 parent5856ed1 commit1f94d76Copy full SHA for 1f94d76

File tree

1 file changed

-6

lines changed

src/backend/optimizer/path
- equivclass.c

1 file changed

-6

lines changed

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

Lines changed: 6 additions & 6 deletions

Original file line number	Diff line number	Diff line change
`@@ -629,12 +629,6 @@ get_eclass_for_sort_expr(PlannerInfo *root,`
`629`	`629`	`*/`
`630`	`630`	`expr=canonicalize_ec_expression(expr,opcintype,collation);`
`631`	`631`
`632`		`-/*`
`633`		`- * Get the precise set of nullable relids appearing in the expression.`
`634`		`- */`
`635`		`-expr_relids=pull_varnos((Node*)expr);`
`636`		`-nullable_relids=bms_intersect(nullable_relids,expr_relids);`
`637`		`-`
`638`	`632`	`/*`
`639`	`633`	`* Scan through the existing EquivalenceClasses for a match`
`640`	`634`	`*/`
`@@ -711,6 +705,12 @@ get_eclass_for_sort_expr(PlannerInfo *root,`
`711`	`705`	`if (newec->ec_has_volatile&&sortref==0)/* should not happen */`
`712`	`706`	`elog(ERROR,"volatile EquivalenceClass has no sortref");`
`713`	`707`
	`708`	`+/*`
	`709`	`+ * Get the precise set of nullable relids appearing in the expression.`
	`710`	`+ */`
	`711`	`+expr_relids=pull_varnos((Node*)expr);`
	`712`	`+nullable_relids=bms_intersect(nullable_relids,expr_relids);`
	`713`	`+`
`714`	`714`	`newem=add_eq_member(newec,copyObject(expr),expr_relids,`
`715`	`715`	`nullable_relids, false,opcintype);`
`716`	`716`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commit1f94d76

File tree

1 file changed

1 file changed

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

0 commit comments