From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Missing update of all_hasnulls in BRIN opclasses
Date: Fri, 21 Oct 2022 17:23:45 +0200
Message-ID: <402430e4-7d9d-6cf1-09ef-464d80afff3b@enterprisedb.com> (raw)
Hi,
While working on some BRIN code, I discovered a bug in handling NULL
values - when inserting a non-NULL value into a NULL-only range, we
reset the all_nulls flag but don't update the has_nulls flag. And
because of that we then fail to return the range for IS NULL ranges.
Reproducing this is trivial:
create table t (a int);
create index on t using brin (a);
insert into t values (null);
insert into t values (1);
set enable_seqscan = off;
select * from t where a is null;
This should return 1 row, but actually it returns no rows.
Attached is a patch fixing this by properly updating the has_nulls flag.
I reproduced this all the way back to 9.5, so it's a long-standing bug.
It's interesting no one noticed / reported it so far, it doesn't seem
like a particularly rare corner case.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachments:brin-hasnulls-fix.patch (text/x-patch, 4.6K) downloaddiff --git a/src/backend/access/brin/brin_bloom.c b/src/backend/access/brin/brin_bloom.cindex 6b0af7267d5..60315450b41 100644--- a/src/backend/access/brin/brin_bloom.c+++ b/src/backend/access/brin/brin_bloom.c@@ -539,6 +539,7 @@ brin_bloom_add_value(PG_FUNCTION_ARGS)
BloomGetFalsePositiveRate(opts));
column->bv_values[0] = PointerGetDatum(filter);
column->bv_allnulls = false;
+ column->bv_hasnulls = true;
updated = true;
}
else
diff --git a/src/backend/access/brin/brin_inclusion.c b/src/backend/access/brin/brin_inclusion.cindex 4b02d374f23..e0f44d3e62c 100644--- a/src/backend/access/brin/brin_inclusion.c+++ b/src/backend/access/brin/brin_inclusion.c@@ -164,6 +164,7 @@ brin_inclusion_add_value(PG_FUNCTION_ARGS)
column->bv_values[INCLUSION_UNMERGEABLE] = BoolGetDatum(false);
column->bv_values[INCLUSION_CONTAINS_EMPTY] = BoolGetDatum(false);
column->bv_allnulls = false;
+ column->bv_hasnulls = true;
new = true;
}
diff --git a/src/backend/access/brin/brin_minmax.c b/src/backend/access/brin/brin_minmax.cindex 9e8a8e056cc..8a5661a8952 100644--- a/src/backend/access/brin/brin_minmax.c+++ b/src/backend/access/brin/brin_minmax.c@@ -90,6 +90,7 @@ brin_minmax_add_value(PG_FUNCTION_ARGS)
column->bv_values[0] = datumCopy(newval, attr->attbyval, attr->attlen);
column->bv_values[1] = datumCopy(newval, attr->attbyval, attr->attlen);
column->bv_allnulls = false;
+ column->bv_hasnulls = true;
PG_RETURN_BOOL(true);
}
diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.cindex 9a0bcf6698d..4e7119e2d78 100644--- a/src/backend/access/brin/brin_minmax_multi.c+++ b/src/backend/access/brin/brin_minmax_multi.c@@ -2500,6 +2500,7 @@ brin_minmax_multi_add_value(PG_FUNCTION_ARGS)
MemoryContextSwitchTo(oldctx);
column->bv_allnulls = false;
+ column->bv_hasnulls = true;
modified = true;
column->bv_mem_value = PointerGetDatum(ranges);
diff --git a/src/test/regress/expected/brin.out b/src/test/regress/expected/brin.outindex 73fa38396e4..cc896c2d9d4 100644--- a/src/test/regress/expected/brin.out+++ b/src/test/regress/expected/brin.out@@ -572,3 +572,39 @@ CREATE UNLOGGED TABLE brintest_unlogged (n numrange);
CREATE INDEX brinidx_unlogged ON brintest_unlogged USING brin (n);
INSERT INTO brintest_unlogged VALUES (numrange(0, 2^1000::numeric));
DROP TABLE brintest_unlogged;
+-- test that we properly update has_nulls when inserting something into+-- a range that only had NULLs before+CREATE TABLE brintest_4 (a INT, b INT, c INT, d INET);+CREATE INDEX brintest_4_idx ON brintest_4 USING brin (a, b int4_minmax_multi_ops, c int4_bloom_ops, d inet_inclusion_ops);+-- insert a NULL value, so that we get an all-nulls range+INSERT INTO brintest_4 VALUES (NULL, NULL, NULL, NULL);+-- now insert a non-NULL value+INSERT INTO brintest_4 VALUES (1, 1, 1, '127.0.0.1');+-- see that we can still match the value when using the brin index+SET enable_seqscan = off;+SELECT * FROM brintest_4 WHERE a IS NULL;+ a | b | c | d +---+---+---+---+ | | | +(1 row)++SELECT * FROM brintest_4 WHERE b IS NULL;+ a | b | c | d +---+---+---+---+ | | | +(1 row)++SELECT * FROM brintest_4 WHERE c IS NULL;+ a | b | c | d +---+---+---+---+ | | | +(1 row)++SELECT * FROM brintest_4 WHERE d IS NULL;+ a | b | c | d +---+---+---+---+ | | | +(1 row)++DROP TABLE brintest_4;+SET enable_seqscan = off;diff --git a/src/test/regress/sql/brin.sql b/src/test/regress/sql/brin.sqlindex e68e9e18df5..17a01a4b82f 100644--- a/src/test/regress/sql/brin.sql+++ b/src/test/regress/sql/brin.sql@@ -515,3 +515,24 @@ CREATE UNLOGGED TABLE brintest_unlogged (n numrange);
CREATE INDEX brinidx_unlogged ON brintest_unlogged USING brin (n);
INSERT INTO brintest_unlogged VALUES (numrange(0, 2^1000::numeric));
DROP TABLE brintest_unlogged;
++-- test that we properly update has_nulls when inserting something into+-- a range that only had NULLs before+CREATE TABLE brintest_4 (a INT, b INT, c INT, d INET);+CREATE INDEX brintest_4_idx ON brintest_4 USING brin (a, b int4_minmax_multi_ops, c int4_bloom_ops, d inet_inclusion_ops);++-- insert a NULL value, so that we get an all-nulls range+INSERT INTO brintest_4 VALUES (NULL, NULL, NULL, NULL);++-- now insert a non-NULL value+INSERT INTO brintest_4 VALUES (1, 1, 1, '127.0.0.1');++-- see that we can still match the value when using the brin index+SET enable_seqscan = off;+SELECT * FROM brintest_4 WHERE a IS NULL;+SELECT * FROM brintest_4 WHERE b IS NULL;+SELECT * FROM brintest_4 WHERE c IS NULL;+SELECT * FROM brintest_4 WHERE d IS NULL;++DROP TABLE brintest_4;+SET enable_seqscan = off;
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@enterprisedb.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Missing update of all_hasnulls in BRIN opclasses
In-Reply-To: <402430e4-7d9d-6cf1-09ef-464d80afff3b@enterprisedb.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox