On 10/22/22 10:00, Alvaro Herrera wrote:
> On 2022-Oct-22, Tomas Vondra wrote:
>
>> I wonder how to do this in a back-patchable way - we can't add
>> parameters to the opclass procedure, and the other solution seems to be
>> storing it right in the BrinMemTuple, somehow. But that's likely an ABI
>> break :-(
>
> Hmm, I don't see the ABI incompatibility. BrinMemTuple is an in-memory
> structure, so you can add new members at the end of the struct and it
> will pose no problems to existing code.
>
But we're not passing BrinMemTuple to the opclass procedures - we're
passing a pointer to BrinValues, so we'd have to add the flag there. And
we're storing an array of those, so adding a field may shift the array
even if you add it at the end. Not sure if that's OK or not.
>> The only solution I can think of is actually passing it using all_nulls
>> and has_nulls - we could set both flags to true (which we never do now)
>> and teach the opclass that it signifies "empty" (and thus not to update
>> has_nulls after resetting all_nulls).
>>
>> Something like the attached (I haven't added any more tests, not sure
>> what would those look like - I can't think of a query testing this,
>> although maybe we could check how the flags change using pageinspect).
>
> I'll try to have a look at these patches tomorrow or on Monday.
>
I was experimenting with this a bit more, and unfortunately the latest
patch is still a few bricks shy - it did fix this particular issue, but
there were other cases that remained/got broken. See the first patch,
that adds a bunch of pageinspect tests testing different combinations.
After thinking about it a bit more, I think we can't quite fix this at
the opclass level, so the yesterday's patches are wrong. Instead, this
should be fixed in values_add_to_range() - the whole trick is we need to
remember the range was empty at the beginning, and only set the flag
when allnulls is false.
The reworked patch does that. And we can use the same logic (both flags
set mean no tuples were added to the range) when building the index, a
separate flag is not needed.
This slightly affects existing regression tests, because we won't create
any ranges for empty table (now we created one, because we initialized a
tuple in brinbuild and then wrote it to disk). This means that
brin_summarize_range now returns 0, but I think that's fine.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From 5456cf819426d3f90c004f673dfc863903e568a1 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.von...@postgresql.org>
Date: Sat, 22 Oct 2022 12:47:33 +0200
Subject: [PATCH 1/2] pageinspect brinbugs test
---
contrib/pageinspect/Makefile | 2 +-
contrib/pageinspect/expected/brinbugs.out | 222 ++++++++++++++++++++++
contrib/pageinspect/sql/brinbugs.sql | 114 +++++++++++
3 files changed, 337 insertions(+), 1 deletion(-)
create mode 100644 contrib/pageinspect/expected/brinbugs.out
create mode 100644 contrib/pageinspect/sql/brinbugs.sql
diff --git a/contrib/pageinspect/Makefile b/contrib/pageinspect/Makefile
index 5c0736564ab..92305e981f7 100644
--- a/contrib/pageinspect/Makefile
+++ b/contrib/pageinspect/Makefile
@@ -21,7 +21,7 @@ DATA = pageinspect--1.9--1.10.sql pageinspect--1.8--1.9.sql \
pageinspect--1.0--1.1.sql
PGFILEDESC = "pageinspect - functions to inspect contents of database pages"
-REGRESS = page btree brin gin gist hash checksum oldextversions
+REGRESS = page btree brin gin gist hash checksum oldextversions brinbugs
ifdef USE_PGXS
PG_CONFIG = pg_config
diff --git a/contrib/pageinspect/expected/brinbugs.out b/contrib/pageinspect/expected/brinbugs.out
new file mode 100644
index 00000000000..23843caa138
--- /dev/null
+++ b/contrib/pageinspect/expected/brinbugs.out
@@ -0,0 +1,222 @@
+create extension pageinspect;
+create table t (a int, b int);
+create index on t using brin (a, b);
+-- both columns should have has_nulls=false and [1,1] range
+truncate t;
+insert into t values (1,1);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+----------
+ 1 | 0 | 1 | f | f | f | {1 .. 1}
+ 1 | 0 | 2 | f | f | f | {1 .. 1}
+(2 rows)
+
+-- first column should have all_nulls=true, second has_nulls=false and [1,1] range
+truncate t;
+insert into t values (null, 1);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+----------
+ 1 | 0 | 1 | t | f | f |
+ 1 | 0 | 2 | f | f | f | {1 .. 1}
+(2 rows)
+
+-- both columns should have all_nulls=true
+truncate t;
+insert into t values (null, null);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+-------
+ 1 | 0 | 1 | t | f | f |
+ 1 | 0 | 2 | t | f | f |
+(2 rows)
+
+-- both columns should have has_nulls=true and [1,1] range
+truncate t;
+insert into t values (null, null);
+insert into t values (1, 1);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+----------
+ 1 | 0 | 1 | f | t | f | {1 .. 1}
+ 1 | 0 | 2 | f | t | f | {1 .. 1}
+(2 rows)
+
+-- both columns should have has_nulls=true and [1,1] range
+truncate t;
+insert into t values (1, 1);
+insert into t values (null, null);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+----------
+ 1 | 0 | 1 | f | t | f | {1 .. 1}
+ 1 | 0 | 2 | f | t | f | {1 .. 1}
+(2 rows)
+
+-- both columns should have has_nulls=true and [1,1] range
+truncate t;
+insert into t values (null, 1);
+insert into t values (1, null);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+----------
+ 1 | 0 | 1 | f | t | f | {1 .. 1}
+ 1 | 0 | 2 | f | t | f | {1 .. 1}
+(2 rows)
+
+-- both columns should have has_nulls=false and [1,1] range
+truncate t;
+insert into t values (1, 1);
+insert into t values (1, 1);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+----------
+ 1 | 0 | 1 | f | f | f | {1 .. 1}
+ 1 | 0 | 2 | f | f | f | {1 .. 1}
+(2 rows)
+
+-- both columns should have all_nulls=true only
+truncate t;
+insert into t values (null, null);
+insert into t values (null, null);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+-------
+ 1 | 0 | 1 | t | f | f |
+ 1 | 0 | 2 | t | f | f |
+(2 rows)
+
+-- first column should have has_nulls=false and [1,1] range, second all_nulls=true
+truncate t;
+insert into t values (1, null);
+insert into t values (1, null);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+----------
+ 1 | 0 | 1 | f | f | f | {1 .. 1}
+ 1 | 0 | 2 | t | f | f |
+(2 rows)
+
+-- both columns should have has_nulls=true and [1,1] range
+truncate t;
+insert into t values (null, null);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+-------
+ 1 | 0 | 1 | t | f | f |
+ 1 | 0 | 2 | t | f | f |
+(2 rows)
+
+insert into t values (1,1);
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+----------
+ 1 | 0 | 1 | f | t | f | {1 .. 1}
+ 1 | 0 | 2 | f | t | f | {1 .. 1}
+(2 rows)
+
+-- both columns should have has_nulls=true and [1,1] range
+truncate t;
+insert into t values (1, 1);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+----------
+ 1 | 0 | 1 | f | f | f | {1 .. 1}
+ 1 | 0 | 2 | f | f | f | {1 .. 1}
+(2 rows)
+
+insert into t values (null, null);
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+----------
+ 1 | 0 | 1 | f | t | f | {1 .. 1}
+ 1 | 0 | 2 | f | t | f | {1 .. 1}
+(2 rows)
+
+-- both columns should have has_nulls=true and [1,1] range
+truncate t;
+insert into t values (null, 1);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+----------
+ 1 | 0 | 1 | t | f | f |
+ 1 | 0 | 2 | f | f | f | {1 .. 1}
+(2 rows)
+
+insert into t values (1, null);
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+----------
+ 1 | 0 | 1 | f | t | f | {1 .. 1}
+ 1 | 0 | 2 | f | t | f | {1 .. 1}
+(2 rows)
+
+-- both columns should have has_nulls=false and [1,1] range
+truncate t;
+insert into t values (1, 1);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+----------
+ 1 | 0 | 1 | f | f | f | {1 .. 1}
+ 1 | 0 | 2 | f | f | f | {1 .. 1}
+(2 rows)
+
+insert into t values (1, 1);
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+----------
+ 1 | 0 | 1 | f | f | f | {1 .. 1}
+ 1 | 0 | 2 | f | f | f | {1 .. 1}
+(2 rows)
+
+-- both columns should have all_nulls=true
+truncate t;
+insert into t values (null, null);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+-------
+ 1 | 0 | 1 | t | f | f |
+ 1 | 0 | 2 | t | f | f |
+(2 rows)
+
+insert into t values (null, null);
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+-------
+ 1 | 0 | 1 | t | f | f |
+ 1 | 0 | 2 | t | f | f |
+(2 rows)
+
+-- first column should have has_nulls=false and [1,1] range, second all_nulls=true
+truncate t;
+insert into t values (1, null);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+----------
+ 1 | 0 | 1 | f | f | f | {1 .. 1}
+ 1 | 0 | 2 | t | f | f |
+(2 rows)
+
+insert into t values (1, null);
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
+------------+--------+--------+----------+----------+-------------+----------
+ 1 | 0 | 1 | f | f | f | {1 .. 1}
+ 1 | 0 | 2 | t | f | f |
+(2 rows)
+
diff --git a/contrib/pageinspect/sql/brinbugs.sql b/contrib/pageinspect/sql/brinbugs.sql
new file mode 100644
index 00000000000..a141aed5adc
--- /dev/null
+++ b/contrib/pageinspect/sql/brinbugs.sql
@@ -0,0 +1,114 @@
+create extension pageinspect;
+
+create table t (a int, b int);
+create index on t using brin (a, b);
+
+
+-- both columns should have has_nulls=false and [1,1] range
+truncate t;
+insert into t values (1,1);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+
+-- first column should have all_nulls=true, second has_nulls=false and [1,1] range
+truncate t;
+insert into t values (null, 1);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+
+-- both columns should have all_nulls=true
+truncate t;
+insert into t values (null, null);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+
+
+-- both columns should have has_nulls=true and [1,1] range
+truncate t;
+insert into t values (null, null);
+insert into t values (1, 1);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+
+-- both columns should have has_nulls=true and [1,1] range
+truncate t;
+insert into t values (1, 1);
+insert into t values (null, null);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+
+-- both columns should have has_nulls=true and [1,1] range
+truncate t;
+insert into t values (null, 1);
+insert into t values (1, null);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+
+-- both columns should have has_nulls=false and [1,1] range
+truncate t;
+insert into t values (1, 1);
+insert into t values (1, 1);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+
+-- both columns should have all_nulls=true only
+truncate t;
+insert into t values (null, null);
+insert into t values (null, null);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+
+-- first column should have has_nulls=false and [1,1] range, second all_nulls=true
+truncate t;
+insert into t values (1, null);
+insert into t values (1, null);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+
+-- both columns should have has_nulls=true and [1,1] range
+truncate t;
+insert into t values (null, null);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+insert into t values (1,1);
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+
+-- both columns should have has_nulls=true and [1,1] range
+truncate t;
+insert into t values (1, 1);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+insert into t values (null, null);
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+
+-- both columns should have has_nulls=true and [1,1] range
+truncate t;
+insert into t values (null, 1);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+insert into t values (1, null);
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+
+-- both columns should have has_nulls=false and [1,1] range
+truncate t;
+insert into t values (1, 1);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+insert into t values (1, 1);
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+
+-- both columns should have all_nulls=true
+truncate t;
+insert into t values (null, null);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+insert into t values (null, null);
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+
+-- first column should have has_nulls=false and [1,1] range, second all_nulls=true
+truncate t;
+insert into t values (1, null);
+vacuum full t;
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
+insert into t values (1, null);
+select * from brin_page_items(get_raw_page('t_a_b_idx', 2), 't_a_b_idx'::regclass);
--
2.37.3
From 3f7e2f05570a11b430e40c184b867775cce5efe9 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.von...@postgresql.org>
Date: Thu, 20 Oct 2022 19:55:23 +0200
Subject: [PATCH 2/2] fixup: brin has_nulls
---
src/backend/access/brin/brin.c | 76 ++++++++++++++++++---
src/backend/access/brin/brin_minmax_multi.c | 1 +
src/backend/access/brin/brin_tuple.c | 13 +++-
src/test/regress/expected/brin.out | 2 +-
src/test/regress/expected/brin_bloom.out | 2 +-
src/test/regress/expected/brin_multi.out | 2 +-
6 files changed, 83 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 20b7d65b948..ee9b3bb0574 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -1568,15 +1568,36 @@ form_and_insert_tuple(BrinBuildState *state)
{
BrinTuple *tup;
Size size;
+ bool modified = false;
+ BrinMemTuple *dtuple = state->bs_dtuple;
+ int i;
- tup = brin_form_tuple(state->bs_bdesc, state->bs_currRangeStart,
- state->bs_dtuple, &size);
- brin_doinsert(state->bs_irel, state->bs_pagesPerRange, state->bs_rmAccess,
- &state->bs_currentInsertBuf, state->bs_currRangeStart,
- tup, size);
- state->bs_numtuples++;
+ /*
+ * Was the memtuple modified (any tuples added to it)?
+ *
+ * Should be enough to check just the first attribute - either we add a row
+ * to all columns or none of them.
+ */
+ for (i = 0; i < state->bs_bdesc->bd_tupdesc->natts; i++)
+ {
+ if (!(dtuple->bt_columns[i].bv_allnulls &&
+ dtuple->bt_columns[i].bv_hasnulls))
+ {
+ modified = true;
+ break;
+ }
+ }
- pfree(tup);
+ if (modified)
+ {
+ tup = brin_form_tuple(state->bs_bdesc, state->bs_currRangeStart,
+ state->bs_dtuple, &size);
+ brin_doinsert(state->bs_irel, state->bs_pagesPerRange, state->bs_rmAccess,
+ &state->bs_currentInsertBuf, state->bs_currRangeStart,
+ tup, size);
+ state->bs_numtuples++;
+ pfree(tup);
+ }
}
/*
@@ -1710,24 +1731,53 @@ add_values_to_range(Relation idxRel, BrinDesc *bdesc, BrinMemTuple *dtup,
Datum result;
BrinValues *bval;
FmgrInfo *addValue;
+ bool first_row;
+ bool has_nulls = false;
bval = &dtup->bt_columns[keyno];
+ /*
+ * Is this the first tuple we're adding to the range range? We track
+ * that by setting both bv_hasnulls and bval->bv_allnulls to true
+ * during initialization. But it's not a valid combination (at most
+ * one of those flags should be set), so we reset the second flag.
+ */
+ first_row = (bval->bv_hasnulls && bval->bv_allnulls);
+
+ if (bval->bv_hasnulls && bval->bv_allnulls)
+ {
+ bval->bv_hasnulls = false;
+ modified = true;
+ }
+
if (bdesc->bd_info[keyno]->oi_regular_nulls && nulls[keyno])
{
/*
* If the new value is null, we record that we saw it if it's the
* first one; otherwise, there's nothing to do.
+ *
+ * XXX This used to check "hasnulls" but now that might result in
+ * having both flags set. That used to be OK, because we just
+ * ignore hasnulls flag in brin_form_tuple when allnulls=true.
+ * But now we interpret this combination as "firt row" so it
+ * would confuse following calls. So make sure to only set one
+ * of the flags - when allnulls=true we're done, as it already
+ * marks the range as containing ranges.
*/
- if (!bval->bv_hasnulls)
+ if (!bval->bv_allnulls)
{
bval->bv_hasnulls = true;
modified = true;
}
-
continue;
}
+ /*
+ * Does the range already has NULL values? Either of the flags can
+ * be set, but we ignore the state before adding first row.
+ */
+ has_nulls = (bval->bv_hasnulls || bval->bv_allnulls) && !first_row;
+
addValue = index_getprocinfo(idxRel, keyno + 1,
BRIN_PROCNUM_ADDVALUE);
result = FunctionCall4Coll(addValue,
@@ -1736,8 +1786,16 @@ add_values_to_range(Relation idxRel, BrinDesc *bdesc, BrinMemTuple *dtup,
PointerGetDatum(bval),
values[keyno],
nulls[keyno]);
+
/* if that returned true, we need to insert the updated tuple */
modified |= DatumGetBool(result);
+
+ /*
+ * If we had NULLS, and the opclass didn't set allnulls=true, set
+ * the hasnulls so that we know there are NULL values.
+ */
+ if (has_nulls && !bval->bv_allnulls)
+ bval->bv_hasnulls = true;
}
return modified;
diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c
index 9a0bcf6698d..d5ce5b47ff4 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;
+
modified = true;
column->bv_mem_value = PointerGetDatum(ranges);
diff --git a/src/backend/access/brin/brin_tuple.c b/src/backend/access/brin/brin_tuple.c
index c0e2dbd23ba..7ea272c2f52 100644
--- a/src/backend/access/brin/brin_tuple.c
+++ b/src/backend/access/brin/brin_tuple.c
@@ -136,6 +136,9 @@ brin_form_tuple(BrinDesc *brdesc, BlockNumber blkno, BrinMemTuple *tuple,
{
int datumno;
+ Assert(!(tuple->bt_columns[keyno].bv_hasnulls &&
+ tuple->bt_columns[keyno].bv_allnulls));
+
/*
* "allnulls" is set when there's no nonnull value in any row in the
* column; when this happens, there is no data to store. Thus set the
@@ -517,7 +520,7 @@ brin_memtuple_initialize(BrinMemTuple *dtuple, BrinDesc *brdesc)
{
dtuple->bt_columns[i].bv_attno = i + 1;
dtuple->bt_columns[i].bv_allnulls = true;
- dtuple->bt_columns[i].bv_hasnulls = false;
+ dtuple->bt_columns[i].bv_hasnulls = true;
dtuple->bt_columns[i].bv_values = (Datum *) currdatum;
dtuple->bt_columns[i].bv_mem_value = PointerGetDatum(NULL);
@@ -585,6 +588,14 @@ brin_deform_tuple(BrinDesc *brdesc, BrinTuple *tuple, BrinMemTuple *dMemtuple)
{
int i;
+ Assert(!(allnulls[keyno] && hasnulls[keyno]));
+
+ /*
+ * Make sure to overwrite the hasnulls flag, because it might have
+ * been initialized as true by brin_memtuple_initialize.
+ */
+ dtup->bt_columns[keyno].bv_hasnulls = hasnulls[keyno];
+
if (allnulls[keyno])
{
valueno += brdesc->bd_info[keyno]->oi_nstored;
diff --git a/src/test/regress/expected/brin.out b/src/test/regress/expected/brin.out
index 73fa38396e4..ebc31222354 100644
--- a/src/test/regress/expected/brin.out
+++ b/src/test/regress/expected/brin.out
@@ -454,7 +454,7 @@ $$;
SELECT brin_summarize_range('brin_summarize_idx', 0);
brin_summarize_range
----------------------
- 0
+ 1
(1 row)
-- nothing: already summarized
diff --git a/src/test/regress/expected/brin_bloom.out b/src/test/regress/expected/brin_bloom.out
index 32c56a996a2..6e847f9113d 100644
--- a/src/test/regress/expected/brin_bloom.out
+++ b/src/test/regress/expected/brin_bloom.out
@@ -373,7 +373,7 @@ $$;
SELECT brin_summarize_range('brin_summarize_bloom_idx', 0);
brin_summarize_range
----------------------
- 0
+ 1
(1 row)
-- nothing: already summarized
diff --git a/src/test/regress/expected/brin_multi.out b/src/test/regress/expected/brin_multi.out
index f3309f433f8..e65f1c20d4f 100644
--- a/src/test/regress/expected/brin_multi.out
+++ b/src/test/regress/expected/brin_multi.out
@@ -407,7 +407,7 @@ $$;
SELECT brin_summarize_range('brin_summarize_multi_idx', 0);
brin_summarize_range
----------------------
- 0
+ 1
(1 row)
-- nothing: already summarized
--
2.37.3
