From 57e53d34f2f7bba91fcc0de6f4eff551669554fb Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Sat, 22 Oct 2022 02:26:48 +0200 Subject: [PATCH 2/9] fixup: brin has_nulls 2 --- src/backend/access/brin/brin.c | 22 +++++++++++++-------- src/backend/access/brin/brin_bloom.c | 10 +++++++++- src/backend/access/brin/brin_inclusion.c | 10 +++++++++- src/backend/access/brin/brin_minmax.c | 10 +++++++++- src/backend/access/brin/brin_minmax_multi.c | 10 +++++++++- src/backend/access/brin/brin_tuple.c | 2 +- 6 files changed, 51 insertions(+), 13 deletions(-) diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index 20b7d65b948..6fabd14c263 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -56,6 +56,7 @@ typedef struct BrinBuildState BrinRevmap *bs_rmAccess; BrinDesc *bs_bdesc; BrinMemTuple *bs_dtuple; + bool bs_modified; } BrinBuildState; /* @@ -793,6 +794,7 @@ brinbuildCallback(Relation index, /* set state to correspond to the next range */ state->bs_currRangeStart += state->bs_pagesPerRange; + state->bs_modified = false; /* re-initialize state for it */ brin_memtuple_initialize(state->bs_dtuple, state->bs_bdesc); @@ -801,6 +803,7 @@ brinbuildCallback(Relation index, /* Accumulate the current tuple into the running state */ (void) add_values_to_range(index, state->bs_bdesc, state->bs_dtuple, values, isnull); + state->bs_modified = true; } /* @@ -1287,6 +1290,7 @@ initialize_brin_buildstate(Relation idxRel, BrinRevmap *revmap, state->bs_rmAccess = revmap; state->bs_bdesc = brin_build_desc(idxRel); state->bs_dtuple = brin_new_memtuple(state->bs_bdesc); + state->bs_modified = false; return state; } @@ -1569,14 +1573,16 @@ form_and_insert_tuple(BrinBuildState *state) BrinTuple *tup; Size size; - 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); + if (state->bs_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); + } } /* diff --git a/src/backend/access/brin/brin_bloom.c b/src/backend/access/brin/brin_bloom.c index 60315450b41..96e5961a408 100644 --- a/src/backend/access/brin/brin_bloom.c +++ b/src/backend/access/brin/brin_bloom.c @@ -539,7 +539,15 @@ brin_bloom_add_value(PG_FUNCTION_ARGS) BloomGetFalsePositiveRate(opts)); column->bv_values[0] = PointerGetDatum(filter); column->bv_allnulls = false; - column->bv_hasnulls = true; + + /* + * When both bv_allnulls and bv_hasnulls are set to true, it means this + * summary was representing no rows. So we just set bv_hasnulls=false. + * Otherwise we need to set it to true, because there already were some + * NULL values, apparently. + */ + column->bv_hasnulls = !column->bv_hasnulls; + updated = true; } else diff --git a/src/backend/access/brin/brin_inclusion.c b/src/backend/access/brin/brin_inclusion.c index e0f44d3e62c..dd8fe379c7b 100644 --- a/src/backend/access/brin/brin_inclusion.c +++ b/src/backend/access/brin/brin_inclusion.c @@ -164,7 +164,15 @@ 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; + + /* + * When both bv_allnulls and bv_hasnulls are set to true, it means this + * summary was representing no rows. So we just set bv_hasnulls=false. + * Otherwise we need to set it to true, because there already were some + * NULL values, apparently. + */ + column->bv_hasnulls = !column->bv_hasnulls; + new = true; } diff --git a/src/backend/access/brin/brin_minmax.c b/src/backend/access/brin/brin_minmax.c index 8a5661a8952..ead9e8f4e36 100644 --- a/src/backend/access/brin/brin_minmax.c +++ b/src/backend/access/brin/brin_minmax.c @@ -90,7 +90,15 @@ 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; + + /* + * When both bv_allnulls and bv_hasnulls are set to true, it means this + * summary was representing no rows. So we just set bv_hasnulls=false. + * Otherwise we need to set it to true, because there already were some + * NULL values, apparently. + */ + column->bv_hasnulls = !column->bv_hasnulls; + PG_RETURN_BOOL(true); } diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c index 4e7119e2d78..410bfdcfa79 100644 --- a/src/backend/access/brin/brin_minmax_multi.c +++ b/src/backend/access/brin/brin_minmax_multi.c @@ -2500,7 +2500,15 @@ brin_minmax_multi_add_value(PG_FUNCTION_ARGS) MemoryContextSwitchTo(oldctx); column->bv_allnulls = false; - column->bv_hasnulls = true; + + /* + * When both bv_allnulls and bv_hasnulls are set to true, it means this + * summary was representing no rows. So we just set bv_hasnulls=false. + * Otherwise we need to set it to true, because there already were some + * NULL values, apparently. + */ + column->bv_hasnulls = !column->bv_hasnulls; + 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..4d2a45ddcb6 100644 --- a/src/backend/access/brin/brin_tuple.c +++ b/src/backend/access/brin/brin_tuple.c @@ -517,7 +517,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); -- 2.37.3