On 5/15/23 12:06, Alvaro Herrera wrote:
> On 2023-May-07, Tomas Vondra wrote:
>
>>> Álvaro wrote:
>>>> In backbranches, the new field to BrinMemTuple needs to be at the end of
>>>> the struct, to avoid ABI breakage.
>>
>> Unfortunately, this is not actually possible :-(
>>
>> The BrinMemTuple has a FLEXIBLE_ARRAY_MEMBER at the end, so we can't
>> place anything after it. I think we have three options:
>>
>> a) some other approach? - I really can't see any, except maybe for going
>> back to the previous approach (i.e. encoding the info using the existing
>> BrinValues allnulls/hasnulls flags)
>
> Actually, mine was quite the stupid suggestion: the BrinMemTuple already
> has a 3 byte hole in the place where you originally wanted to add the
> flag:
>
> struct BrinMemTuple {
> _Bool bt_placeholder; /* 0 1 */
>
> /* XXX 3 bytes hole, try to pack */
>
> BlockNumber bt_blkno; /* 4 4 */
> MemoryContext bt_context; /* 8 8 */
> Datum * bt_values; /* 16 8 */
> _Bool * bt_allnulls; /* 24 8 */
> _Bool * bt_hasnulls; /* 32 8 */
> BrinValues bt_columns[]; /* 40 0 */
>
> /* size: 40, cachelines: 1, members: 7 */
> /* sum members: 37, holes: 1, sum holes: 3 */
> /* last cacheline: 40 bytes */
> };
>
> so putting it there was already not causing any ABI breakage. So, the
> solution to this problem of not being able to put it at the end is just
> to return the struct to your original formulation.
>
Thanks, that's pretty lucky. It means we're not breaking on-disk format
nor ABI, which is great.
Attached is a final version of the patches - I intend to do a bit more
testing, go through the comments once more, and get this committed today
or perhaps tomorrow morning, so that it gets into beta1.
Unfortunately, while polishing the patches, I realized union_tuples()
has yet another long-standing bug with handling NULL values, because it
does this:
/* Adjust "hasnulls". */
if (!col_a->bv_hasnulls && col_b->bv_hasnulls)
col_a->bv_hasnulls = true;
but let's assume "col_a" is a summary representing "1" and "col_b"
represents NULL (col_b->bv_hasnulls=false col_b->bv_allnulls=true).
Well, in that case we fail to "remember" col_a should represent NULL
values too :-(
This is somewhat separate issue, because it's unrelated to empty ranges
(neither of the two ranges is empty). It's hard to test it, because it
requires a particular timing of the concurrent actions, but a breakpoint
in brin.c on the brin_can_do_samepage_update call (in summarize_range)
does the trick for manual testing.
0001 fixes the issue. 0002 is the original fix, and 0003 is just the
pageinspect changes (for master only).
For the backbranches, I thought about making the code more like master
(by moving some of the handling from opclasses to brin.c), but decided
not to. It'd be low-risk, but it feels wrong to kinda do what the master
does under "oi_regular_nulls" flag.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL CompanyFrom 698609154324f64831ac44e4440a2283691184e2 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.von...@postgresql.org>
Date: Thu, 18 May 2023 13:00:31 +0200
Subject: [PATCH 1/3] Fix handling of NULLs when merging BRIN summaries
When merging BRIN summaries, union_tuples() did not correctly update the
target hasnulls/allnulls flags. When merging all-NULL summary into a
summary without any NULL values, the result had both flags set to false
(instead of having hasnulls=true).
This happened because the code only considered the hasnulls flags,
ignoring the possibility the source summary has allnulls=true.
Discovered while investigating issues with handling empty BRIN ranges
and handling of NULL values, but it's a separate problem (has nothing to
do with empty ranges).
Fixed by considering both flags on the source summary, and updating the
hasnulls flag on the target summary.
Backpatch to 11. The bug exists since 9.5 (where BRIN indexes were
introduced), but those releases are EOL already.
Discussion: https://postgr.es/m/402430e4-7d9d-6cf1-09ef-464d80afff3b%40enterprisedb.com
---
src/backend/access/brin/brin.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 41bf950a4af..a155525b7df 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -1613,10 +1613,13 @@ union_tuples(BrinDesc *bdesc, BrinMemTuple *a, BrinTuple *b)
BrinValues *col_b = &db->bt_columns[keyno];
BrinOpcInfo *opcinfo = bdesc->bd_info[keyno];
if (opcinfo->oi_regular_nulls)
{
+ /* Does the "b" summary represent any NULL values? */
+ bool b_has_nulls = (col_b->bv_hasnulls || col_b->bv_allnulls);
+
/* Adjust "hasnulls". */
- if (!col_a->bv_hasnulls && col_b->bv_hasnulls)
+ if (!col_a->bv_allnulls && b_has_nulls)
col_a->bv_hasnulls = true;
/* If there are no values in B, there's nothing left to do. */
@@ -1628,12 +1631,17 @@ union_tuples(BrinDesc *bdesc, BrinMemTuple *a, BrinTuple *b)
* values from B into A, and we're done. We cannot run the
* operators in this case, because values in A might contain
* garbage. Note we already established that B contains values.
+ *
+ * Also adjust "hasnulls" in order not to forget the summary
+ * represents NULL values. This is not redundant with the earlier
+ * update, because that only happens when allnulls=false.
*/
if (col_a->bv_allnulls)
{
int i;
col_a->bv_allnulls = false;
+ col_a->bv_hasnulls = true;
for (i = 0; i < opcinfo->oi_nstored; i++)
col_a->bv_values[i] =
--
2.40.1
From 57ab42cfc55ff7f078de00af8d0e0c44a5354658 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.von...@postgresql.org>
Date: Thu, 18 May 2023 13:33:10 +0200
Subject: [PATCH 1/2] Fix handling of NULLs when merging BRIN summaries
When merging BRIN summaries, union_tuples() did not correctly update the
target hasnulls/allnulls flags. When merging all-NULL summary into a
summary without any NULL values, the result had both flags set to false
(instead of having hasnulls=true).
This happened because the code only considered the hasnulls flags,
ignoring the possibility the source summary has allnulls=true.
Discovered while investigating issues with handling empty BRIN ranges
and handling of NULL values, but it's a separate problem (has nothing to
do with empty ranges).
Fixed by considering both flags on the source summary, and updating the
hasnulls flag on the target summary.
Backpatch to 11. The bug exists since 9.5 (where BRIN indexes were
introduced), but those releases are EOL already.
Discussion: https://postgr.es/m/402430e4-7d9d-6cf1-09ef-464d80afff3b%40enterprisedb.com
---
src/backend/access/brin/brin_inclusion.c | 10 +++++++++-
src/backend/access/brin/brin_minmax.c | 10 +++++++++-
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/brin/brin_inclusion.c b/src/backend/access/brin/brin_inclusion.c
index 7e380d66ed5..ca45178fbff 100644
--- a/src/backend/access/brin/brin_inclusion.c
+++ b/src/backend/access/brin/brin_inclusion.c
@@ -515,10 +515,13 @@ brin_inclusion_union(PG_FUNCTION_ARGS)
FmgrInfo *finfo;
Datum result;
+ /* Does the "b" summary represent any NULL values? */
+ bool b_has_nulls = (col_b->bv_hasnulls || col_b->bv_allnulls);
+
Assert(col_a->bv_attno == col_b->bv_attno);
/* Adjust "hasnulls". */
- if (!col_a->bv_hasnulls && col_b->bv_hasnulls)
+ if (!col_a->bv_allnulls && b_has_nulls)
col_a->bv_hasnulls = true;
/* If there are no values in B, there's nothing left to do. */
@@ -533,10 +536,15 @@ brin_inclusion_union(PG_FUNCTION_ARGS)
* B into A, and we're done. We cannot run the operators in this case,
* because values in A might contain garbage. Note we already established
* that B contains values.
+ *
+ * Also adjust "hasnulls" in order not to forget the summary represents NULL
+ * values. This is not redundant with the earlier update, because that only
+ * happens when allnulls=false.
*/
if (col_a->bv_allnulls)
{
col_a->bv_allnulls = false;
+ col_a->bv_hasnulls = true;
col_a->bv_values[INCLUSION_UNION] =
datumCopy(col_b->bv_values[INCLUSION_UNION],
attr->attbyval, attr->attlen);
diff --git a/src/backend/access/brin/brin_minmax.c b/src/backend/access/brin/brin_minmax.c
index 4b5d6a72135..40da0c8094a 100644
--- a/src/backend/access/brin/brin_minmax.c
+++ b/src/backend/access/brin/brin_minmax.c
@@ -248,10 +248,13 @@ brin_minmax_union(PG_FUNCTION_ARGS)
FmgrInfo *finfo;
bool needsadj;
+ /* Does the "b" summary represent any NULL values? */
+ bool b_has_nulls = (col_b->bv_hasnulls || col_b->bv_allnulls);
+
Assert(col_a->bv_attno == col_b->bv_attno);
/* Adjust "hasnulls" */
- if (!col_a->bv_hasnulls && col_b->bv_hasnulls)
+ if (!col_a->bv_allnulls && b_has_nulls)
col_a->bv_hasnulls = true;
/* If there are no values in B, there's nothing left to do */
@@ -266,10 +269,15 @@ brin_minmax_union(PG_FUNCTION_ARGS)
* B into A, and we're done. We cannot run the operators in this case,
* because values in A might contain garbage. Note we already established
* that B contains values.
+ *
+ * Also adjust "hasnulls" in order not to forget the summary represents NULL
+ * values. This is not redundant with the earlier update, because that only
+ * happens when allnulls=false.
*/
if (col_a->bv_allnulls)
{
col_a->bv_allnulls = false;
+ col_a->bv_hasnulls = true;
col_a->bv_values[0] = datumCopy(col_b->bv_values[0],
attr->attbyval, attr->attlen);
col_a->bv_values[1] = datumCopy(col_b->bv_values[1],
--
2.40.1
From 638fe7f96d21bc90622dab8a48bb8018fed9150e Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.von...@postgresql.org>
Date: Sun, 23 Apr 2023 19:26:18 +0200
Subject: [PATCH 2/2] Fix handling of NULLs in BRIN indexes
BRIN indexes did not properly distinguish between summaries for empty
(no rows) and all-NULL ranges, treating them as essentially the same
thing. Summaries were initialized with allnulls=true, and opclasses
simply reset allnulls to false when processing the first non-NULL value.
This however produces incorrect results if the range starts with a NULL
value (or a sequence of NULL values), in which case we forget the range
contains NULL values when adding the first non-NULL value.
This happens because the allnulls flag is used for two separate
purposes - to mark empty ranges (not representing any rows yet) and
ranges containing only NULL values.
Opclasses don't know which of these cases it is, and so don't know
whether to set hasnulls=true. Setting the flag in both cases would make
it correct, but it would also make BRIN indexes useless for queries with
IS NULL clauses. All ranges start empty (and thus allnulls=true), so all
ranges would end up with either allnulls=true or hasnulls=true.
The severity of the issue is somewhat reduced by the fact that it only
happens when adding values to an existing summary with allnulls=true.
This can happen e.g. for small tables (because a summary for the first
range exists for all BRIN indexes), or for tables with large fraction of
NULL values in the indexed columns.
Bulk summarization (e.g. during CREATE INDEX or automatic summarization)
that processes all values at once is not affected by this issue. In this
case the flags were updated in a slightly different way, not forgetting
the NULL values.
To identify empty ranges we use a new flag, stored in an unused bit in
the BRIN tuple header so the on-disk format remains the same. A matching
flag is added to BrinMemTuple, into a 3B gap after bt_placeholder.
That means there's no risk of ABI breakage, although we don't actually
pass the BrinMemTuple to any public API.
We could also skip storing index tuples for empty summaries, but then
we'd have to always process such ranges - even if there are no rows in
large parts of the table (e.g. after a bulk DELETE), it would still
require reading the pages etc. So we store them, but ignore them when
building the bitmap.
Backpatch to 11. The issue exists since BRIN indexes were introduced in
9.5, but older releases are already EOL.
Backpatch-through: 11
Reviewed-by: Justin Pryzby, Matthias van de Meent, Alvaro Herrera
Discussion: https://postgr.es/m/402430e4-7d9d-6cf1-09ef-464d80aff...@enterprisedb.com
---
src/backend/access/brin/brin.c | 176 +++++++++++++++++-
src/backend/access/brin/brin_tuple.c | 14 +-
src/include/access/brin_tuple.h | 6 +-
...summarization-and-inprogress-insertion.out | 8 +-
...ummarization-and-inprogress-insertion.spec | 1 +
5 files changed, 191 insertions(+), 14 deletions(-)
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 0becfde1133..176ae0099ff 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -35,6 +35,7 @@
#include "storage/freespace.h"
#include "utils/acl.h"
#include "utils/builtins.h"
+#include "utils/datum.h"
#include "utils/index_selfuncs.h"
#include "utils/memutils.h"
#include "utils/rel.h"
@@ -173,7 +174,7 @@ brininsert(Relation idxRel, Datum *values, bool *nulls,
for (;;)
{
- bool need_insert = false;
+ bool need_insert;
OffsetNumber off;
BrinTuple *brtup;
BrinMemTuple *dtup;
@@ -241,6 +242,9 @@ brininsert(Relation idxRel, Datum *values, bool *nulls,
dtup = brin_deform_tuple(bdesc, brtup, NULL);
+ /* If the range starts empty, we're certainly going to modify it. */
+ need_insert = dtup->bt_empty_range;
+
/*
* Compare the key values of the new tuple to the stored index values;
* our deformed tuple will get updated if the new tuple doesn't fit
@@ -253,8 +257,20 @@ brininsert(Relation idxRel, Datum *values, bool *nulls,
Datum result;
BrinValues *bval;
FmgrInfo *addValue;
+ bool has_nulls;
bval = &dtup->bt_columns[keyno];
+
+ /*
+ * Does the range have actual NULL values? Either of the flags can
+ * be set, but we ignore the state before adding first row.
+ *
+ * We have to remember this, because we'll modify the flags and we
+ * need to know if the range started as empty.
+ */
+ has_nulls = ((!dtup->bt_empty_range) &&
+ (bval->bv_hasnulls || bval->bv_allnulls));
+
addValue = index_getprocinfo(idxRel, keyno + 1,
BRIN_PROCNUM_ADDVALUE);
result = FunctionCall4Coll(addValue,
@@ -265,8 +281,33 @@ brininsert(Relation idxRel, Datum *values, bool *nulls,
nulls[keyno]);
/* if that returned true, we need to insert the updated tuple */
need_insert |= DatumGetBool(result);
+
+ /*
+ * If the range was had actual NULL values (i.e. did not start empty),
+ * make sure we don't forget about the NULL values. Either the allnulls
+ * flag is still set to true, or (if the opclass cleared it) we need to
+ * set hasnulls=true.
+ *
+ * XXX This can only happen when the opclass modified the tuple, so the
+ * modified flag should be set.
+ */
+ if (has_nulls && !(bval->bv_hasnulls || bval->bv_allnulls))
+ {
+ Assert(need_insert);
+ bval->bv_hasnulls = true;
+ }
}
+ /*
+ * After updating summaries for all the keys, mark it as not empty.
+ *
+ * If we're actually changing the flag value (i.e. tuple started as
+ * empty), we should have modified the tuple. So we should not see
+ * empty range that was not modified.
+ */
+ Assert(!dtup->bt_empty_range || need_insert);
+ dtup->bt_empty_range = false;
+
if (!need_insert)
{
/*
@@ -508,6 +549,17 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
CurrentMemoryContext);
}
+ /*
+ * If the BRIN tuple indicates that this range is empty,
+ * we can skip it: there's nothing to match. We don't
+ * need to examine the next columns.
+ */
+ if (dtup->bt_empty_range)
+ {
+ addrange = false;
+ break;
+ }
+
/*
* Check whether the scan key is consistent with the page
* range values; if so, have the pages in the range added
@@ -612,6 +664,7 @@ brinbuildCallback(Relation index,
BrinBuildState *state = (BrinBuildState *) brstate;
BlockNumber thisblock;
int i;
+ bool modified;
thisblock = ItemPointerGetBlockNumber(tid);
@@ -640,25 +693,76 @@ brinbuildCallback(Relation index,
}
/* Accumulate the current tuple into the running state */
+
+ /* If the range starts empty, we're certainly going to modify it. */
+ modified = state->bs_dtuple->bt_empty_range;
+
+ /*
+ * Compare the key values of the new tuple to the stored index values;
+ * our deformed tuple will get updated if the new tuple doesn't fit
+ * the original range (note this means we can't break out of the loop
+ * early). Make a note of whether this happens, so that we know to
+ * insert the modified tuple later.
+ */
for (i = 0; i < state->bs_bdesc->bd_tupdesc->natts; i++)
{
FmgrInfo *addValue;
BrinValues *col;
Form_pg_attribute attr = TupleDescAttr(state->bs_bdesc->bd_tupdesc, i);
+ bool has_nulls;
+ Datum result;
col = &state->bs_dtuple->bt_columns[i];
+
+ /*
+ * Does the range have actual NULL values? Either of the flags can
+ * be set, but we ignore the state before adding first row.
+ *
+ * We have to remember this, because we'll modify the flags and we
+ * need to know if the range started as empty.
+ */
+ has_nulls = ((!state->bs_dtuple->bt_empty_range) &&
+ (col->bv_hasnulls || col->bv_allnulls));
+
+ /*
+ * Call the BRIN_PROCNUM_ADDVALUE procedure. We do this even for NULL
+ * values, because who knows what the opclass is doing.
+ */
addValue = index_getprocinfo(index, i + 1,
BRIN_PROCNUM_ADDVALUE);
+ result = FunctionCall4Coll(addValue,
+ attr->attcollation,
+ PointerGetDatum(state->bs_bdesc),
+ PointerGetDatum(col),
+ values[i], isnull[i]);
+ /* if that returned true, we need to insert the updated tuple */
+ modified |= DatumGetBool(result);
/*
- * Update dtuple state, if and as necessary.
+ * If the range was had actual NULL values (i.e. did not start empty),
+ * make sure we don't forget about the NULL values. Either the allnulls
+ * flag is still set to true, or (if the opclass cleared it) we need to
+ * set hasnulls=true.
+ *
+ * XXX This can only happen when the opclass modified the tuple, so the
+ * modified flag should be set.
*/
- FunctionCall4Coll(addValue,
- attr->attcollation,
- PointerGetDatum(state->bs_bdesc),
- PointerGetDatum(col),
- values[i], isnull[i]);
+ if (has_nulls && !(col->bv_hasnulls || col->bv_allnulls))
+ {
+ Assert(modified);
+ col->bv_hasnulls = true;
+ }
}
+
+ /*
+ * After updating summaries for all the keys, mark it as not empty.
+ *
+ * If we're actually changing the flag value (i.e. tuple started as
+ * empty), we should have modified the tuple. So we should not see
+ * empty range that was not modified.
+ */
+ Assert(!state->bs_dtuple->bt_empty_range || modified);
+ state->bs_dtuple->bt_empty_range = false;
}
/*
@@ -1465,6 +1569,64 @@ union_tuples(BrinDesc *bdesc, BrinMemTuple *a, BrinTuple *b)
db = brin_deform_tuple(bdesc, b, NULL);
MemoryContextSwitchTo(oldcxt);
+ /*
+ * Check if the ranges are empty.
+ *
+ * If at least one of them is empty, we don't need to call per-key union
+ * functions at all. If "b" is empty, we just use "a" as the result (it
+ * might be empty fine, but that's fine). If "a" is empty but "b" is not,
+ * we use "b" as the result (but we have to copy the data into "a" first).
+ *
+ * Only when both ranges are non-empty, we actually do the per-key merge.
+ */
+
+ /* If "b" is empty - ignore it and just use "a" (even if it's empty etc.). */
+ if (db->bt_empty_range)
+ {
+ /* skip the per-key merge */
+ MemoryContextDelete(cxt);
+ return;
+ }
+
+ /*
+ * Now we know "b" is not empty. If "a" is empty, then "b" is the result.
+ * But we need to copy the data from "b" to "a" first, because that's how
+ * we pass result out.
+ *
+ * We have to copy all the global/per-key flags etc. too.
+ */
+ if (a->bt_empty_range)
+ {
+ for (keyno = 0; keyno < bdesc->bd_tupdesc->natts; keyno++)
+ {
+ int i;
+ BrinValues *col_a = &a->bt_columns[keyno];
+ BrinValues *col_b = &db->bt_columns[keyno];
+ BrinOpcInfo *opcinfo = bdesc->bd_info[keyno];
+
+ col_a->bv_allnulls = col_b->bv_allnulls;
+ col_a->bv_hasnulls = col_b->bv_hasnulls;
+
+ /* If "b" has no data, we're done. */
+ if (col_b->bv_allnulls)
+ continue;
+
+ for (i = 0; i < opcinfo->oi_nstored; i++)
+ col_a->bv_values[i] =
+ datumCopy(col_b->bv_values[i],
+ opcinfo->oi_typcache[i]->typbyval,
+ opcinfo->oi_typcache[i]->typlen);
+ }
+
+ /* "a" started empty, but "b" was not empty, so remember that */
+ a->bt_empty_range = false;
+
+ /* skip the per-key merge */
+ MemoryContextDelete(cxt);
+ return;
+ }
+
+ /* Neither range is empty, so call the union proc. */
for (keyno = 0; keyno < bdesc->bd_tupdesc->natts; keyno++)
{
FmgrInfo *unionFn;
diff --git a/src/backend/access/brin/brin_tuple.c b/src/backend/access/brin/brin_tuple.c
index b3b453aed12..aafd2f17ca0 100644
--- a/src/backend/access/brin/brin_tuple.c
+++ b/src/backend/access/brin/brin_tuple.c
@@ -349,6 +349,9 @@ brin_form_tuple(BrinDesc *brdesc, BlockNumber blkno, BrinMemTuple *tuple,
if (tuple->bt_placeholder)
rettuple->bt_info |= BRIN_PLACEHOLDER_MASK;
+ if (tuple->bt_empty_range)
+ rettuple->bt_info |= BRIN_EMPTY_RANGE_MASK;
+
*size = len;
return rettuple;
}
@@ -376,7 +379,7 @@ brin_form_placeholder_tuple(BrinDesc *brdesc, BlockNumber blkno, Size *size)
rettuple = palloc0(len);
rettuple->bt_blkno = blkno;
rettuple->bt_info = hoff;
- rettuple->bt_info |= BRIN_NULLS_MASK | BRIN_PLACEHOLDER_MASK;
+ rettuple->bt_info |= BRIN_NULLS_MASK | BRIN_PLACEHOLDER_MASK | BRIN_EMPTY_RANGE_MASK;
bitP = ((bits8 *) ((char *) rettuple + SizeOfBrinTuple)) - 1;
bitmask = HIGHBIT;
@@ -466,6 +469,8 @@ brin_new_memtuple(BrinDesc *brdesc)
dtup->bt_allnulls = palloc(sizeof(bool) * brdesc->bd_tupdesc->natts);
dtup->bt_hasnulls = palloc(sizeof(bool) * brdesc->bd_tupdesc->natts);
+ dtup->bt_empty_range = true;
+
dtup->bt_context = AllocSetContextCreate(CurrentMemoryContext,
"brin dtuple",
ALLOCSET_DEFAULT_SIZES);
@@ -499,6 +504,8 @@ brin_memtuple_initialize(BrinMemTuple *dtuple, BrinDesc *brdesc)
currdatum += sizeof(Datum) * brdesc->bd_info[i]->oi_nstored;
}
+ dtuple->bt_empty_range = true;
+
return dtuple;
}
@@ -532,6 +539,11 @@ brin_deform_tuple(BrinDesc *brdesc, BrinTuple *tuple, BrinMemTuple *dMemtuple)
if (BrinTupleIsPlaceholder(tuple))
dtup->bt_placeholder = true;
+
+ /* ranges start as empty, depends on the BrinTuple */
+ if (!BrinTupleIsEmptyRange(tuple))
+ dtup->bt_empty_range = false;
+
dtup->bt_blkno = tuple->bt_blkno;
values = dtup->bt_values;
diff --git a/src/include/access/brin_tuple.h b/src/include/access/brin_tuple.h
index a9ccc3995b4..5280872707a 100644
--- a/src/include/access/brin_tuple.h
+++ b/src/include/access/brin_tuple.h
@@ -36,6 +36,7 @@ typedef struct BrinValues
typedef struct BrinMemTuple
{
bool bt_placeholder; /* this is a placeholder tuple */
+ bool bt_empty_range; /* range represents no tuples */
BlockNumber bt_blkno; /* heap blkno that the tuple is for */
MemoryContext bt_context; /* memcxt holding the bt_columns values */
/* output arrays for brin_deform_tuple: */
@@ -61,7 +62,7 @@ typedef struct BrinTuple
*
* 7th (high) bit: has nulls
* 6th bit: is placeholder tuple
- * 5th bit: unused
+ * 5th bit: range is empty
* 4-0 bit: offset of data
* ---------------
*/
@@ -74,13 +75,14 @@ typedef struct BrinTuple
* bt_info manipulation macros
*/
#define BRIN_OFFSET_MASK 0x1F
-/* bit 0x20 is not used at present */
+#define BRIN_EMPTY_RANGE_MASK 0x20
#define BRIN_PLACEHOLDER_MASK 0x40
#define BRIN_NULLS_MASK 0x80
#define BrinTupleDataOffset(tup) ((Size) (((BrinTuple *) (tup))->bt_info & BRIN_OFFSET_MASK))
#define BrinTupleHasNulls(tup) (((((BrinTuple *) (tup))->bt_info & BRIN_NULLS_MASK)) != 0)
#define BrinTupleIsPlaceholder(tup) (((((BrinTuple *) (tup))->bt_info & BRIN_PLACEHOLDER_MASK)) != 0)
+#define BrinTupleIsEmptyRange(tup) (((((BrinTuple *) (tup))->bt_info & BRIN_EMPTY_RANGE_MASK)) != 0)
extern BrinTuple *brin_form_tuple(BrinDesc *brdesc, BlockNumber blkno,
diff --git a/src/test/modules/brin/expected/summarization-and-inprogress-insertion.out b/src/test/modules/brin/expected/summarization-and-inprogress-insertion.out
index 2a4755d0998..584ac2602f7 100644
--- a/src/test/modules/brin/expected/summarization-and-inprogress-insertion.out
+++ b/src/test/modules/brin/expected/summarization-and-inprogress-insertion.out
@@ -4,7 +4,7 @@ starting permutation: s2check s1b s2b s1i s2summ s1c s2c s2check
step s2check: SELECT * FROM brin_page_items(get_raw_page('brinidx', 2), 'brinidx'::regclass);
itemoffset|blknum|attnum|allnulls|hasnulls|placeholder|value
----------+------+------+--------+--------+-----------+--------
- 1| 0| 1|f |f |f |{1 .. 1}
+ 1| 0| 1|f |t |f |{1 .. 1}
(1 row)
step s1b: BEGIN ISOLATION LEVEL REPEATABLE READ;
@@ -26,7 +26,7 @@ step s2c: COMMIT;
step s2check: SELECT * FROM brin_page_items(get_raw_page('brinidx', 2), 'brinidx'::regclass);
itemoffset|blknum|attnum|allnulls|hasnulls|placeholder|value
----------+------+------+--------+--------+-----------+-----------
- 1| 0| 1|f |f |f |{1 .. 1}
+ 1| 0| 1|f |t |f |{1 .. 1}
2| 1| 1|f |f |f |{1 .. 1000}
(2 rows)
@@ -35,7 +35,7 @@ starting permutation: s2check s1b s1i s2vacuum s1c s2check
step s2check: SELECT * FROM brin_page_items(get_raw_page('brinidx', 2), 'brinidx'::regclass);
itemoffset|blknum|attnum|allnulls|hasnulls|placeholder|value
----------+------+------+--------+--------+-----------+--------
- 1| 0| 1|f |f |f |{1 .. 1}
+ 1| 0| 1|f |t |f |{1 .. 1}
(1 row)
step s1b: BEGIN ISOLATION LEVEL REPEATABLE READ;
@@ -45,7 +45,7 @@ step s1c: COMMIT;
step s2check: SELECT * FROM brin_page_items(get_raw_page('brinidx', 2), 'brinidx'::regclass);
itemoffset|blknum|attnum|allnulls|hasnulls|placeholder|value
----------+------+------+--------+--------+-----------+-----------
- 1| 0| 1|f |f |f |{1 .. 1}
+ 1| 0| 1|f |t |f |{1 .. 1}
2| 1| 1|f |f |f |{1 .. 1000}
(2 rows)
diff --git a/src/test/modules/brin/specs/summarization-and-inprogress-insertion.spec b/src/test/modules/brin/specs/summarization-and-inprogress-insertion.spec
index 19ac18a2e88..18ba92b7ba1 100644
--- a/src/test/modules/brin/specs/summarization-and-inprogress-insertion.spec
+++ b/src/test/modules/brin/specs/summarization-and-inprogress-insertion.spec
@@ -9,6 +9,7 @@ setup
) WITH (fillfactor=10);
CREATE INDEX brinidx ON brin_iso USING brin (value) WITH (pages_per_range=1);
-- this fills the first page
+ INSERT INTO brin_iso VALUES (NULL);
DO $$
DECLARE curtid tid;
BEGIN
--
2.40.1
From d82099ce68e58b28f5f7331f0dcbe013dbac65b0 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.von...@postgresql.org>
Date: Sun, 8 Jan 2023 16:43:06 +0100
Subject: [PATCH 2/3] Fix handling of NULLs in BRIN indexes
BRIN indexes did not properly distinguish between summaries for empty
(no rows) and all-NULL ranges, treating them as essentially the same
thing. Summaries were initialized with allnulls=true, and opclasses
simply reset allnulls to false when processing the first non-NULL value.
This however produces incorrect results if the range starts with a NULL
value (or a sequence of NULL values), in which case we forget the range
contains NULL values when adding the first non-NULL value.
This happens because the allnulls flag is used for two separate
purposes - to mark empty ranges (not representing any rows yet) and
ranges containing only NULL values.
Opclasses don't know which of these cases it is, and so don't know
whether to set hasnulls=true. Setting the flag in both cases would make
it correct, but it would also make BRIN indexes useless for queries with
IS NULL clauses. All ranges start empty (and thus allnulls=true), so all
ranges would end up with either allnulls=true or hasnulls=true.
The severity of the issue is somewhat reduced by the fact that it only
happens when adding values to an existing summary with allnulls=true.
This can happen e.g. for small tables (because a summary for the first
range exists for all BRIN indexes), or for tables with large fraction of
NULL values in the indexed columns.
Bulk summarization (e.g. during CREATE INDEX or automatic summarization)
that processes all values at once is not affected by this issue. In this
case the flags were updated in a slightly different way, not forgetting
the NULL values.
To identify empty ranges we use a new flag, stored in an unused bit in
the BRIN tuple header so the on-disk format remains the same. A matching
flag is added to BrinMemTuple, into a 3B gap after bt_placeholder.
That means there's no risk of ABI breakage, although we don't actually
pass the BrinMemTuple to any public API.
We could also skip storing index tuples for empty summaries, but then
we'd have to always process such ranges - even if there are no rows in
large parts of the table (e.g. after a bulk DELETE), it would still
require reading the pages etc. So we store them, but ignore them when
building the bitmap.
Backpatch to 11. The issue exists since BRIN indexes were introduced in
9.5, but older releases are already EOL.
Backpatch-through: 11
Reviewed-by: Justin Pryzby, Matthias van de Meent, Alvaro Herrera
Discussion: https://postgr.es/m/402430e4-7d9d-6cf1-09ef-464d80aff...@enterprisedb.com
---
src/backend/access/brin/brin.c | 113 +++++++++++++++++-
src/backend/access/brin/brin_tuple.c | 14 ++-
src/include/access/brin_tuple.h | 6 +-
...summarization-and-inprogress-insertion.out | 8 +-
...ummarization-and-inprogress-insertion.spec | 1 +
5 files changed, 134 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index a155525b7df..46aa1f1bc80 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -592,6 +592,17 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
bval = &dtup->bt_columns[attno - 1];
+ /*
+ * If the BRIN tuple indicates that this range is empty,
+ * we can skip it: there's nothing to match. We don't
+ * need to examine the next columns.
+ */
+ if (dtup->bt_empty_range)
+ {
+ addrange = false;
+ break;
+ }
+
/*
* First check if there are any IS [NOT] NULL scan keys,
* and if we're violating them. In that case we can
@@ -1606,6 +1617,64 @@ union_tuples(BrinDesc *bdesc, BrinMemTuple *a, BrinTuple *b)
db = brin_deform_tuple(bdesc, b, NULL);
MemoryContextSwitchTo(oldcxt);
+ /*
+ * Check if the ranges are empty.
+ *
+ * If at least one of them is empty, we don't need to call per-key union
+ * functions at all. If "b" is empty, we just use "a" as the result (it
+ * might be empty fine, but that's fine). If "a" is empty but "b" is not,
+ * we use "b" as the result (but we have to copy the data into "a" first).
+ *
+ * Only when both ranges are non-empty, we actually do the per-key merge.
+ */
+
+ /* If "b" is empty - ignore it and just use "a" (even if it's empty etc.). */
+ if (db->bt_empty_range)
+ {
+ /* skip the per-key merge */
+ MemoryContextDelete(cxt);
+ return;
+ }
+
+ /*
+ * Now we know "b" is not empty. If "a" is empty, then "b" is the result.
+ * But we need to copy the data from "b" to "a" first, because that's how
+ * we pass result out.
+ *
+ * We have to copy all the global/per-key flags etc. too.
+ */
+ if (a->bt_empty_range)
+ {
+ for (keyno = 0; keyno < bdesc->bd_tupdesc->natts; keyno++)
+ {
+ int i;
+ BrinValues *col_a = &a->bt_columns[keyno];
+ BrinValues *col_b = &db->bt_columns[keyno];
+ BrinOpcInfo *opcinfo = bdesc->bd_info[keyno];
+
+ col_a->bv_allnulls = col_b->bv_allnulls;
+ col_a->bv_hasnulls = col_b->bv_hasnulls;
+
+ /* If "b" has no data, we're done. */
+ if (col_b->bv_allnulls)
+ continue;
+
+ for (i = 0; i < opcinfo->oi_nstored; i++)
+ col_a->bv_values[i] =
+ datumCopy(col_b->bv_values[i],
+ opcinfo->oi_typcache[i]->typbyval,
+ opcinfo->oi_typcache[i]->typlen);
+ }
+
+ /* "a" started empty, but "b" was not empty, so remember that */
+ a->bt_empty_range = false;
+
+ /* skip the per-key merge */
+ MemoryContextDelete(cxt);
+ return;
+ }
+
+ /* Now we know neither range is empty. */
for (keyno = 0; keyno < bdesc->bd_tupdesc->natts; keyno++)
{
FmgrInfo *unionFn;
@@ -1711,7 +1780,9 @@ add_values_to_range(Relation idxRel, BrinDesc *bdesc, BrinMemTuple *dtup,
Datum *values, bool *nulls)
{
int keyno;
- bool modified = false;
+
+ /* If the range starts empty, we're certainly going to modify it. */
+ bool modified = dtup->bt_empty_range;
/*
* Compare the key values of the new tuple to the stored index values; our
@@ -1725,9 +1796,24 @@ add_values_to_range(Relation idxRel, BrinDesc *bdesc, BrinMemTuple *dtup,
Datum result;
BrinValues *bval;
FmgrInfo *addValue;
+ bool has_nulls;
bval = &dtup->bt_columns[keyno];
+ /*
+ * Does the range have actual NULL values? Either of the flags can
+ * be set, but we ignore the state before adding first row.
+ *
+ * We have to remember this, because we'll modify the flags and we
+ * need to know if the range started as empty.
+ */
+ has_nulls = ((!dtup->bt_empty_range) &&
+ (bval->bv_hasnulls || bval->bv_allnulls));
+
+ /*
+ * If the value we're adding is NULL, handle it locally. Otherwise
+ * call the BRIN_PROCNUM_ADDVALUE procedure.
+ */
if (bdesc->bd_info[keyno]->oi_regular_nulls && nulls[keyno])
{
/*
@@ -1753,8 +1839,33 @@ add_values_to_range(Relation idxRel, BrinDesc *bdesc, BrinMemTuple *dtup,
nulls[keyno]);
/* if that returned true, we need to insert the updated tuple */
modified |= DatumGetBool(result);
+
+ /*
+ * If the range was had actual NULL values (i.e. did not start empty),
+ * make sure we don't forget about the NULL values. Either the allnulls
+ * flag is still set to true, or (if the opclass cleared it) we need to
+ * set hasnulls=true.
+ *
+ * XXX This can only happen when the opclass modified the tuple, so the
+ * modified flag should be set.
+ */
+ if (has_nulls && !(bval->bv_hasnulls || bval->bv_allnulls))
+ {
+ Assert(modified);
+ bval->bv_hasnulls = true;
+ }
}
+ /*
+ * After updating summaries for all the keys, mark it as not empty.
+ *
+ * If we're actually changing the flag value (i.e. tuple started as empty),
+ * we should have modified the tuple. So we should not see empty range that
+ * was not modified.
+ */
+ Assert(!dtup->bt_empty_range || modified);
+ dtup->bt_empty_range = false;
+
return modified;
}
diff --git a/src/backend/access/brin/brin_tuple.c b/src/backend/access/brin/brin_tuple.c
index 84b79dbfc0d..23dfeab7de8 100644
--- a/src/backend/access/brin/brin_tuple.c
+++ b/src/backend/access/brin/brin_tuple.c
@@ -372,6 +372,9 @@ brin_form_tuple(BrinDesc *brdesc, BlockNumber blkno, BrinMemTuple *tuple,
if (tuple->bt_placeholder)
rettuple->bt_info |= BRIN_PLACEHOLDER_MASK;
+ if (tuple->bt_empty_range)
+ rettuple->bt_info |= BRIN_EMPTY_RANGE_MASK;
+
*size = len;
return rettuple;
}
@@ -399,7 +402,7 @@ brin_form_placeholder_tuple(BrinDesc *brdesc, BlockNumber blkno, Size *size)
rettuple = palloc0(len);
rettuple->bt_blkno = blkno;
rettuple->bt_info = hoff;
- rettuple->bt_info |= BRIN_NULLS_MASK | BRIN_PLACEHOLDER_MASK;
+ rettuple->bt_info |= BRIN_NULLS_MASK | BRIN_PLACEHOLDER_MASK | BRIN_EMPTY_RANGE_MASK;
bitP = ((bits8 *) ((char *) rettuple + SizeOfBrinTuple)) - 1;
bitmask = HIGHBIT;
@@ -489,6 +492,8 @@ brin_new_memtuple(BrinDesc *brdesc)
dtup->bt_allnulls = palloc(sizeof(bool) * brdesc->bd_tupdesc->natts);
dtup->bt_hasnulls = palloc(sizeof(bool) * brdesc->bd_tupdesc->natts);
+ dtup->bt_empty_range = true;
+
dtup->bt_context = AllocSetContextCreate(CurrentMemoryContext,
"brin dtuple",
ALLOCSET_DEFAULT_SIZES);
@@ -527,6 +532,8 @@ brin_memtuple_initialize(BrinMemTuple *dtuple, BrinDesc *brdesc)
currdatum += sizeof(Datum) * brdesc->bd_info[i]->oi_nstored;
}
+ dtuple->bt_empty_range = true;
+
return dtuple;
}
@@ -560,6 +567,11 @@ brin_deform_tuple(BrinDesc *brdesc, BrinTuple *tuple, BrinMemTuple *dMemtuple)
if (BrinTupleIsPlaceholder(tuple))
dtup->bt_placeholder = true;
+
+ /* ranges start as empty, depends on the BrinTuple */
+ if (!BrinTupleIsEmptyRange(tuple))
+ dtup->bt_empty_range = false;
+
dtup->bt_blkno = tuple->bt_blkno;
values = dtup->bt_values;
diff --git a/src/include/access/brin_tuple.h b/src/include/access/brin_tuple.h
index 732f91edf11..c56747aca4a 100644
--- a/src/include/access/brin_tuple.h
+++ b/src/include/access/brin_tuple.h
@@ -44,6 +44,7 @@ typedef struct BrinValues
typedef struct BrinMemTuple
{
bool bt_placeholder; /* this is a placeholder tuple */
+ bool bt_empty_range; /* range represents no tuples */
BlockNumber bt_blkno; /* heap blkno that the tuple is for */
MemoryContext bt_context; /* memcxt holding the bt_columns values */
/* output arrays for brin_deform_tuple: */
@@ -69,7 +70,7 @@ typedef struct BrinTuple
*
* 7th (high) bit: has nulls
* 6th bit: is placeholder tuple
- * 5th bit: unused
+ * 5th bit: range is empty
* 4-0 bit: offset of data
* ---------------
*/
@@ -82,13 +83,14 @@ typedef struct BrinTuple
* bt_info manipulation macros
*/
#define BRIN_OFFSET_MASK 0x1F
-/* bit 0x20 is not used at present */
+#define BRIN_EMPTY_RANGE_MASK 0x20
#define BRIN_PLACEHOLDER_MASK 0x40
#define BRIN_NULLS_MASK 0x80
#define BrinTupleDataOffset(tup) ((Size) (((BrinTuple *) (tup))->bt_info & BRIN_OFFSET_MASK))
#define BrinTupleHasNulls(tup) (((((BrinTuple *) (tup))->bt_info & BRIN_NULLS_MASK)) != 0)
#define BrinTupleIsPlaceholder(tup) (((((BrinTuple *) (tup))->bt_info & BRIN_PLACEHOLDER_MASK)) != 0)
+#define BrinTupleIsEmptyRange(tup) (((((BrinTuple *) (tup))->bt_info & BRIN_EMPTY_RANGE_MASK)) != 0)
extern BrinTuple *brin_form_tuple(BrinDesc *brdesc, BlockNumber blkno,
diff --git a/src/test/modules/brin/expected/summarization-and-inprogress-insertion.out b/src/test/modules/brin/expected/summarization-and-inprogress-insertion.out
index 2a4755d0998..584ac2602f7 100644
--- a/src/test/modules/brin/expected/summarization-and-inprogress-insertion.out
+++ b/src/test/modules/brin/expected/summarization-and-inprogress-insertion.out
@@ -4,7 +4,7 @@ starting permutation: s2check s1b s2b s1i s2summ s1c s2c s2check
step s2check: SELECT * FROM brin_page_items(get_raw_page('brinidx', 2), 'brinidx'::regclass);
itemoffset|blknum|attnum|allnulls|hasnulls|placeholder|value
----------+------+------+--------+--------+-----------+--------
- 1| 0| 1|f |f |f |{1 .. 1}
+ 1| 0| 1|f |t |f |{1 .. 1}
(1 row)
step s1b: BEGIN ISOLATION LEVEL REPEATABLE READ;
@@ -26,7 +26,7 @@ step s2c: COMMIT;
step s2check: SELECT * FROM brin_page_items(get_raw_page('brinidx', 2), 'brinidx'::regclass);
itemoffset|blknum|attnum|allnulls|hasnulls|placeholder|value
----------+------+------+--------+--------+-----------+-----------
- 1| 0| 1|f |f |f |{1 .. 1}
+ 1| 0| 1|f |t |f |{1 .. 1}
2| 1| 1|f |f |f |{1 .. 1000}
(2 rows)
@@ -35,7 +35,7 @@ starting permutation: s2check s1b s1i s2vacuum s1c s2check
step s2check: SELECT * FROM brin_page_items(get_raw_page('brinidx', 2), 'brinidx'::regclass);
itemoffset|blknum|attnum|allnulls|hasnulls|placeholder|value
----------+------+------+--------+--------+-----------+--------
- 1| 0| 1|f |f |f |{1 .. 1}
+ 1| 0| 1|f |t |f |{1 .. 1}
(1 row)
step s1b: BEGIN ISOLATION LEVEL REPEATABLE READ;
@@ -45,7 +45,7 @@ step s1c: COMMIT;
step s2check: SELECT * FROM brin_page_items(get_raw_page('brinidx', 2), 'brinidx'::regclass);
itemoffset|blknum|attnum|allnulls|hasnulls|placeholder|value
----------+------+------+--------+--------+-----------+-----------
- 1| 0| 1|f |f |f |{1 .. 1}
+ 1| 0| 1|f |t |f |{1 .. 1}
2| 1| 1|f |f |f |{1 .. 1000}
(2 rows)
diff --git a/src/test/modules/brin/specs/summarization-and-inprogress-insertion.spec b/src/test/modules/brin/specs/summarization-and-inprogress-insertion.spec
index 19ac18a2e88..18ba92b7ba1 100644
--- a/src/test/modules/brin/specs/summarization-and-inprogress-insertion.spec
+++ b/src/test/modules/brin/specs/summarization-and-inprogress-insertion.spec
@@ -9,6 +9,7 @@ setup
) WITH (fillfactor=10);
CREATE INDEX brinidx ON brin_iso USING brin (value) WITH (pages_per_range=1);
-- this fills the first page
+ INSERT INTO brin_iso VALUES (NULL);
DO $$
DECLARE curtid tid;
BEGIN
--
2.40.1
From 736811865435feaf98b2c7d60d83fa113ff2aa67 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.von...@postgresql.org>
Date: Mon, 27 Mar 2023 22:47:12 +0200
Subject: [PATCH 3/3] Show empty ranges in brin_page_items
Show which BRIN ranges are empty (no rows), as indicated by the newly
introduced flag.
---
contrib/pageinspect/brinfuncs.c | 10 ++++---
contrib/pageinspect/expected/brin.out | 6 ++--
.../pageinspect/pageinspect--1.11--1.12.sql | 17 +++++++++++
doc/src/sgml/pageinspect.sgml | 16 +++++------
...summarization-and-inprogress-insertion.out | 28 +++++++++----------
5 files changed, 48 insertions(+), 29 deletions(-)
diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c
index 000dcd8f5d8..a781f265514 100644
--- a/contrib/pageinspect/brinfuncs.c
+++ b/contrib/pageinspect/brinfuncs.c
@@ -201,8 +201,8 @@ brin_page_items(PG_FUNCTION_ARGS)
dtup = NULL;
for (;;)
{
- Datum values[7];
- bool nulls[7] = {0};
+ Datum values[8];
+ bool nulls[8] = {0};
/*
* This loop is called once for every attribute of every tuple in the
@@ -239,6 +239,7 @@ brin_page_items(PG_FUNCTION_ARGS)
nulls[4] = true;
nulls[5] = true;
nulls[6] = true;
+ nulls[7] = true;
}
else
{
@@ -261,6 +262,7 @@ brin_page_items(PG_FUNCTION_ARGS)
values[3] = BoolGetDatum(dtup->bt_columns[att].bv_allnulls);
values[4] = BoolGetDatum(dtup->bt_columns[att].bv_hasnulls);
values[5] = BoolGetDatum(dtup->bt_placeholder);
+ values[6] = BoolGetDatum(dtup->bt_empty_range);
if (!dtup->bt_columns[att].bv_allnulls)
{
BrinValues *bvalues = &dtup->bt_columns[att];
@@ -286,12 +288,12 @@ brin_page_items(PG_FUNCTION_ARGS)
}
appendStringInfoChar(&s, '}');
- values[6] = CStringGetTextDatum(s.data);
+ values[7] = CStringGetTextDatum(s.data);
pfree(s.data);
}
else
{
- nulls[6] = true;
+ nulls[7] = true;
}
}
diff --git a/contrib/pageinspect/expected/brin.out b/contrib/pageinspect/expected/brin.out
index e12fbeb4774..098ddc202f4 100644
--- a/contrib/pageinspect/expected/brin.out
+++ b/contrib/pageinspect/expected/brin.out
@@ -43,9 +43,9 @@ SELECT * FROM brin_revmap_data(get_raw_page('test1_a_idx', 1)) LIMIT 5;
SELECT * FROM brin_page_items(get_raw_page('test1_a_idx', 2), 'test1_a_idx')
ORDER BY blknum, attnum LIMIT 5;
- itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
-------------+--------+--------+----------+----------+-------------+----------
- 1 | 0 | 1 | f | f | f | {1 .. 1}
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | empty | value
+------------+--------+--------+----------+----------+-------------+-------+----------
+ 1 | 0 | 1 | f | f | f | f | {1 .. 1}
(1 row)
-- Mask DETAIL messages as these are not portable across architectures.
diff --git a/contrib/pageinspect/pageinspect--1.11--1.12.sql b/contrib/pageinspect/pageinspect--1.11--1.12.sql
index 70c3abccf57..a20d67a9e82 100644
--- a/contrib/pageinspect/pageinspect--1.11--1.12.sql
+++ b/contrib/pageinspect/pageinspect--1.11--1.12.sql
@@ -21,3 +21,20 @@ CREATE FUNCTION bt_multi_page_stats(IN relname text, IN blkno int8, IN blk_count
RETURNS SETOF record
AS 'MODULE_PATHNAME', 'bt_multi_page_stats'
LANGUAGE C STRICT PARALLEL RESTRICTED;
+
+--
+-- add information about BRIN empty ranges
+--
+DROP FUNCTION brin_page_items(IN page bytea, IN index_oid regclass);
+CREATE FUNCTION brin_page_items(IN page bytea, IN index_oid regclass,
+ OUT itemoffset int,
+ OUT blknum int8,
+ OUT attnum int,
+ OUT allnulls bool,
+ OUT hasnulls bool,
+ OUT placeholder bool,
+ OUT empty bool,
+ OUT value text)
+RETURNS SETOF record
+AS 'MODULE_PATHNAME', 'brin_page_items'
+LANGUAGE C STRICT PARALLEL RESTRICTED;
diff --git a/doc/src/sgml/pageinspect.sgml b/doc/src/sgml/pageinspect.sgml
index 01f1e96204b..e4225ecd485 100644
--- a/doc/src/sgml/pageinspect.sgml
+++ b/doc/src/sgml/pageinspect.sgml
@@ -613,14 +613,14 @@ test=# SELECT * FROM brin_revmap_data(get_raw_page('brinidx', 2)) LIMIT 5;
test=# SELECT * FROM brin_page_items(get_raw_page('brinidx', 5),
'brinidx')
ORDER BY blknum, attnum LIMIT 6;
- itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | value
-------------+--------+--------+----------+----------+-------------+--------------
- 137 | 0 | 1 | t | f | f |
- 137 | 0 | 2 | f | f | f | {1 .. 88}
- 138 | 4 | 1 | t | f | f |
- 138 | 4 | 2 | f | f | f | {89 .. 176}
- 139 | 8 | 1 | t | f | f |
- 139 | 8 | 2 | f | f | f | {177 .. 264}
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | empty | value
+------------+--------+--------+----------+----------+-------------+-------+--------------
+ 137 | 0 | 1 | t | f | f | f |
+ 137 | 0 | 2 | f | f | f | f | {1 .. 88}
+ 138 | 4 | 1 | t | f | f | f |
+ 138 | 4 | 2 | f | f | f | f | {89 .. 176}
+ 139 | 8 | 1 | t | f | f | f |
+ 139 | 8 | 2 | f | f | f | f | {177 .. 264}
</screen>
The returned columns correspond to the fields in the
<structname>BrinMemTuple</structname> and <structname>BrinValues</structname> structs.
diff --git a/src/test/modules/brin/expected/summarization-and-inprogress-insertion.out b/src/test/modules/brin/expected/summarization-and-inprogress-insertion.out
index 584ac2602f7..201786c82c0 100644
--- a/src/test/modules/brin/expected/summarization-and-inprogress-insertion.out
+++ b/src/test/modules/brin/expected/summarization-and-inprogress-insertion.out
@@ -2,9 +2,9 @@ Parsed test spec with 2 sessions
starting permutation: s2check s1b s2b s1i s2summ s1c s2c s2check
step s2check: SELECT * FROM brin_page_items(get_raw_page('brinidx', 2), 'brinidx'::regclass);
-itemoffset|blknum|attnum|allnulls|hasnulls|placeholder|value
-----------+------+------+--------+--------+-----------+--------
- 1| 0| 1|f |t |f |{1 .. 1}
+itemoffset|blknum|attnum|allnulls|hasnulls|placeholder|empty|value
+----------+------+------+--------+--------+-----------+-----+--------
+ 1| 0| 1|f |t |f |f |{1 .. 1}
(1 row)
step s1b: BEGIN ISOLATION LEVEL REPEATABLE READ;
@@ -24,18 +24,18 @@ brin_summarize_new_values
step s1c: COMMIT;
step s2c: COMMIT;
step s2check: SELECT * FROM brin_page_items(get_raw_page('brinidx', 2), 'brinidx'::regclass);
-itemoffset|blknum|attnum|allnulls|hasnulls|placeholder|value
-----------+------+------+--------+--------+-----------+-----------
- 1| 0| 1|f |t |f |{1 .. 1}
- 2| 1| 1|f |f |f |{1 .. 1000}
+itemoffset|blknum|attnum|allnulls|hasnulls|placeholder|empty|value
+----------+------+------+--------+--------+-----------+-----+-----------
+ 1| 0| 1|f |t |f |f |{1 .. 1}
+ 2| 1| 1|f |f |f |f |{1 .. 1000}
(2 rows)
starting permutation: s2check s1b s1i s2vacuum s1c s2check
step s2check: SELECT * FROM brin_page_items(get_raw_page('brinidx', 2), 'brinidx'::regclass);
-itemoffset|blknum|attnum|allnulls|hasnulls|placeholder|value
-----------+------+------+--------+--------+-----------+--------
- 1| 0| 1|f |t |f |{1 .. 1}
+itemoffset|blknum|attnum|allnulls|hasnulls|placeholder|empty|value
+----------+------+------+--------+--------+-----------+-----+--------
+ 1| 0| 1|f |t |f |f |{1 .. 1}
(1 row)
step s1b: BEGIN ISOLATION LEVEL REPEATABLE READ;
@@ -43,9 +43,9 @@ step s1i: INSERT INTO brin_iso VALUES (1000);
step s2vacuum: VACUUM brin_iso;
step s1c: COMMIT;
step s2check: SELECT * FROM brin_page_items(get_raw_page('brinidx', 2), 'brinidx'::regclass);
-itemoffset|blknum|attnum|allnulls|hasnulls|placeholder|value
-----------+------+------+--------+--------+-----------+-----------
- 1| 0| 1|f |t |f |{1 .. 1}
- 2| 1| 1|f |f |f |{1 .. 1000}
+itemoffset|blknum|attnum|allnulls|hasnulls|placeholder|empty|value
+----------+------+------+--------+--------+-----------+-----+-----------
+ 1| 0| 1|f |t |f |f |{1 .. 1}
+ 2| 1| 1|f |f |f |f |{1 .. 1000}
(2 rows)
--
2.40.1
