On Sat, Aug 22, 2026, at 9:38 PM, David Rowley wrote: > On Thu, 9 Jul 2026 at 11:55, David Rowley <[email protected]> wrote: >> I pushed a change to make seed int64 instead. It's still possible to >> specify the full range of possible values that GetCurrentTimestamp() >> can (or could ever) return with the int64 type. This way, there's no >> longer a mismatch in the format specifier for elog and the type we're >> formatting. That's an improvement, so thanks.
Hey David, > I was just in this area again and noticed that the nmembers > calculation does not take into account the min_value parameter. The > bad code is: > > nmembers = pg_prng_uint32(&state) % max_range + min_value; Indeed, good catch. > That should really be subtracting the min_value, as I had intended > nmembers to do was populate a random set of members in the range > specified by the function. While there, I noticed that the parameter > names are not very consistently named and having the max_value before > the min_value seems weird too. I agree that is an implicit convention and is more intuitive. > I understand test_random_operations() > has it that way. I didn't adjust that function. Maybe that could be > done separately since it's also in v19, whereas > test_random_offset_operations() is new to v20. I think this is a good idea, simply to be consistent/tidy and re-enforce the pattern for future work. > I've attached a patch which fixes this plus a few more things that > Claude code pointed out to me when I asked it to review my changes. > Those are primarily around overflow risk if max_value was given as > INT32_MAX. I applied, reviewed, and tested the v1 patch. I think it is good and necessary for the reasons you've outlined. I'd happily help or implement the additional test_random_operations() for v20/v19 when I get a second (currently traveling) if you're not going to do so. Thanks for continuing to improve this testing module. best. -greg > David > > Attachments: > * v1-0001-Fix-incorrect-nmembers-calculation-in-test_bitmap.patch
