Skip to content

Commit

Permalink
MONGOCRYPT-712 Improve double handling with precision (#869)
Browse files Browse the repository at this point in the history 
Improve handling of doubles in precision calculations to prevent loss of precision. Also tighten thresholds to thresholds for 2^53 which is the largest integer a double can hold without loss of precision.
  • Loading branch information
@markbenvenuto
markbenvenuto authored Jul 26, 2024
1 parent 87e1bb7 commit 3fd7377
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 15 deletions.
14 changes: 9 additions & 5 deletions src/mc-range-encoding.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ bool mc_getTypeInfo64(mc_getTypeInfo64_args_t args, mc_OSTType_Int64 *out, mongo
}

#define exp10Double(x) pow(10, x)
#define SCALED_DOUBLE_BOUNDS 9223372036854775807.0 // 2^63 - 1
#define SCALED_DOUBLE_BOUNDS 9007199254740992.0 // 2^53

uint64_t subtract_int64_t(int64_t max, int64_t min) {
BSON_ASSERT(max > min);
Expand Down Expand Up @@ -272,7 +272,10 @@ bool mc_canUsePrecisionModeDouble(double min,
return false;
}

if (*maxBitsOut >= 64) {
// Integers between -2^53 and 2^53 can be exactly represented. Outside this range, doubles lose precision by a
// multiple of 2^(n-52) where n = #bits. We disallow users from using precision mode when the bounds exceed 2^53 to
// prevent the users from being surprised by how floating point math works.
if (*maxBitsOut >= 53) {
return false;
}

Expand Down Expand Up @@ -351,7 +354,7 @@ bool mc_getTypeInfoDouble(mc_getTypeInfoDouble_args_t args,
}

CLIENT_ERR("The domain of double values specified by the min, max, and precision cannot be represented in "
"fewer than 64 bits. min: %g, max: %g, precision: %" PRIu32,
"fewer than 53 bits. min: %g, max: %g, precision: %" PRIu32,
args.min.value,
args.max.value,
args.precision.value);
Expand All @@ -366,8 +369,9 @@ bool mc_getTypeInfoDouble(mc_getTypeInfoDouble_args_t args,
if (use_precision_mode) {
// Take a number of xxxx.ppppp and truncate it xxxx.ppp if precision = 3.
// We do not change the digits before the decimal place.
double v_prime = trunc(args.value * exp10Double(args.precision.value)) / exp10Double(args.precision.value);
int64_t v_prime2 = (int64_t)((v_prime - args.min.value) * exp10Double(args.precision.value));
int64_t v_prime = (int64_t)(trunc(args.value * exp10Double(args.precision.value)));
int64_t scaled_min = (int64_t)(args.min.value * exp10Double(args.precision.value));
int64_t v_prime2 = v_prime - scaled_min;

BSON_ASSERT(v_prime2 < INT64_MAX && v_prime2 >= 0);

Expand Down
2 changes: 1 addition & 1 deletion test/data/range-min-cover/mincover_double_precision.cstruct
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@
.max = { .set = true, .value = 314.159265 },
.precision = { .set = true, .value = 6 },
.expectMincoverStrings = {
"0000010011100111000101111011",
"00000100111001110001011110111",
"00000100111001110001011111",
"000001001110011100011",
"0000010011100111001",
Expand Down
65 changes: 56 additions & 9 deletions test/test-mc-range-encoding.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,12 +224,15 @@ static void _test_canUsePrecisionModeDouble(_mongocrypt_tester_t *tester) {
CAN_USE_PRECISION_MODE(0.0, 16.0, 0, true, 5);
// 2^53 + 1 is where double starts to lose precision, so we need to ensure that we get the
// correct value for max_bits out.
CAN_USE_PRECISION_MODE(1.0, 9007199254740992.0, 0, true, 53);
CAN_USE_PRECISION_MODE(0.0, 9007199254740992.0, 0, true, 54);
CAN_USE_PRECISION_MODE_ERRORS(1.0, 9007199254740992.0, 0, "Invalid upper bound for double precision. Absolute");
CAN_USE_PRECISION_MODE_ERRORS(0.0, 9007199254740992.0, 0, "Invalid upper bound for double precision. Absolute");

CAN_USE_PRECISION_MODE(2.718281, 314.159265, 6, true, 29);

CAN_USE_PRECISION_MODE(-1000000000.0, 9223372036844775424.0, 0, false, 64);
CAN_USE_PRECISION_MODE_ERRORS(-1000000000.0,
9223372036844775424.0,
0,
"Invalid upper bound for double precision. Absolute");

CAN_USE_PRECISION_MODE_ERRORS(2.710000,
314.150000,
Expand All @@ -242,10 +245,13 @@ static void _test_canUsePrecisionModeDouble(_mongocrypt_tester_t *tester) {
0,
"Invalid upper bound for double precision. Absolute scaled value");
CAN_USE_PRECISION_MODE_ERRORS(-1 * INT_64_MAX_DOUBLE,
-1 * (double)9007199254740992,
1.0,
0,
"Invalid lower bound for double precision. Absolute scaled value");
CAN_USE_PRECISION_MODE_ERRORS(-92233720368547.0, 92233720368547.0, 5, "Invalid value for precision.");
CAN_USE_PRECISION_MODE_ERRORS(-92233720368547.0,
92233720368547.0,
5,
"Invalid upper bound for double precision. Absolute");

#undef CAN_USE_PRECISION_MODE
#undef CAN_USE_PRECISION_MODE_ERRORS
Expand All @@ -262,6 +268,10 @@ typedef struct {
bool use_range_v1; // By default, use range v2.
} DoubleTest;

// Smallest and Largest integer that fits in a double before precision is lost
#define DOUBLE_MIN_SAFE_INT -9007199254740992 // -2^53
#define DOUBLE_MAX_SAFE_INT 9007199254740992 // 2^53

static void _test_RangeTest_Encode_Double(_mongocrypt_tester_t *tester) {
DoubleTest tests[] = {/* Test cases copied from server Double_Bounds test ... begin */
// Larger numbers map to larger uint64
Expand Down Expand Up @@ -414,8 +424,7 @@ static void _test_RangeTest_Encode_Double(_mongocrypt_tester_t *tester) {
.max = OPT_DOUBLE_C(5),
.min = OPT_DOUBLE_C(0),
.precision = OPT_U32_C(16),
.expect = 31415926535890000,
.expectMax = OPT_U64_C(72057594037927935)},
.expectError = "Invalid upper bound for double precision"},
{.value = -5,
.max = OPT_DOUBLE_C(-1),
.min = OPT_DOUBLE_C(-10),
Expand Down Expand Up @@ -464,16 +473,54 @@ static void _test_RangeTest_Encode_Double(_mongocrypt_tester_t *tester) {
.max = OPT_DOUBLE_C(10E-30),
.min = OPT_DOUBLE_C(1E-30),
.precision = OPT_U32_C(35),
// Applying min/max/precision result in a domain needing >= 64 bits to represent.
// Applying min/max/precision result in a domain needing >= 53 bits to represent.
// For range v2, expect an error.
.expectError = "Invalid upper bound for double precision."},
{.value = 1E-30,
.max = OPT_DOUBLE_C(10E-30),
.min = OPT_DOUBLE_C(1E-30),
.precision = OPT_U32_C(35),
// Applying min/max/precision result in a domain needing >= 64 bits to represent.
// Applying min/max/precision result in a domain needing >= 53 bits to represent.
// For range v2, expect an error.
.expectError = "Invalid upper bound for double precision."},
/* Test max and min integer bounds for doubles */
{.value = DOUBLE_MIN_SAFE_INT,
.max = OPT_DOUBLE_C(DOUBLE_MAX_SAFE_INT),
.min = OPT_DOUBLE_C(DOUBLE_MIN_SAFE_INT),
.precision = OPT_U32_C(0),
.expect = 0,
// Applying min/max/precision result in a domain needing >= 53 bits to represent.
// For range v2, expect an error.
.expectError = "Invalid upper bound for double precision. Absolute"},
{.value = 900719925474099.6,
.max = OPT_DOUBLE_C(900719925474100.0),
.min = OPT_DOUBLE_C(900719925474099.0),
.precision = OPT_U32_C(0),
.expect = 0,
.expectMax = OPT_U64_C(1)},
// Domain size is small but min/max * 10^precision loses precision.
{.value = 900719925474099.6,
.max = OPT_DOUBLE_C(900719925474100.0),
.min = OPT_DOUBLE_C(900719925474099.0),
.precision = OPT_U32_C(1),
.expectError = "Invalid upper bound for double precision. Absolute"},
{.value = -900719925474099.6,
.max = OPT_DOUBLE_C(-900719925474099.0),
.min = OPT_DOUBLE_C(-900719925474100.0),
.precision = OPT_U32_C(1),
.expectError = "Invalid lower bound for double precision. Absolute"},
// 2^52
// The expected values increase by 4503599627370496 * 2^(i-52) + j
// i.e. the gaps between integers as the exponent increases since doubles lose precision as
// the exponent increases
{.value = 0,
.max = OPT_DOUBLE_C(4503599627370496),
.min = OPT_DOUBLE_C(-4503599627370496),
.precision = OPT_U32_C(0),
.expect = 0,
// Applying min/max/precision result in a domain needing >= 53 bits to represent.
// For range v2, expect an error.
.expectError = "The domain of double values specified by the min"},
/* Test cases copied from Double_Bounds_Precision ... end */
{.value = -1,
.min = OPT_DOUBLE_C(0),
Expand Down

0 comments on commit 3fd7377

Please sign in to comment.