Protect GIST logic that assumes penalty values can't be negative.

Apparently sane-looking penalty code might return small negative values,
for example because of roundoff error.  This will confuse places like
gistchoose().  Prevent problems by clamping negative penalty values to
zero.  (Just to be really sure, I also made it force NaNs to zero.)
Back-patch to all supported branches.

Alexander Korotkov

diff --git a/doc/src/sgml/gist.sgml b/doc/src/sgml/gist.sgml
index eddaaad5dfa39f34c26a5aee1a2302f6113ce1e7..fd5800f336c27ac6d2417dd6efc75c1e437adea5 100644 (file)
--- a/doc/src/sgml/gist.sgml
+++ b/doc/src/sgml/gist.sgml
@@ -374,6 +374,8 @@ my_decompress(PG_FUNCTION_ARGS)
        Returns a value indicating the <quote>cost</quote> of inserting the new
        entry into a particular branch of the tree.  Items will be inserted
        down the path of least <function>penalty</function> in the tree.
+       Values returned by <function>penalty</function> should be non-negative.
+       If a negative value is returned, it will be treated as zero.
       </para>
 
       <para>

diff --git a/src/backend/access/gist/gistutil.c b/src/backend/access/gist/gistutil.c
index a2e8fe0cfb9f1b61ee94d9e2c7b3d3ae6a565aac..2e70b5e5c7df132815f438f73098492e60f49031 100644 (file)
--- a/src/backend/access/gist/gistutil.c
+++ b/src/backend/access/gist/gistutil.c
@@ -13,6 +13,8 @@
  */
 #include "postgres.h"
 
+#include <math.h>
+
 #include "access/gist_private.h"
 #include "access/reloptions.h"
 #include "storage/freespace.h"
@@ -532,16 +534,22 @@ gistpenalty(GISTSTATE *giststate, int attno,
 {
    float       penalty = 0.0;
 
-   if (giststate->penaltyFn[attno].fn_strict == FALSE || (isNullOrig == FALSE && isNullAdd == FALSE))
+   if (giststate->penaltyFn[attno].fn_strict == FALSE ||
+       (isNullOrig == FALSE && isNullAdd == FALSE))
+   {
        FunctionCall3(&giststate->penaltyFn[attno],
                      PointerGetDatum(orig),
                      PointerGetDatum(add),
                      PointerGetDatum(&penalty));
+       /* disallow negative or NaN penalty */
+       if (isnan(penalty) || penalty < 0.0)
+           penalty = 0.0;
+   }
    else if (isNullOrig && isNullAdd)
        penalty = 0.0;
    else
-       penalty = 1e10;         /* try to prevent to mix null and non-null
-                                * value */
+       penalty = 1e10;         /* try to prevent mixing null and non-null
+                                * values */
 
    return penalty;
 }