summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorTomas Vondra <tomas.vondra@postgresql.org>2023-05-18 13:00:31 +0200
committerTomas Vondra <tomas.vondra@postgresql.org>2023-05-18 23:34:56 +0200
commitd42ffda685f5dbd123df59c2f8050e8fb09164c7 (patch)
tree1c370f4422126bc24895bec2810fe91ecabf87f5 /src
parent0966291a4ade47f7c14313ab7868d1de4e4f6688 (diff)
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
Diffstat (limited to 'src')
-rw-r--r--src/backend/access/brin/brin_inclusion.c10
-rw-r--r--src/backend/access/brin/brin_minmax.c10
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 9d7cb36a47a..75195ce23bd 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 ad0d18ed39f..57c9294f269 100644
--- a/src/backend/access/brin/brin_minmax.c
+++ b/src/backend/access/brin/brin_minmax.c
@@ -249,10 +249,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 */
@@ -267,10 +270,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],