On 2/24/23 22:07, Heikki Linnakangas wrote: > I had a quick look at just the preliminary cleanup patches: > >> 0001-BRIN-bloom-cleanup-20230218.patch > > Looks good to me > >> 0002-BRIN-minmax-multi-cleanup-20230218.patch > > Looks good, although it would feel more natural to me to do it the other > way round, and define 'matches' as 'bool matches', and use DatumGetBool. >
Yeah, probably. I was trying to only do the minimal change because of (maybe) backpatching this. > Not new with this patch, but I find the 'matches' and 'matching' > variables a bit strange. Wouldn't it be simpler to have just one variable? > True. I don't recall why we did it this way. >> 0003-Introduce-bloom_filter_size-20230218.patch > > Looks good > >> 0004-Add-minmax-multi-inequality-tests-20230218.patch > > Looks good > >> +SELECT i/5 + mod(911 * i + 483, 25), >> + i/10 + mod(751 * i + 221, 41) > > Peculiar formulas. Was there a particular reason for these values? > No, not really. I simply wanted a random-looking data, but reproducible and deterministic. And linear congruential generator is a simple way to do that. I just picked a couple co-prime numbers, and that's it. regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
