MIME-Version: 1.0
References: 
 <CAEudQAq_zOSA2NUQSWePTGV_=90Uw0WcXxGOWnN-vwF046OOqA@mail.gmail.com>
 <2D90FFB0-C80A-4189-A5BF-C37F05E271D7@greg.burd.me>
 <CAApHDvqXdBWMwGuzJBSbq4eiNLDaR738-upd8NJpmhUq3a+XLQ@mail.gmail.com>
 <aOcaIr8QKMvbIlnA@paquier.xyz>
 <CAApHDvpn3bshX+_2t0NbJ263r1etcBdmkROjuJw8DvyZkMcXqg@mail.gmail.com>
 <aOcwU_0-U3zbNDki@paquier.xyz>
 <CAApHDvp=pEkO6LKfXKDjRNeQ1LkV7A5fsTn29ahUBXxmjyWPHg@mail.gmail.com>
 <aOg3X3BXjiXljHcR@paquier.xyz>
In-Reply-To: <aOg3X3BXjiXljHcR@paquier.xyz>
From: David Rowley <dgrowleyml@gmail.com>
Date: Sat, 18 Apr 2026 21:06:02 +1200
Message-ID: 
 <CAApHDvrDW9W72vAr7h7XeCu7+Qz-_Vff02Q+RPPuVeM0Qf0MCw@mail.gmail.com>
Subject: Re: [PATCH] Add tests for Bitmapset
To: Michael Paquier <michael@paquier.xyz>
Cc: Greg Burd <greg@burd.me>, Ranier Vilela <ranier.vf@gmail.com>,
	Daniel Gustafsson <daniel@yesql.se>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/mixed; boundary="000000000000b319a2064fb860f1"
Archived-At: 
 <https://www.postgresql.org/message-id/CAApHDvrDW9W72vAr7h7XeCu7%2BQz-_Vff02Q%2BRPPuVeM0Qf0MCw%40mail.gmail.com>
Precedence: bulk

--000000000000b319a2064fb860f1
Content-Type: text/plain; charset="UTF-8"

On Fri, 10 Oct 2025 at 11:30, Michael Paquier <michael@paquier.xyz> wrote:
> Thanks for double-checking.  Applied after running an indent.

I was working on test_bitmapset.c to add some tests for a new
bitmapset function. I noticed a few weird things.

1. test_random_operations() is coded to use GetCurrentTimestamp() as a
seed when the given seed is <= 0. Of course, it'll be a while before
the return value of that wraps beyond 2^63 (292250 years), but I still
can't help but think that NULL is a better value to use to have the
seed auto-generate.
2. Doing #1 means the function can't be STRICT. I do think it's wrong
that the function is marked as strict. That's normally reserved for
functions that we needn't call because NULL input(s) yield a NULL
output. That's not the case for this function.
3. There's no CHECK_FOR_INTERRUPTS() in test_random_operations(). If
someone uses a large num_ops, there's no way to cancel the query.
4. If there happened to be some rare bug in bitmapset.c that
test_random_operations() we might struggle to find it again, as we
don't report which seed we used in the ERROR message.
5. Couple of minor cosmetics; header order, whilespace.

I felt it was worth fixing these now as the function I plan to add
there does #1, #2, #3 and #4. If I add the new function for v20, the
discrepancy seems questionable.

David

--000000000000b319a2064fb860f1
Content-Type: application/octet-stream; name="test_bitmapset_fixes.patch"
Content-Disposition: attachment; filename="test_bitmapset_fixes.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mo43z52a0>
X-Attachment-Id: f_mo43z52a0

ZGlmZiAtLWdpdCBhL3NyYy90ZXN0L21vZHVsZXMvdGVzdF9iaXRtYXBzZXQvZXhwZWN0ZWQvdGVz
dF9iaXRtYXBzZXQub3V0IGIvc3JjL3Rlc3QvbW9kdWxlcy90ZXN0X2JpdG1hcHNldC9leHBlY3Rl
ZC90ZXN0X2JpdG1hcHNldC5vdXQKaW5kZXggZjc1Y2I0NmI4NjkuLjBiNzJiOTFjZDFmIDEwMDY0
NAotLS0gYS9zcmMvdGVzdC9tb2R1bGVzL3Rlc3RfYml0bWFwc2V0L2V4cGVjdGVkL3Rlc3RfYml0
bWFwc2V0Lm91dAorKysgYi9zcmMvdGVzdC9tb2R1bGVzL3Rlc3RfYml0bWFwc2V0L2V4cGVjdGVk
L3Rlc3RfYml0bWFwc2V0Lm91dApAQCAtMTU2OSw3ICsxNTY5LDcgQEAgU0VMRUNUIHRlc3RfYm1z
X25vbmVtcHR5X2RpZmZlcmVuY2UoJyhiIDEgMiknLCAnKGIgNTAgMTAwKScpIEFTIHJlc3VsdDsK
ICgxIHJvdykKIAogLS0gcmFuZG9tIG9wZXJhdGlvbnMKLVNFTEVDVCB0ZXN0X3JhbmRvbV9vcGVy
YXRpb25zKC0xLCAxMDAwMCwgODE5MjAsIDApID4gMCBBUyByZXN1bHQ7CitTRUxFQ1QgdGVzdF9y
YW5kb21fb3BlcmF0aW9ucyhOVUxMLCAxMDAwMCwgODE5MjAsIDApID4gMCBBUyByZXN1bHQ7CiAg
cmVzdWx0IAogLS0tLS0tLS0KICB0CmRpZmYgLS1naXQgYS9zcmMvdGVzdC9tb2R1bGVzL3Rlc3Rf
Yml0bWFwc2V0L3NxbC90ZXN0X2JpdG1hcHNldC5zcWwgYi9zcmMvdGVzdC9tb2R1bGVzL3Rlc3Rf
Yml0bWFwc2V0L3NxbC90ZXN0X2JpdG1hcHNldC5zcWwKaW5kZXggZTc1YWI4ZjYyMGEuLmM1MzIz
MmUwYWRhIDEwMDY0NAotLS0gYS9zcmMvdGVzdC9tb2R1bGVzL3Rlc3RfYml0bWFwc2V0L3NxbC90
ZXN0X2JpdG1hcHNldC5zcWwKKysrIGIvc3JjL3Rlc3QvbW9kdWxlcy90ZXN0X2JpdG1hcHNldC9z
cWwvdGVzdF9iaXRtYXBzZXQuc3FsCkBAIC00MDEsNiArNDAxLDYgQEAgU0VMRUNUIHRlc3RfYm1z
X25vbmVtcHR5X2RpZmZlcmVuY2UoJyhiIDEwMCknLCAnKGIgNSknKSBBUyByZXN1bHQ7CiBTRUxF
Q1QgdGVzdF9ibXNfbm9uZW1wdHlfZGlmZmVyZW5jZSgnKGIgMSAyKScsICcoYiA1MCAxMDApJykg
QVMgcmVzdWx0OwogCiAtLSByYW5kb20gb3BlcmF0aW9ucwotU0VMRUNUIHRlc3RfcmFuZG9tX29w
ZXJhdGlvbnMoLTEsIDEwMDAwLCA4MTkyMCwgMCkgPiAwIEFTIHJlc3VsdDsKK1NFTEVDVCB0ZXN0
X3JhbmRvbV9vcGVyYXRpb25zKE5VTEwsIDEwMDAwLCA4MTkyMCwgMCkgPiAwIEFTIHJlc3VsdDsK
IAogRFJPUCBFWFRFTlNJT04gdGVzdF9iaXRtYXBzZXQ7CmRpZmYgLS1naXQgYS9zcmMvdGVzdC9t
b2R1bGVzL3Rlc3RfYml0bWFwc2V0L3Rlc3RfYml0bWFwc2V0LS0xLjAuc3FsIGIvc3JjL3Rlc3Qv
bW9kdWxlcy90ZXN0X2JpdG1hcHNldC90ZXN0X2JpdG1hcHNldC0tMS4wLnNxbAppbmRleCAyMjdl
Y2I1YWEzYi4uZTdiMjYzZTUxZjUgMTAwNjQ0Ci0tLSBhL3NyYy90ZXN0L21vZHVsZXMvdGVzdF9i
aXRtYXBzZXQvdGVzdF9iaXRtYXBzZXQtLTEuMC5zcWwKKysrIGIvc3JjL3Rlc3QvbW9kdWxlcy90
ZXN0X2JpdG1hcHNldC90ZXN0X2JpdG1hcHNldC0tMS4wLnNxbApAQCAtMTMzLDggKzEzMyw4IEBA
IFJFVFVSTlMgaW50CiBBUyAnTU9EVUxFX1BBVEhOQU1FJyBMQU5HVUFHRSBDOwogCiAtLSBUZXN0
IHV0aWxpdHkgZnVuY3Rpb25zCi1DUkVBVEUgRlVOQ1RJT04gdGVzdF9yYW5kb21fb3BlcmF0aW9u
cyhpbnRlZ2VyLCBpbnRlZ2VyLCBpbnRlZ2VyLCBpbnRlZ2VyKQotUkVUVVJOUyBpbnRlZ2VyIFNU
UklDVAorQ1JFQVRFIEZVTkNUSU9OIHRlc3RfcmFuZG9tX29wZXJhdGlvbnMoYmlnaW50LCBpbnRl
Z2VyLCBpbnRlZ2VyLCBpbnRlZ2VyKQorUkVUVVJOUyBpbnRlZ2VyCiBBUyAnTU9EVUxFX1BBVEhO
QU1FJyBMQU5HVUFHRSBDOwogCiBDT01NRU5UIE9OIEVYVEVOU0lPTiB0ZXN0X2JpdG1hcHNldCBJ
UyAnVGVzdCBjb2RlIGZvciBCaXRtYXBzZXQnOwpkaWZmIC0tZ2l0IGEvc3JjL3Rlc3QvbW9kdWxl
cy90ZXN0X2JpdG1hcHNldC90ZXN0X2JpdG1hcHNldC5jIGIvc3JjL3Rlc3QvbW9kdWxlcy90ZXN0
X2JpdG1hcHNldC90ZXN0X2JpdG1hcHNldC5jCmluZGV4IDI3MmJlZDM5MGE2Li4zYTE4NTM2OTY1
MSAxMDA2NDQKLS0tIGEvc3JjL3Rlc3QvbW9kdWxlcy90ZXN0X2JpdG1hcHNldC90ZXN0X2JpdG1h
cHNldC5jCisrKyBiL3NyYy90ZXN0L21vZHVsZXMvdGVzdF9iaXRtYXBzZXQvdGVzdF9iaXRtYXBz
ZXQuYwpAQCAtMSw3ICsxLDcgQEAKIC8qLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQogICoKICAqIHRlc3RfYml0
bWFwc2V0LmMKLSAqICAgICAgVGVzdCB0aGUgQml0bWFwc2V0IGRhdGEgc3RydWN0dXJlLgorICoJ
ICBUZXN0IHRoZSBCaXRtYXBzZXQgZGF0YSBzdHJ1Y3R1cmUuCiAgKgogICogVGhpcyBtb2R1bGUg
dGVzdHMgdGhlIEJpdG1hcHNldCBpbXBsZW1lbnRhdGlvbiBpbiBQb3N0Z3JlU1FMLCBjb3Zlcmlu
ZwogICogYWxsIHB1YmxpYyBBUEkgZnVuY3Rpb25zLgpAQCAtMTksMTEgKzE5LDEyIEBACiAjaW5j
bHVkZSA8c3RkZGVmLmg+CiAjaW5jbHVkZSAiY2F0YWxvZy9wZ190eXBlLmgiCiAjaW5jbHVkZSAi
Y29tbW9uL3BnX3BybmcuaCIKLSNpbmNsdWRlICJ1dGlscy9hcnJheS5oIgogI2luY2x1ZGUgImZt
Z3IuaCIKKyNpbmNsdWRlICJtaXNjYWRtaW4uaCIKICNpbmNsdWRlICJub2Rlcy9iaXRtYXBzZXQu
aCIKICNpbmNsdWRlICJub2Rlcy9ub2Rlcy5oIgogI2luY2x1ZGUgIm5vZGVzL3BnX2xpc3QuaCIK
KyNpbmNsdWRlICJ1dGlscy9hcnJheS5oIgogI2luY2x1ZGUgInV0aWxzL2J1aWx0aW5zLmgiCiAj
aW5jbHVkZSAidXRpbHMvdGltZXN0YW1wLmgiCiAKQEAgLTU4Nyw3ICs1ODgsNyBAQCB0ZXN0X2Jp
dG1hcF9tYXRjaChQR19GVU5DVElPTl9BUkdTKQogICogIm1pbl92YWx1ZSIgaXMgdGhlIG1pbmlt
YWwgdmFsdWUgdXNlZCBmb3IgdGhlIG1lbWJlcnMsIHRoYXQgd2lsbCBzdGFuZAogICogdXAgdG8g
YSByYW5nZSBvZiAibWF4X3JhbmdlIi4gICJudW1fb3BzIiBkZWZpbmVzIHRoZSBudW1iZXIgb2Yg
dGltZSBlYWNoCiAgKiBvcGVyYXRpb24gaXMgZG9uZS4gICJzZWVkIiBpcyBhIHJhbmRvbSBzZWVk
IHVzZWQgdG8gY2FsY3VsYXRlIHRoZSBtZW1iZXIKLSAqIHZhbHVlcy4gIFdoZW4gInNlZWQiIGlz
IDw9IDAsIGEgcmFuZG9tIHNlZWQgd2lsbCBiZSBjaG9zZW4gYXV0b21hdGljYWxseS4KKyAqIHZh
bHVlcy4gIFdoZW4gInNlZWQiIGlzIE5VTEwsIGEgcmFuZG9tIHNlZWQgd2lsbCBiZSBjaG9zZW4g
YXV0b21hdGljYWxseS4KICAqCiAgKiBUaGUgcmV0dXJuIHZhbHVlIGlzIHRoZSBudW1iZXIgb2Yg
dGltZXMgYWxsIG9wZXJhdGlvbnMgaGF2ZSBiZWVuIGV4ZWN1dGVkLgogICovCkBAIC02MDgsMTMg
KzYwOSwyMCBAQCB0ZXN0X3JhbmRvbV9vcGVyYXRpb25zKFBHX0ZVTkNUSU9OX0FSR1MpCiAJaW50
CQkJbnVtX21lbWJlcnMgPSAwOwogCWludAkJCXRvdGFsX29wcyA9IDA7CiAKLQlpZiAoUEdfR0VU
QVJHX0lOVDMyKDApID4gMCkKLQkJc2VlZCA9IFBHX0dFVEFSR19JTlQzMigwKTsKKwlpZiAoIVBH
X0FSR0lTTlVMTCgwKSkKKwkJc2VlZCA9IFBHX0dFVEFSR19JTlQ2NCgwKTsKIAogCW51bV9vcHMg
PSBQR19HRVRBUkdfSU5UMzIoMSk7CiAJbWF4X3JhbmdlID0gUEdfR0VUQVJHX0lOVDMyKDIpOwog
CW1pbl92YWx1ZSA9IFBHX0dFVEFSR19JTlQzMigzKTsKIAorCWlmIChQR19BUkdJU05VTEwoMSkg
fHwgbnVtX29wcyA8PSAwKQorCQllbG9nKEVSUk9SLCAiaW52YWxpZCBudW1iZXIgb2Ygb3BlcmF0
aW9ucyIpOworCWlmIChQR19BUkdJU05VTEwoMikgfHwgbWF4X3JhbmdlIDw9IDApCisJCWVsb2co
RVJST1IsICJpbnZhbGlkIG1heGltdW0gcmFuZ2UiKTsKKwlpZiAoUEdfQVJHSVNOVUxMKDMpIHx8
IG1pbl92YWx1ZSA8IDApCisJCWVsb2coRVJST1IsICJpbnZhbGlkIG1pbmltdW0gdmFsdWUiKTsK
KwogCXBnX3Bybmdfc2VlZCgmc3RhdGUsIHNlZWQpOwogCiAJLyoKQEAgLTYyNyw2ICs2MzUsOCBA
QCB0ZXN0X3JhbmRvbV9vcGVyYXRpb25zKFBHX0ZVTkNUSU9OX0FSR1MpCiAJLyogUGhhc2UgMTog
UmFuZG9tIGluc2VydGlvbnMgaW4gZmlyc3Qgc2V0ICovCiAJZm9yIChpbnQgaSA9IDA7IGkgPCBu
dW1fb3BzIC8gMjsgaSsrKQogCXsKKwkJQ0hFQ0tfRk9SX0lOVEVSUlVQVFMoKTsKKwogCQltZW1i
ZXIgPSBwZ19wcm5nX3VpbnQzMigmc3RhdGUpICUgbWF4X3JhbmdlICsgbWluX3ZhbHVlOwogCiAJ
CWlmICghYm1zX2lzX21lbWJlcihtZW1iZXIsIGJtczEpKQpAQCAtNjM3LDYgKzY0Nyw4IEBAIHRl
c3RfcmFuZG9tX29wZXJhdGlvbnMoUEdfRlVOQ1RJT05fQVJHUykKIAkvKiBQaGFzZSAyOiBSYW5k
b20gaW5zZXJ0aW9ucyBpbiBzZWNvbmQgc2V0ICovCiAJZm9yIChpbnQgaSA9IDA7IGkgPCBudW1f
b3BzIC8gNDsgaSsrKQogCXsKKwkJQ0hFQ0tfRk9SX0lOVEVSUlVQVFMoKTsKKwogCQltZW1iZXIg
PSBwZ19wcm5nX3VpbnQzMigmc3RhdGUpICUgbWF4X3JhbmdlICsgbWluX3ZhbHVlOwogCiAJCWlm
ICghYm1zX2lzX21lbWJlcihtZW1iZXIsIGJtczIpKQpAQCAtNjUxLDggKzY2MywxMSBAQCB0ZXN0
X3JhbmRvbV9vcGVyYXRpb25zKFBHX0ZVTkNUSU9OX0FSR1MpCiAJLyogVmVyaWZ5IHVuaW9uIGNv
bnRhaW5zIGFsbCBtZW1iZXJzIGZyb20gZmlyc3QgYW5kIHNlY29uZCBzZXRzICovCiAJZm9yIChp
bnQgaSA9IDA7IGkgPCBudW1fbWVtYmVyczsgaSsrKQogCXsKKwkJQ0hFQ0tfRk9SX0lOVEVSUlVQ
VFMoKTsKKwogCQlpZiAoIWJtc19pc19tZW1iZXIobWVtYmVyc1tpXSwgcmVzdWx0KSkKLQkJCWVs
b2coRVJST1IsICJ1bmlvbiBtaXNzaW5nIG1lbWJlciAlZCIsIG1lbWJlcnNbaV0pOworCQkJZWxv
ZyhFUlJPUiwgInVuaW9uIG1pc3NpbmcgbWVtYmVyICVkLCBzZWVkICIgSU5UNjRfRk9STUFULAor
CQkJCSBtZW1iZXJzW2ldLCBzZWVkKTsKIAl9CiAJYm1zX2ZyZWUocmVzdWx0KTsKIApAQCAtNjY3
LDggKzY4MiwxMSBAQCB0ZXN0X3JhbmRvbV9vcGVyYXRpb25zKFBHX0ZVTkNUSU9OX0FSR1MpCiAK
IAkJd2hpbGUgKChtZW1iZXIgPSBibXNfbmV4dF9tZW1iZXIocmVzdWx0LCBtZW1iZXIpKSA+PSAw
KQogCQl7CisJCQlDSEVDS19GT1JfSU5URVJSVVBUUygpOworCiAJCQlpZiAoIWJtc19pc19tZW1i
ZXIobWVtYmVyLCBibXMxKSB8fCAhYm1zX2lzX21lbWJlcihtZW1iZXIsIGJtczIpKQotCQkJCWVs
b2coRVJST1IsICJpbnRlcnNlY3Rpb24gY29udGFpbnMgaW52YWxpZCBtZW1iZXIgJWQiLCBtZW1i
ZXIpOworCQkJCWVsb2coRVJST1IsICJpbnRlcnNlY3Rpb24gY29udGFpbnMgaW52YWxpZCBtZW1i
ZXIgJWQsIHNlZWQgIiBJTlQ2NF9GT1JNQVQsCisJCQkJCSBtZW1iZXIsIHNlZWQpOwogCQl9CiAJ
CWJtc19mcmVlKHJlc3VsdCk7CiAJfQpAQCAtNjgwLDYgKzY5OCw4IEBAIHRlc3RfcmFuZG9tX29w
ZXJhdGlvbnMoUEdfRlVOQ1RJT05fQVJHUykKIAkJaW50CQkJbG93ZXIgPSBwZ19wcm5nX3VpbnQz
Migmc3RhdGUpICUgMTAwOwogCQlpbnQJCQl1cHBlciA9IGxvd2VyICsgKHBnX3BybmdfdWludDMy
KCZzdGF0ZSkgJSAyMCk7CiAKKwkJQ0hFQ0tfRk9SX0lOVEVSUlVQVFMoKTsKKwogCQlyZXN1bHQg
PSBibXNfYWRkX3JhbmdlKHJlc3VsdCwgbG93ZXIsIHVwcGVyKTsKIAl9CiAJaWYgKHJlc3VsdCAh
PSBOVUxMKQpAQCAtNjk5LDYgKzcxOSw4IEBAIHRlc3RfcmFuZG9tX29wZXJhdGlvbnMoUEdfRlVO
Q1RJT05fQVJHUykKIAogCWZvciAoaW50IG9wID0gMDsgb3AgPCBudW1fb3BzOyBvcCsrKQogCXsK
KwkJQ0hFQ0tfRk9SX0lOVEVSUlVQVFMoKTsKKwogCQlzd2l0Y2ggKHBnX3BybmdfdWludDMyKCZz
dGF0ZSkgJSAzKQogCQl7CiAJCQljYXNlIDA6CQkJCS8qIGFkZCAqLwpAQCAtNzE0LDcgKzczNiw4
IEBAIHRlc3RfcmFuZG9tX29wZXJhdGlvbnMoUEdfRlVOQ1RJT05fQVJHUykKIAogCQkJCQltZW1i
ZXIgPSBtZW1iZXJzW3Bvc107CiAJCQkJCWlmICghYm1zX2lzX21lbWJlcihtZW1iZXIsIGJtcykp
Ci0JCQkJCQllbG9nKEVSUk9SLCAiZXhwZWN0ZWQgJWQgdG8gYmUgYSB2YWxpZCBtZW1iZXIiLCBt
ZW1iZXIpOworCQkJCQkJZWxvZyhFUlJPUiwgImV4cGVjdGVkICVkIHRvIGJlIGEgdmFsaWQgbWVt
YmVyLCBzZWVkICIgSU5UNjRfRk9STUFULAorCQkJCQkJCSBtZW1iZXIsIHNlZWQpOwogCiAJCQkJ
CWJtcyA9IGJtc19kZWxfbWVtYmVyKGJtcywgbWVtYmVyKTsKIApAQCAtNzMwLDcgKzc1Myw4IEBA
IHRlc3RfcmFuZG9tX29wZXJhdGlvbnMoUEdfRlVOQ1RJT05fQVJHUykKIAkJCQlmb3IgKGludCBp
ID0gMDsgaSA8IG51bV9tZW1iZXJzOyBpKyspCiAJCQkJewogCQkJCQlpZiAoIWJtc19pc19tZW1i
ZXIobWVtYmVyc1tpXSwgYm1zKSkKLQkJCQkJCWVsb2coRVJST1IsICJtaXNzaW5nIG1lbWJlciAl
ZCIsIG1lbWJlcnNbaV0pOworCQkJCQkJZWxvZyhFUlJPUiwgIm1pc3NpbmcgbWVtYmVyICVkLCBz
ZWVkICIgSU5UNjRfRk9STUFULAorCQkJCQkJCSBtZW1iZXJzW2ldLCBzZWVkKTsKIAkJCQl9CiAJ
CQkJYnJlYWs7CiAJCX0K
--000000000000b319a2064fb860f1--