From d9476b83808a39d9985845071bf0a150a3063b37 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 25 Jul 2015 19:42:32 -0400 Subject: [PATCH] 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.) --- src/backend/access/tablesample/bernoulli.c | 4 +++- src/backend/access/tablesample/system.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/backend/access/tablesample/bernoulli.c b/src/backend/access/tablesample/bernoulli.c index cf88f95e75..ccef4f7f84 100644 --- 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 43c5dab716..080a312114 100644 --- 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; -- 2.40.0