From 4bc6fb57f774ea18187fd8565aad9994160bfc17 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Tue, 29 May 2012 22:22:43 +0300 Subject: [PATCH] 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. --- src/backend/access/gist/gistbuild.c | 32 +++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.c index 988896d289..eb39c667c1 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++; } -- GitLab