This change makes sense to me. But I have a question here, because we already have a limitation in rel option level and that will make sure the parameter is in 0.0001 - 0.25 range, this CHECK seems like a little redundant for me especially it does not match the constraint in rel option. Maybe I missed some context here, but could we just remove this CHECK?
Thanks,
Shihao
On Mon, Oct 23, 2023 at 4:53 PM Alexander Lakhin <exclusion@gmail.com> wrote:
16.06.2023 05:23, Tom Lane wrote: > Masahiko Sawada <sawada.mshk@gmail.com> writes: >> It seems that the minimum false positive rate also doesn't work: >> postgres(1:3419179)=# create table t (a int); >> CREATE TABLE >> postgres(1:3419179)=# create index t_idx on t using brin (a >> int4_bloom_ops (false_positive_rate = 0.0001)); >> CREATE INDEX >> postgres(1:3419179)=# insert into t values (1); >> 2023-06-16 11:14:01.349 JST [3419179] ERROR: the bloom filter is too >> large (8924 > 8144) >> 2023-06-16 11:14:01.349 JST [3419179] STATEMENT: insert into t values (1); >> ERROR: the bloom filter is too large (8924 > 8144) > Hmm, but it would work with a larger page size, right? If so > I'm disinclined to move the minimum. What I don't like about > the above is that the failure doesn't occur until you actually > insert an index entry. Is there a way to check for this at > CREATE INDEX time?
With a larger page size, say, 32 kB, you'll get the similar error: ERROR: the bloom filter is too large (35856 > 32720)
but, if you define also a smaller n_distinct_per_range value explicitly. e. g.: CREATE INDEX idx ON tbl USING brin (i int4_bloom_ops(false_positive_rate = 0.00010, n_distinct_per_range = 100)); false_positive_rate = 0.00010 will do.
So I see an issue only with false_positive_rate. And I think the assertion that false_positive_rate is a valid probability would be good. The patch is attached.
Tomas, I saw that you refactored that place (introduced bloom_filter_size()) in 2b8b2852b, could you also consider checking for invalid parameter values at CREATE INDEX time, as Tom suggested above?