MIME-Version: 1.0
From: Jianghua Yang <yjhjstz@gmail.com>
Date: Mon, 23 Mar 2026 18:44:40 -0700
Message-ID: 
 <CAAZLFmSOD8R7tZjRLZsmpKtJLoqjgawAaM-Pne1j8B_Q2aQK8w@mail.gmail.com>
Subject: BUG: test_ginpostinglist second itemptr check is a no-op due to
 copy-paste error
To: pgsql-hackers@lists.postgresql.org
Content-Type: multipart/mixed; boundary="000000000000ad2c07064dbb4da6"
Archived-At: 
 <https://www.postgresql.org/message-id/CAAZLFmSOD8R7tZjRLZsmpKtJLoqjgawAaM-Pne1j8B_Q2aQK8w%40mail.gmail.com>
Precedence: bulk

--000000000000ad2c07064dbb4da6
Content-Type: multipart/alternative; boundary="000000000000ad2c05064dbb4da4"

--000000000000ad2c05064dbb4da4
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

 Hi,

  I found a copy-paste bug in
src/test/modules/test_ginpostinglist/test_ginpostinglist.c.

  The function test_itemptr_pair() encodes a pair of item pointers and
  decodes them back to verify the round-trip. The two original item
  pointers are:

  orig_itemptrs[0] =3D (0, 1)
  orig_itemptrs[1] =3D (blk, off)

  After the first itemptr check (lines 68-72), there is a second check
  intended to verify the second decoded item pointer when ndecoded =3D=3D 2=
:

  if (ndecoded =3D=3D 2 &&
      !ItemPointerEquals(&orig_itemptrs[0], &decoded_itemptrs[0]))
  {
      elog(ERROR, "mismatch on second itemptr: (%u, %d) vs (%u, %d)",
           0, 1,
           ItemPointerGetBlockNumber(&decoded_itemptrs[0]),
           ItemPointerGetOffsetNumber(&decoded_itemptrs[0]));
  }

  This is an exact duplicate of the first check =E2=80=94 it uses index [0]
  throughout, and the expected value is hardcoded as (0, 1) instead of
  (blk, off). As a result, any decoding error in decoded_itemptrs[1]
  would go undetected.

  This is particularly unfortunate because the whole purpose of testing a
  pair (as the comment at line 30-33 explains) is to exercise the varbyte
  delta encoding, which only applies to the second and subsequent item
  pointers. The broken check defeats exactly that goal.

  The attached patch fixes the second check to compare orig_itemptrs[1]
  with decoded_itemptrs[1] and reports (blk, off) as the expected
  value in the error message.

  Regards,
  Jianghua Yang

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

<div dir=3D"ltr">=C2=A0Hi,<br><br>=C2=A0 I found a copy-paste bug in src/te=
st/modules/test_ginpostinglist/test_ginpostinglist.c.<br><br>=C2=A0 The fun=
ction test_itemptr_pair() encodes a pair of item pointers and<br>=C2=A0 dec=
odes them back to verify the round-trip. The two original item<br>=C2=A0 po=
inters are:<br><br>=C2=A0 orig_itemptrs[0] =3D (0, 1)<br>=C2=A0 orig_itempt=
rs[1] =3D (blk, off)<br><br>=C2=A0 After the first itemptr check (lines 68-=
72), there is a second check<br>=C2=A0 intended to verify the second decode=
d item pointer when ndecoded =3D=3D 2:<br><br>=C2=A0 if (ndecoded =3D=3D 2 =
&amp;&amp;<br>=C2=A0 =C2=A0 =C2=A0 !ItemPointerEquals(&amp;orig_itemptrs[0]=
, &amp;decoded_itemptrs[0]))<br>=C2=A0 {<br>=C2=A0 =C2=A0 =C2=A0 elog(ERROR=
, &quot;mismatch on second itemptr: (%u, %d) vs (%u, %d)&quot;,<br>=C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A00, 1,<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0ItemPointerGetBlockNumber(&amp;decoded_itemptrs[0]),<br>=C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ItemPointerGetOffsetNumber(&amp;decoded_i=
temptrs[0]));<br>=C2=A0 }<br><br>=C2=A0 This is an exact duplicate of the f=
irst check =E2=80=94 it uses index [0]<br>=C2=A0 throughout, and the expect=
ed value is hardcoded as (0, 1) instead of<br>=C2=A0 (blk, off). As a resul=
t, any decoding error in decoded_itemptrs[1]<br>=C2=A0 would go undetected.=
<br><br>=C2=A0 This is particularly unfortunate because the whole purpose o=
f testing a<br>=C2=A0 pair (as the comment at line 30-33 explains) is to ex=
ercise the varbyte<br>=C2=A0 delta encoding, which only applies to the seco=
nd and subsequent item<br>=C2=A0 pointers. The broken check defeats exactly=
 that goal.<br><br>=C2=A0 The attached patch fixes the second check to comp=
are orig_itemptrs[1]<br>=C2=A0 with decoded_itemptrs[1] and reports (blk, o=
ff) as the expected<br>=C2=A0 value in the error message.<br><br>=C2=A0 Reg=
ards,<br>=C2=A0 Jianghua Yang</div>

--000000000000ad2c05064dbb4da4--

--000000000000ad2c07064dbb4da6
Content-Type: application/octet-stream;
	name="v1-0001-Fix-copy-paste-error-in-test_ginpostinglist-secon.patch"
Content-Disposition: attachment;
	filename="v1-0001-Fix-copy-paste-error-in-test_ginpostinglist-secon.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mn3yb71h0>
X-Attachment-Id: f_mn3yb71h0

RnJvbSBkZGEzYzY2MTFlZjVmZDRhNjE5YjgyODRlNTZhOTkwYjllMGZlZmU3IE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBKaWFuZ2h1YSBZYW5nIDx5amhqc3R6QGdtYWlsLmNvbT4KRGF0
ZTogTW9uLCAyMyBNYXIgMjAyNiAxODoyOTo1NCAtMDcwMApTdWJqZWN0OiBbUEFUQ0ggdjFdIEZp
eCBjb3B5LXBhc3RlIGVycm9yIGluIHRlc3RfZ2lucG9zdGluZ2xpc3Qgc2Vjb25kIGl0ZW1wdHIK
IGNoZWNrCgpUaGUgY2hlY2sgZm9yIGEgbWlzbWF0Y2ggb24gdGhlIHNlY29uZCBkZWNvZGVkIGl0
ZW0gcG9pbnRlciB3YXMgYW4KZXhhY3QgY29weSBvZiB0aGUgZmlyc3QgaXRlbSBwb2ludGVyIGNo
ZWNrLCBjb21wYXJpbmcgb3JpZ19pdGVtcHRyc1swXQp3aXRoIGRlY29kZWRfaXRlbXB0cnNbMF0g
aW5zdGVhZCBvZiBvcmlnX2l0ZW1wdHJzWzFdIHdpdGgKZGVjb2RlZF9pdGVtcHRyc1sxXS4gIFRo
ZSBlcnJvciBtZXNzYWdlIGFsc28gcmVwb3J0ZWQgKDAsIDEpIGFzIHRoZQpleHBlY3RlZCB2YWx1
ZSBpbnN0ZWFkIG9mIChibGssIG9mZikuICBBcyBhIHJlc3VsdCwgYW55IGRlY29kaW5nIGVycm9y
CmluIHRoZSBzZWNvbmQgaXRlbSBwb2ludGVyICh3aGVyZSB0aGUgdmFyYnl0ZSBkZWx0YSBlbmNv
ZGluZyBpcwpleGVyY2lzZWQpIHdvdWxkIGdvIHVuZGV0ZWN0ZWQuCgpBdXRob3I6IEppYW5naHVh
IFlhbmcgPHlqaGpzdHpAZ21haWwuY29tPgotLS0KIC4uLi9tb2R1bGVzL3Rlc3RfZ2lucG9zdGlu
Z2xpc3QvdGVzdF9naW5wb3N0aW5nbGlzdC5jICAgICB8IDggKysrKy0tLS0KIDEgZmlsZSBjaGFu
Z2VkLCA0IGluc2VydGlvbnMoKyksIDQgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvc3JjL3Rl
c3QvbW9kdWxlcy90ZXN0X2dpbnBvc3RpbmdsaXN0L3Rlc3RfZ2lucG9zdGluZ2xpc3QuYyBiL3Ny
Yy90ZXN0L21vZHVsZXMvdGVzdF9naW5wb3N0aW5nbGlzdC90ZXN0X2dpbnBvc3RpbmdsaXN0LmMK
aW5kZXggODNjZjM0Njk4ODYuLjZkODQzNjRmNWM1IDEwMDY0NAotLS0gYS9zcmMvdGVzdC9tb2R1
bGVzL3Rlc3RfZ2lucG9zdGluZ2xpc3QvdGVzdF9naW5wb3N0aW5nbGlzdC5jCisrKyBiL3NyYy90
ZXN0L21vZHVsZXMvdGVzdF9naW5wb3N0aW5nbGlzdC90ZXN0X2dpbnBvc3RpbmdsaXN0LmMKQEAg
LTcyLDEyICs3MiwxMiBAQCB0ZXN0X2l0ZW1wdHJfcGFpcihCbG9ja051bWJlciBibGssIE9mZnNl
dE51bWJlciBvZmYsIGludCBtYXhzaXplKQogCQkJIEl0ZW1Qb2ludGVyR2V0T2Zmc2V0TnVtYmVy
KCZkZWNvZGVkX2l0ZW1wdHJzWzBdKSk7CiAKIAlpZiAobmRlY29kZWQgPT0gMiAmJgotCQkhSXRl
bVBvaW50ZXJFcXVhbHMoJm9yaWdfaXRlbXB0cnNbMF0sICZkZWNvZGVkX2l0ZW1wdHJzWzBdKSkK
KwkJIUl0ZW1Qb2ludGVyRXF1YWxzKCZvcmlnX2l0ZW1wdHJzWzFdLCAmZGVjb2RlZF9pdGVtcHRy
c1sxXSkpCiAJewogCQllbG9nKEVSUk9SLCAibWlzbWF0Y2ggb24gc2Vjb25kIGl0ZW1wdHI6ICgl
dSwgJWQpIHZzICgldSwgJWQpIiwKLQkJCSAwLCAxLAotCQkJIEl0ZW1Qb2ludGVyR2V0QmxvY2tO
dW1iZXIoJmRlY29kZWRfaXRlbXB0cnNbMF0pLAotCQkJIEl0ZW1Qb2ludGVyR2V0T2Zmc2V0TnVt
YmVyKCZkZWNvZGVkX2l0ZW1wdHJzWzBdKSk7CisJCQkgYmxrLCBvZmYsCisJCQkgSXRlbVBvaW50
ZXJHZXRCbG9ja051bWJlcigmZGVjb2RlZF9pdGVtcHRyc1sxXSksCisJCQkgSXRlbVBvaW50ZXJH
ZXRPZmZzZXROdW1iZXIoJmRlY29kZWRfaXRlbXB0cnNbMV0pKTsKIAl9CiB9CiAKLS0gCjIuNTAu
MSAoQXBwbGUgR2l0LTE1NSkKCg==
--000000000000ad2c07064dbb4da6--