Protect GIST logic that assumes penalty values can't be negative.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 31 May 2011 21:54:11 +0000 (17:54 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 31 May 2011 21:54:11 +0000 (17:54 -0400)
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

doc/src/sgml/gist.sgml
src/backend/access/gist/gistutil.c

index 2418c72aa2a3c649b0b7617cf24cf88205b5407b..4442ae37a12a68f1c7926c37a10e8eec0dbb96ca 100644 (file)
        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>
      </listitem>
     </varlistentry>
index fda7c5df2ee0c721e71114b655b7c251c8d6211c..176783b5a569102f0b06a36c49afbc8b12c46d15 100644 (file)
@@ -13,6 +13,8 @@
  */
 #include "postgres.h"
 
+#include <math.h>
+
 #include "access/gist_private.h"
 #include "access/heapam.h"
 #include "access/reloptions.h"
@@ -530,16 +532,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;
 }