From 3f1356e5d60a310b9ceb73497ec49af1b69bafa8 Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Thu, 18 May 2023 13:00:31 +0200 Subject: [PATCH] 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/9d993d0d-e431-2196-9ccc-0554d0e60154%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 de7e96c70e61d..2614ba9f47f6a 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -1623,8 +1623,11 @@ union_tuples(BrinDesc *bdesc, BrinMemTuple *a, BrinTuple *b) 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. */ @@ -1636,12 +1639,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] =