From d91353f4b21f10417d142e6ac17a0490adae867c Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Sat, 6 Nov 2021 01:25:31 +0100 Subject: [PATCH] Fix handling of NaN values in BRIN minmax multi When calculating distance between float4/float8 values, we need to be a bit more careful about NaN values in order not to trigger assert. We consider NaN values to be equal (distace 0.0) and in infinite distance from all other values. On builds without asserts, this issue is mostly harmless - the ranges may be merged in less efficient order, but the index is still correct. Per report from Andreas Seltenreich. Backpatch to 14, where this new BRIN opclass was introduced. Reported-by: Andreas Seltenreich Discussion: https://postgr.es/m/87r1bw9ukm.fsf@credativ.de --- src/backend/access/brin/brin_minmax_multi.c | 17 +++++++++++++++++ src/test/regress/expected/brin_multi.out | 3 +++ src/test/regress/sql/brin_multi.sql | 4 ++++ 3 files changed, 24 insertions(+) diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c index ee6c4795f1..a85dfdfec4 100644 --- a/src/backend/access/brin/brin_minmax_multi.c +++ b/src/backend/access/brin/brin_minmax_multi.c @@ -73,6 +73,7 @@ #include "utils/builtins.h" #include "utils/date.h" #include "utils/datum.h" +#include "utils/float.h" #include "utils/inet.h" #include "utils/lsyscache.h" #include "utils/memutils.h" @@ -1872,6 +1873,14 @@ brin_minmax_multi_distance_float4(PG_FUNCTION_ARGS) float a1 = PG_GETARG_FLOAT4(0); float a2 = PG_GETARG_FLOAT4(1); + /* if both values are NaN, then we consider them the same */ + if (isnan(a1) && isnan(a2)) + PG_RETURN_FLOAT8(0.0); + + /* if one value is NaN, use infinite distance */ + if (isnan(a1) || isnan(a2)) + PG_RETURN_FLOAT8(get_float8_infinity()); + /* * We know the values are range boundaries, but the range may be collapsed * (i.e. single points), with equal values. @@ -1890,6 +1899,14 @@ brin_minmax_multi_distance_float8(PG_FUNCTION_ARGS) double a1 = PG_GETARG_FLOAT8(0); double a2 = PG_GETARG_FLOAT8(1); + /* if both values are NaN, then we consider them the same */ + if (isnan(a1) && isnan(a2)) + PG_RETURN_FLOAT8(0.0); + + /* if one value is NaN, use infinite distance */ + if (isnan(a1) || isnan(a2)) + PG_RETURN_FLOAT8(get_float8_infinity()); + /* * We know the values are range boundaries, but the range may be collapsed * (i.e. single points), with equal values. diff --git a/src/test/regress/expected/brin_multi.out b/src/test/regress/expected/brin_multi.out index d3d693e464..f3309f433f 100644 --- a/src/test/regress/expected/brin_multi.out +++ b/src/test/regress/expected/brin_multi.out @@ -347,6 +347,9 @@ SELECT brin_desummarize_range('brinidx_multi', 0); (1 row) VACUUM brintest_multi; -- force a summarization cycle in brinidx +-- Try inserting a values with NaN, to test distance calculation. +insert into public.brintest_multi (float4col) values (real 'nan'); +insert into public.brintest_multi (float8col) values (real 'nan'); UPDATE brintest_multi SET int8col = int8col * int4col; -- Tests for brin_summarize_new_values SELECT brin_summarize_new_values('brintest_multi'); -- error, not an index diff --git a/src/test/regress/sql/brin_multi.sql b/src/test/regress/sql/brin_multi.sql index 70b751068b..2189b6ccf4 100644 --- a/src/test/regress/sql/brin_multi.sql +++ b/src/test/regress/sql/brin_multi.sql @@ -351,6 +351,10 @@ FROM tenk1 ORDER BY unique2 LIMIT 5 OFFSET 5; SELECT brin_desummarize_range('brinidx_multi', 0); VACUUM brintest_multi; -- force a summarization cycle in brinidx +-- Try inserting a values with NaN, to test distance calculation. +insert into public.brintest_multi (float4col) values (real 'nan'); +insert into public.brintest_multi (float8col) values (real 'nan'); + UPDATE brintest_multi SET int8col = int8col * int4col; -- Tests for brin_summarize_new_values