agora inbox for pgsql-committers@postgresql.org  
help / 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
------
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

* 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
------
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


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