Dodge portability issue (apparent compiler bug) in new tablesample code.

Some of the older OS X critters in the buildfarm are failing regression,
with symptoms showing that a request for 100% sampling in BERNOULLI or
SYSTEM methods actually gets only around 50% of the table.  gdb revealed
that the computation of the "cutoff" number was producing 0x7FFFFFFF
rather than the expected 0x100000000.  Inspecting the assembly code,
it looks like gcc is trying to use lrint() instead of rint() and then
fumbling the conversion from long double to uint64.  This seems like a
clear compiler bug, but assigning the intermediate result into a plain
double variable works around it, so let's just do that.  (Another idea
would be to give up one bit of hash width so that we don't need to use
a uint64 cutoff, but let's see if this is enough.)

diff --git a/src/backend/access/tablesample/bernoulli.c b/src/backend/access/tablesample/bernoulli.c
index cf88f95e757b1754da8b4d074c9abfc367560208..ccef4f7f8438828bf6b3de8b6097ca5695335d91 100644 (file)
--- a/src/backend/access/tablesample/bernoulli.c
+++ b/src/backend/access/tablesample/bernoulli.c
@@ -144,6 +144,7 @@ bernoulli_beginsamplescan(SampleScanState *node,
 {
        BernoulliSamplerData *sampler = (BernoulliSamplerData *) node->tsm_state;
        double          percent = DatumGetFloat4(params[0]);
+       double          dcutoff;
 
        if (percent < 0 || percent > 100 || isnan(percent))
                ereport(ERROR,
@@ -155,7 +156,8 @@ bernoulli_beginsamplescan(SampleScanState *node,
         * store that as a uint64, of course.  Note that this gives strictly
         * correct behavior at the limits of zero or one probability.
         */
-       sampler->cutoff = rint(((double) PG_UINT32_MAX + 1) * percent / 100);
+       dcutoff = rint(((double) PG_UINT32_MAX + 1) * percent / 100);
+       sampler->cutoff = (uint64) dcutoff;
        sampler->seed = seed;
        sampler->lt = InvalidOffsetNumber;
 

diff --git a/src/backend/access/tablesample/system.c b/src/backend/access/tablesample/system.c
index 43c5dab71619a7a6d8e2ee22bc306e56674191c0..080a3121141e05f7078ce072aa661d72510bba7b 100644 (file)
--- a/src/backend/access/tablesample/system.c
+++ b/src/backend/access/tablesample/system.c
@@ -148,6 +148,7 @@ system_beginsamplescan(SampleScanState *node,
 {
        SystemSamplerData *sampler = (SystemSamplerData *) node->tsm_state;
        double          percent = DatumGetFloat4(params[0]);
+       double          dcutoff;
 
        if (percent < 0 || percent > 100 || isnan(percent))
                ereport(ERROR,
@@ -159,7 +160,8 @@ system_beginsamplescan(SampleScanState *node,
         * store that as a uint64, of course.  Note that this gives strictly
         * correct behavior at the limits of zero or one probability.
         */
-       sampler->cutoff = rint(((double) PG_UINT32_MAX + 1) * percent / 100);
+       dcutoff = rint(((double) PG_UINT32_MAX + 1) * percent / 100);
+       sampler->cutoff = (uint64) dcutoff;
        sampler->seed = seed;
        sampler->nextblock = 0;
        sampler->lt = InvalidOffsetNumber;