]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Fix handling of NULLs when merging BRIN summaries
authorTomas Vondra <tomas.vondra@postgresql.org>
Thu, 18 May 2023 11:00:31 +0000 (13:00 +0200)
committerTomas Vondra <tomas.vondra@postgresql.org>
Thu, 18 May 2023 21:35:16 +0000 (23:35 +0200)
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_inclusion.c
src/backend/access/brin/brin_minmax.c

index e063f40b371a5e7c960f3beee6a2621d30c5c791..adb2439df7535f59be00ddbf067bff57ac468fd7 100644 (file)
@@ -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);
index 0f6aa33a45bdf8410f029f2c7e70b665846e74c8..8fb25fc49473f67b7016584ab503e13475afaf3d 100644 (file)
@@ -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],