Fix integer overflow bug in GiST buffering build calculations.

The result of (maintenance_work_mem * 1024) / BLCKSZ doesn't fit in a signed
32-bit integer, if maintenance_work_mem >= 2GB. Use double instead. And
while we're at it, write the calculations in an easier to understand form,
with the intermediary steps written out and commented.

diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.c
index 988896d289f5a727db81c97e01edab947b11e1ae..eb39c667c15f51c5ebe276ff0476b00dea9af813 100644 (file)
--- a/src/backend/access/gist/gistbuild.c
+++ b/src/backend/access/gist/gistbuild.c
@@ -323,8 +323,8 @@ gistInitBuffering(GISTBuildState *buildstate)
         * calculating levelStep is very close to Arge et al's formula. For a
         * large B, our formula gives a value that is 2x higher.
         *
-        * The average size of a subtree of depth n can be calculated as a
-        * geometric series:
+        * The average size (in pages) of a subtree of depth n can be calculated
+        * as a geometric series:
         *
         * B^0 + B^1 + B^2 + ... + B^n = (1 - B^(n + 1)) / (1 - B)
         *
@@ -353,14 +353,28 @@ gistInitBuffering(GISTBuildState *buildstate)
         * the hash table.
         */
        levelStep = 1;
-       while (
-       /* subtree must fit in cache (with safety factor of 4) */
-                  (1 - pow(avgIndexTuplesPerPage, (double) (levelStep + 1))) / (1 - avgIndexTuplesPerPage) < effective_cache_size / 4
-                  &&
-       /* each node in the lowest level of a subtree has one page in memory */
-                  (pow(maxIndexTuplesPerPage, (double) levelStep) < (maintenance_work_mem * 1024) / BLCKSZ)
-               )
+       for (;;)
        {
+               double          subtreesize;
+               double          maxlowestlevelpages;
+
+               /* size of an average subtree at this levelStep (in pages). */
+               subtreesize =
+                       (1 - pow(avgIndexTuplesPerPage, (double) (levelStep + 1))) /
+                       (1 - avgIndexTuplesPerPage);
+
+               /* max number of pages at the lowest level of a subtree */
+               maxlowestlevelpages = pow(maxIndexTuplesPerPage, (double) levelStep);
+
+               /* subtree must fit in cache (with safety factor of 4) */
+               if (subtreesize > effective_cache_size / 4)
+                       break;
+
+               /* each node in the lowest level of a subtree has one page in memory */
+               if (maxlowestlevelpages > ((double) maintenance_work_mem * 1024) / BLCKSZ)
+                       break;
+
+               /* Good, we can handle this levelStep. See if we can go one higher. */
                levelStep++;
        }