MIME-Version: 1.0
From: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
Date: Sat, 25 Apr 2026 00:44:01 -0700
Message-ID: 
 <CAHg+QDf+fJsyT_Oj6xDCGkyKWMQ=oEjLdxZ_K4WmKApQWP8hjA@mail.gmail.com>
Subject: Discarded adjust_relid_set() return values in remove_self_join_rel
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/mixed; boundary="00000000000032fff70650440c42"
Archived-At: 
 <https://www.postgresql.org/message-id/CAHg%2BQDf%2BfJsyT_Oj6xDCGkyKWMQ%3DoEjLdxZ_K4WmKApQWP8hjA%40mail.gmail.com>
Precedence: bulk

--00000000000032fff70650440c42
Content-Type: multipart/alternative; boundary="00000000000032fff50650440c40"

--00000000000032fff50650440c40
Content-Type: text/plain; charset="UTF-8"

Hi hackers,

I found that remove_self_join_rel() in analyzejoins.c discards the
return values of two adjust_relid_set() calls when updating
root->all_result_relids and root->leaf_result_relids:

    adjust_relid_set(root->all_result_relids, toRemove->relid,
toKeep->relid);
    adjust_relid_set(root->leaf_result_relids, toRemove->relid,
toKeep->relid);

adjust_relid_set() does not modify its input in-place.  When it finds
the old relid in the set, it calls bms_copy() to create a new
Bitmapset, performs the substitution on the copy, and returns the new
pointer.  Without capturing the return value, the original sets are
never updated. Every other call site in the file  analyzejoins.c correctly
assigns the return value.

I am not able to craft a query to show it as a problem.
For the current release, these two lines appears to be a no-op?

Attached a patch to assign the returned values.

Thanks,
Satya

--00000000000032fff50650440c40
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Hi hackers,<br><br>I found that remove_self_join_rel() in =
analyzejoins.c discards the<br>return values of two adjust_relid_set() call=
s when updating<br>root-&gt;all_result_relids and root-&gt;leaf_result_reli=
ds:<br><br>=C2=A0 =C2=A0 adjust_relid_set(root-&gt;all_result_relids, toRem=
ove-&gt;relid, toKeep-&gt;relid);<br>=C2=A0 =C2=A0 adjust_relid_set(root-&g=
t;leaf_result_relids, toRemove-&gt;relid, toKeep-&gt;relid);<br><br>adjust_=
relid_set() does not modify its input in-place.=C2=A0 When it finds<br>the =
old relid in the set, it calls bms_copy() to create a new<br>Bitmapset, per=
forms the substitution on the copy, and returns the new<br>pointer.=C2=A0 W=
ithout capturing the return value, the original sets are<br>never updated. =
Every other call site in the file =C2=A0analyzejoins.c correctly<br>assigns=
 the return value.<br><br>I am not able to craft a query to show it as a pr=
oblem.<br>For the current release, these two lines appears to be a no-op?<b=
r><br>Attached a patch to assign the returned values.<br><br>Thanks,<br>Sat=
ya<br></div>

--00000000000032fff50650440c40--

--00000000000032fff70650440c42
Content-Type: application/octet-stream;
	name="0001-Fix-discarded-adjust_relid_set-return-values-in-SJE.patch"
Content-Disposition: attachment;
	filename="0001-Fix-discarded-adjust_relid_set-return-values-in-SJE.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_moe18h5e0>
X-Attachment-Id: f_moe18h5e0

RnJvbSAyMTFjYWY0ZDYwNDhjMzMyYjYxMzAyMmNiOTY4Y2RjZDA1ZjVjNTAzIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBTYXR5YSBOYXJsYXB1cmFtIDxzYXR5YW5hcmxhcHVyYW1AZ21h
aWwuY29tPgpEYXRlOiBTYXQsIDI1IEFwciAyMDI2IDA3OjMwOjEzICswMDAwClN1YmplY3Q6IFtQ
QVRDSF0gRml4IGRpc2NhcmRlZCBhZGp1c3RfcmVsaWRfc2V0KCkgcmV0dXJuIHZhbHVlcyBpbiBT
SkUKCnJlbW92ZV9zZWxmX2pvaW5fcmVsKCkgY2FsbHMgYWRqdXN0X3JlbGlkX3NldCgpIHRvIHVw
ZGF0ZQpyb290LT5hbGxfcmVzdWx0X3JlbGlkcyBhbmQgcm9vdC0+bGVhZl9yZXN1bHRfcmVsaWRz
IGFmdGVyCmVsaW1pbmF0aW5nIGEgc2VsZi1qb2luZWQgcmVsYXRpb24sIGJ1dCBkaXNjYXJkcyB0
aGUgcmV0dXJuIHZhbHVlcy4KCmFkanVzdF9yZWxpZF9zZXQoKSBhbGxvY2F0ZXMgYSBuZXcgQml0
bWFwc2V0IHZpYSBibXNfY29weSgpIHdoZW4KaXQgZmluZHMgdGhlIG9sZCByZWxpZCBpbiB0aGUg
c2V0LCBzbyB0aGUgY2FsbGVyIG11c3QgY2FwdHVyZSB0aGUKcmV0dXJuZWQgcG9pbnRlci4gIEV2
ZXJ5IG90aGVyIGNhbGwgc2l0ZSBpbiB0aGUgZmlsZSAoMjArIGluc3RhbmNlcykKY29ycmVjdGx5
IGFzc2lnbnMgdGhlIHJldHVybiB2YWx1ZS4KCi0tLQogc3JjL2JhY2tlbmQvb3B0aW1pemVyL3Bs
YW4vYW5hbHl6ZWpvaW5zLmMgfCA4ICsrKysrKy0tCiAxIGZpbGUgY2hhbmdlZCwgNiBpbnNlcnRp
b25zKCspLCAyIGRlbGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL29wdGltaXpl
ci9wbGFuL2FuYWx5emVqb2lucy5jIGIvc3JjL2JhY2tlbmQvb3B0aW1pemVyL3BsYW4vYW5hbHl6
ZWpvaW5zLmMKaW5kZXggMDMwNTZiZGYuLjRmYTNhYmM3IDEwMDY0NAotLS0gYS9zcmMvYmFja2Vu
ZC9vcHRpbWl6ZXIvcGxhbi9hbmFseXplam9pbnMuYworKysgYi9zcmMvYmFja2VuZC9vcHRpbWl6
ZXIvcGxhbi9hbmFseXplam9pbnMuYwpAQCAtMTk5NCw4ICsxOTk0LDEyIEBAIHJlbW92ZV9zZWxm
X2pvaW5fcmVsKFBsYW5uZXJJbmZvICpyb290LCBQbGFuUm93TWFyayAqa21hcmssIFBsYW5Sb3dN
YXJrICpybWFyaywKIAlDaGFuZ2VWYXJOb2Rlc0V4dGVuZGVkKChOb2RlICopIHJvb3QtPnByb2Nl
c3NlZF90bGlzdCwgdG9SZW1vdmUtPnJlbGlkLAogCQkJCQkJICAgdG9LZWVwLT5yZWxpZCwgMCwg
cmVwbGFjZV9yZWxpZF9jYWxsYmFjayk7CiAKLQlhZGp1c3RfcmVsaWRfc2V0KHJvb3QtPmFsbF9y
ZXN1bHRfcmVsaWRzLCB0b1JlbW92ZS0+cmVsaWQsIHRvS2VlcC0+cmVsaWQpOwotCWFkanVzdF9y
ZWxpZF9zZXQocm9vdC0+bGVhZl9yZXN1bHRfcmVsaWRzLCB0b1JlbW92ZS0+cmVsaWQsIHRvS2Vl
cC0+cmVsaWQpOworCXJvb3QtPmFsbF9yZXN1bHRfcmVsaWRzID0gYWRqdXN0X3JlbGlkX3NldChy
b290LT5hbGxfcmVzdWx0X3JlbGlkcywKKwkJCQkJCQkJCQkJICAgdG9SZW1vdmUtPnJlbGlkLAor
CQkJCQkJCQkJCQkgICB0b0tlZXAtPnJlbGlkKTsKKwlyb290LT5sZWFmX3Jlc3VsdF9yZWxpZHMg
PSBhZGp1c3RfcmVsaWRfc2V0KHJvb3QtPmxlYWZfcmVzdWx0X3JlbGlkcywKKwkJCQkJCQkJCQkJ
CXRvUmVtb3ZlLT5yZWxpZCwKKwkJCQkJCQkJCQkJCXRvS2VlcC0+cmVsaWQpOwogCiAJLyoKIAkg
KiBUaGVyZSBtYXkgYmUgcmVmZXJlbmNlcyB0byB0aGUgcmVsIGluIHJvb3QtPmZrZXlfbGlzdCwg
YnV0IGlmIHNvLAotLSAKMi40My4wCgo=
--00000000000032fff70650440c42--