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