aboutsummaryrefslogtreecommitdiff
path: root/src/backend/access/gist/gistbuild.c
diff options
context:
space:
mode:
authorHeikki Linnakangas <heikki.linnakangas@iki.fi>2012-05-29 22:22:43 +0300
committerHeikki Linnakangas <heikki.linnakangas@iki.fi>2012-05-29 22:27:42 +0300
commit4bc6fb57f774ea18187fd8565aad9994160bfc17 (patch)
tree7559f20bd7f1e97efe8b6d9a329a2ccf81b79cc0 /src/backend/access/gist/gistbuild.c
parent2755abf386e6572bad15cb6a032e504ad32308cc (diff)
downloadpostgresql-4bc6fb57f774ea18187fd8565aad9994160bfc17.tar.gz
postgresql-4bc6fb57f774ea18187fd8565aad9994160bfc17.zip
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.
Diffstat (limited to 'src/backend/access/gist/gistbuild.c')
-rw-r--r--src/backend/access/gist/gistbuild.c32
1 files changed, 23 insertions, 9 deletions
diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.c
index 988896d289f..eb39c667c15 100644
--- 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++;
}