pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Fix two latent(?) bugs in equivclass.c. 7+ messages / 1 participants [nested] [flat]
* pgsql: Fix two latent(?) bugs in equivclass.c. @ 2020-10-05 17:16 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2020-10-05 17:16 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 f3b3b8d5b), 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 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/53c6daff4364219256119fcd79b2d71b57c13e37 Modified Files -------------- src/backend/optimizer/path/equivclass.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix two latent(?) bugs in equivclass.c. @ 2020-10-05 17:16 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2020-10-05 17:16 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 f3b3b8d5b), 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 Branch ------ REL_13_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/d1c23d726d50e10179235b6cee6b34543a879b19 Modified Files -------------- src/backend/optimizer/path/equivclass.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix two latent(?) bugs in equivclass.c. @ 2020-10-05 17:16 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2020-10-05 17:16 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 f3b3b8d5b), 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 Branch ------ REL_12_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/1f94d76856bc7e473e300134e092eb231cba83fe Modified Files -------------- src/backend/optimizer/path/equivclass.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix two latent(?) bugs in equivclass.c. @ 2020-10-05 17:16 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2020-10-05 17:16 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 f3b3b8d5b), 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 Branch ------ REL_11_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/d5109a163f46df0dbda6125cea27b4b2951a6401 Modified Files -------------- src/backend/optimizer/path/equivclass.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix two latent(?) bugs in equivclass.c. @ 2020-10-05 17:16 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2020-10-05 17:16 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 f3b3b8d5b), 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 Branch ------ REL_10_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/74dae41e5ac39c41c902119c2101c80d400c0718 Modified Files -------------- src/backend/optimizer/path/equivclass.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix two latent(?) bugs in equivclass.c. @ 2020-10-05 17:16 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2020-10-05 17:16 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 f3b3b8d5b), 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 Branch ------ REL9_6_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/f6a23344f8c8ec155399375d048a890afaa575ab Modified Files -------------- src/backend/optimizer/path/equivclass.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix two latent(?) bugs in equivclass.c. @ 2020-10-05 17:16 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2020-10-05 17:16 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 f3b3b8d5b), 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 Branch ------ REL9_5_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/e3bd026fbb91c64e99c2a66eceb2dba3cc3ec6f0 Modified Files -------------- src/backend/optimizer/path/equivclass.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2020-10-05 17:16 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-10-05 17:16 pgsql: Fix two latent(?) bugs in equivclass.c. Tom Lane <tgl@sss.pgh.pa.us> 2020-10-05 17:16 pgsql: Fix two latent(?) bugs in equivclass.c. Tom Lane <tgl@sss.pgh.pa.us> 2020-10-05 17:16 pgsql: Fix two latent(?) bugs in equivclass.c. Tom Lane <tgl@sss.pgh.pa.us> 2020-10-05 17:16 pgsql: Fix two latent(?) bugs in equivclass.c. Tom Lane <tgl@sss.pgh.pa.us> 2020-10-05 17:16 pgsql: Fix two latent(?) bugs in equivclass.c. Tom Lane <tgl@sss.pgh.pa.us> 2020-10-05 17:16 pgsql: Fix two latent(?) bugs in equivclass.c. Tom Lane <tgl@sss.pgh.pa.us> 2020-10-05 17:16 pgsql: Fix two latent(?) bugs in equivclass.c. Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox