agora inbox for pgsql-committers@postgresql.orghelp / color / mirror / Atom feed
pgsql: Fix relid-set clobber during join removal. 3+ messages / 1 participants [nested] [flat]
* pgsql: Fix relid-set clobber during join removal. @ 2026-04-20 23:25 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 3+ messages in thread From: Tom Lane @ 2026-04-20 23:25 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix relid-set clobber during join removal. Commit cfcd57111 et al fell over under Valgrind testing. (It seems to be enough to #define USE_VALGRIND, you don't actually need to run it under Valgrind to see failures.) The cause is that remove_rel_from_eclass updates each EquivalenceMember's em_relids, and those can be aliases of the left_relids or right_relids of some RestrictInfo in ec_sources. If the update made em_relids empty then bms_del_member will have pfree'd the relid set, so that the subsequent attempt to clean up ec_sources accesses already-freed memory. We missed seeing ill effects before cfcd57111 because (a) if the pfree happens then we will remove the EquivalenceMember altogether, making the source RestrictInfo no longer of use, and (b) the cleanup of ec_sources didn't touch left/right_relids before that. I'm unclear though on how cfcd57111 managed to pass non-USE_VALGRIND testing. Apparently we managed to store another Bitmapset into the freed space before trying to access it, but you'd not think that would happen 100% of the time. I think what USE_VALGRIND changes is that it makes list.c much more memory-hungry, so that the freed space gets claimed by some List node before a Bitmapset can be put there. This failure can be seen in v16, v17, and master, but oddly enough not v18. That's because the SJE patch replaced the simple bms_del_members calls used here with adjust_relid_set, which is careful not to scribble on its input. But commit 20efbdffe just recently put back the old coding and thus resurrected the problem. Discussion: https://postgr.es/m/458729.1776724816@sss.pgh.pa.us Backpatch-through: 16, 17, master Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/798dabe8388764a8a9979f5c91237f807cd09188 Modified Files -------------- src/backend/optimizer/plan/analyzejoins.c | 2 ++ 1 file changed, 2 insertions(+) ^ permalink raw reply [nested|flat] 3+ messages in thread
* pgsql: Fix relid-set clobber during join removal. @ 2026-04-20 23:25 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 3+ messages in thread From: Tom Lane @ 2026-04-20 23:25 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix relid-set clobber during join removal. Commit cfcd57111 et al fell over under Valgrind testing. (It seems to be enough to #define USE_VALGRIND, you don't actually need to run it under Valgrind to see failures.) The cause is that remove_rel_from_eclass updates each EquivalenceMember's em_relids, and those can be aliases of the left_relids or right_relids of some RestrictInfo in ec_sources. If the update made em_relids empty then bms_del_member will have pfree'd the relid set, so that the subsequent attempt to clean up ec_sources accesses already-freed memory. We missed seeing ill effects before cfcd57111 because (a) if the pfree happens then we will remove the EquivalenceMember altogether, making the source RestrictInfo no longer of use, and (b) the cleanup of ec_sources didn't touch left/right_relids before that. I'm unclear though on how cfcd57111 managed to pass non-USE_VALGRIND testing. Apparently we managed to store another Bitmapset into the freed space before trying to access it, but you'd not think that would happen 100% of the time. I think what USE_VALGRIND changes is that it makes list.c much more memory-hungry, so that the freed space gets claimed by some List node before a Bitmapset can be put there. This failure can be seen in v16, v17, and master, but oddly enough not v18. That's because the SJE patch replaced the simple bms_del_members calls used here with adjust_relid_set, which is careful not to scribble on its input. But commit 20efbdffe just recently put back the old coding and thus resurrected the problem. Discussion: https://postgr.es/m/458729.1776724816@sss.pgh.pa.us Backpatch-through: 16, 17, master Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/53cb4ec1ded7537770c68f709416de068a3e40d5 Modified Files -------------- src/backend/optimizer/plan/analyzejoins.c | 2 ++ 1 file changed, 2 insertions(+) ^ permalink raw reply [nested|flat] 3+ messages in thread
* pgsql: Fix relid-set clobber during join removal. @ 2026-04-20 23:25 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 3+ messages in thread From: Tom Lane @ 2026-04-20 23:25 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix relid-set clobber during join removal. Commit cfcd57111 et al fell over under Valgrind testing. (It seems to be enough to #define USE_VALGRIND, you don't actually need to run it under Valgrind to see failures.) The cause is that remove_rel_from_eclass updates each EquivalenceMember's em_relids, and those can be aliases of the left_relids or right_relids of some RestrictInfo in ec_sources. If the update made em_relids empty then bms_del_member will have pfree'd the relid set, so that the subsequent attempt to clean up ec_sources accesses already-freed memory. We missed seeing ill effects before cfcd57111 because (a) if the pfree happens then we will remove the EquivalenceMember altogether, making the source RestrictInfo no longer of use, and (b) the cleanup of ec_sources didn't touch left/right_relids before that. I'm unclear though on how cfcd57111 managed to pass non-USE_VALGRIND testing. Apparently we managed to store another Bitmapset into the freed space before trying to access it, but you'd not think that would happen 100% of the time. I think what USE_VALGRIND changes is that it makes list.c much more memory-hungry, so that the freed space gets claimed by some List node before a Bitmapset can be put there. This failure can be seen in v16, v17, and master, but oddly enough not v18. That's because the SJE patch replaced the simple bms_del_members calls used here with adjust_relid_set, which is careful not to scribble on its input. But commit 20efbdffe just recently put back the old coding and thus resurrected the problem. Discussion: https://postgr.es/m/458729.1776724816@sss.pgh.pa.us Backpatch-through: 16, 17, master Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/f0ac6d494b56b83cf49d328ee0c5dd20df937fce Modified Files -------------- src/backend/optimizer/plan/analyzejoins.c | 2 ++ 1 file changed, 2 insertions(+) ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-04-20 23:25 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-04-20 23:25 pgsql: Fix relid-set clobber during join removal. Tom Lane <tgl@sss.pgh.pa.us> 2026-04-20 23:25 pgsql: Fix relid-set clobber during join removal. Tom Lane <tgl@sss.pgh.pa.us> 2026-04-20 23:25 pgsql: Fix relid-set clobber during join removal. Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox