Re: BUG #17969: Assert failed in bloom_init() when false_positive_rate = 0.25
Alexander Law <exclusion@gmail.com>
From: Alexander Lakhin <exclusion@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>, Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: pgsql-bugs@lists.postgresql.org, Masahiko Sawada <sawada.mshk@gmail.com>
Date: 2023-09-26T07:00:00Z
Lists: pgsql-bugs
Attachments
- v1-01-fix-assert-in-brin-bloom.patch (text/x-patch) patch v1
- v1-01-fix-assert-in-brin-bloom.backpatch14-16 (text/plain)
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? Best regards, Alexander
Commits
-
Enlarge assertion in bloom_init() for false_positive_rate
- 59fc39c0d5d9 14.11 landed
- 7e18c0bd63ae 15.6 landed
- 4dccf9575369 16.2 landed
- 1b2c6b756eaa 17.0 landed