MONGOCRYPT-698 Make trimFactor and sparsity optional (#867)

Supply a constant default sparsity and calculate a default trimFactor if not provided by the user.
mongodb · Jul 19, 2024 · 14ccd9c · 14ccd9c
1 parent ea59418
commit 14ccd9c
Show file tree

Hide file tree

Showing 14 changed files with 154 additions and 84 deletions.
diff --git a/src/mc-range-edge-generation-private.h b/src/mc-range-edge-generation-private.h
@@ -45,32 +45,32 @@ typedef struct {
     mc_optional_int32_t min;
     mc_optional_int32_t max;
     size_t sparsity;
-    uint32_t trimFactor;
+    mc_optional_uint32_t trimFactor;
 } mc_getEdgesInt32_args_t;
 
 // mc_getEdgesInt32 implements the Edge Generation algorithm described in
 // SERVER-67751 for int32_t.
-mc_edges_t *mc_getEdgesInt32(mc_getEdgesInt32_args_t args, mongocrypt_status_t *status);
+mc_edges_t *mc_getEdgesInt32(mc_getEdgesInt32_args_t args, mongocrypt_status_t *status, bool use_range_v2);
 
 typedef struct {
     int64_t value;
     mc_optional_int64_t min;
     mc_optional_int64_t max;
     size_t sparsity;
-    uint32_t trimFactor;
+    mc_optional_uint32_t trimFactor;
 } mc_getEdgesInt64_args_t;
 
 // mc_getEdgesInt64 implements the Edge Generation algorithm described in
 // SERVER-67751 for int64_t.
-mc_edges_t *mc_getEdgesInt64(mc_getEdgesInt64_args_t args, mongocrypt_status_t *status);
+mc_edges_t *mc_getEdgesInt64(mc_getEdgesInt64_args_t args, mongocrypt_status_t *status, bool use_range_v2);
 
 typedef struct {
     double value;
     size_t sparsity;
     mc_optional_double_t min;
     mc_optional_double_t max;
     mc_optional_uint32_t precision;
-    uint32_t trimFactor;
+    mc_optional_uint32_t trimFactor;
 } mc_getEdgesDouble_args_t;
 
 // mc_getEdgesDouble implements the Edge Generation algorithm described in
@@ -83,7 +83,7 @@ typedef struct {
     size_t sparsity;
     mc_optional_dec128_t min, max;
     mc_optional_uint32_t precision;
-    uint32_t trimFactor;
+    mc_optional_uint32_t trimFactor;
 } mc_getEdgesDecimal128_args_t;
 
 mc_edges_t *mc_getEdgesDecimal128(mc_getEdgesDecimal128_args_t args, mongocrypt_status_t *status, bool use_range_v2);

diff --git a/src/mc-range-edge-generation.c b/src/mc-range-edge-generation.c
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#include "mc-optional-private.h"
 #include "mc-range-edge-generation-private.h"
 
 #include "mc-array-private.h"
@@ -28,14 +29,19 @@ struct _mc_edges_t {
     char *leaf;
 };
 
-static mc_edges_t *mc_edges_new(const char *leaf, size_t sparsity, uint32_t trimFactor, mongocrypt_status_t *status) {
+static mc_edges_t *mc_edges_new(const char *leaf,
+                                size_t sparsity,
+                                mc_optional_uint32_t opt_trimFactor,
+                                mongocrypt_status_t *status,
+                                bool use_range_v2) {
     BSON_ASSERT_PARAM(leaf);
     if (sparsity < 1) {
         CLIENT_ERR("sparsity must be 1 or larger");
         return NULL;
     }
 
     const size_t leaf_len = strlen(leaf);
+    const uint32_t trimFactor = trimFactorDefault(leaf_len, opt_trimFactor, use_range_v2);
     if (trimFactor != 0 && trimFactor >= leaf_len) {
         // We append a total of leaf_len + 1 (for the root) - trimFactor edges. When this number is equal to 1, we
         // degenerate into equality, which is not desired, so trimFactor must be less than leaf_len.
@@ -142,7 +148,7 @@ mc_bitstring mc_convert_to_bitstring_u128(mlib_int128 i) {
     return ret;
 }
 
-mc_edges_t *mc_getEdgesInt32(mc_getEdgesInt32_args_t args, mongocrypt_status_t *status) {
+mc_edges_t *mc_getEdgesInt32(mc_getEdgesInt32_args_t args, mongocrypt_status_t *status, bool use_range_v2) {
     mc_OSTType_Int32 got;
     if (!mc_getTypeInfo32((mc_getTypeInfo32_args_t){.value = args.value, .min = args.min, .max = args.max},
                           &got,
@@ -158,11 +164,11 @@ mc_edges_t *mc_getEdgesInt32(mc_getEdgesInt32_args_t args, mongocrypt_status_t *
     mc_bitstring valueBin = mc_convert_to_bitstring_u32(got.value);
     size_t offset = mc_count_leading_zeros_u32(got.max);
     const char *leaf = valueBin.str + offset;
-    mc_edges_t *ret = mc_edges_new(leaf, args.sparsity, args.trimFactor, status);
+    mc_edges_t *ret = mc_edges_new(leaf, args.sparsity, args.trimFactor, status, use_range_v2);
     return ret;
 }
 
-mc_edges_t *mc_getEdgesInt64(mc_getEdgesInt64_args_t args, mongocrypt_status_t *status) {
+mc_edges_t *mc_getEdgesInt64(mc_getEdgesInt64_args_t args, mongocrypt_status_t *status, bool use_range_v2) {
     mc_OSTType_Int64 got;
     if (!mc_getTypeInfo64((mc_getTypeInfo64_args_t){.value = args.value, .min = args.min, .max = args.max},
                           &got,
@@ -178,7 +184,7 @@ mc_edges_t *mc_getEdgesInt64(mc_getEdgesInt64_args_t args, mongocrypt_status_t *
     mc_bitstring valueBin = mc_convert_to_bitstring_u64(got.value);
     size_t offset = mc_count_leading_zeros_u64(got.max);
     const char *leaf = valueBin.str + offset;
-    mc_edges_t *ret = mc_edges_new(leaf, args.sparsity, args.trimFactor, status);
+    mc_edges_t *ret = mc_edges_new(leaf, args.sparsity, args.trimFactor, status, use_range_v2);
     return ret;
 }
 
@@ -202,7 +208,7 @@ mc_edges_t *mc_getEdgesDouble(mc_getEdgesDouble_args_t args, mongocrypt_status_t
     mc_bitstring valueBin = mc_convert_to_bitstring_u64(got.value);
     size_t offset = mc_count_leading_zeros_u64(got.max);
     const char *leaf = valueBin.str + offset;
-    mc_edges_t *ret = mc_edges_new(leaf, args.sparsity, args.trimFactor, status);
+    mc_edges_t *ret = mc_edges_new(leaf, args.sparsity, args.trimFactor, status, use_range_v2);
     return ret;
 }
 
@@ -227,7 +233,7 @@ mc_edges_t *mc_getEdgesDecimal128(mc_getEdgesDecimal128_args_t args, mongocrypt_
     mc_bitstring bits = mc_convert_to_bitstring_u128(got.value);
     size_t offset = mc_count_leading_zeros_u128(got.max);
     const char *leaf = bits.str + offset;
-    mc_edges_t *ret = mc_edges_new(leaf, args.sparsity, args.trimFactor, status);
+    mc_edges_t *ret = mc_edges_new(leaf, args.sparsity, args.trimFactor, status, use_range_v2);
     return ret;
 }
 #endif // MONGOCRYPT_HAVE_DECIMAL128_SUPPORT
diff --git a/src/mc-range-encoding-private.h b/src/mc-range-encoding-private.h
@@ -127,4 +127,7 @@ bool mc_getTypeInfoDecimal128(mc_getTypeInfoDecimal128_args_t args,
                               bool use_range_v2) MONGOCRYPT_WARN_UNUSED_RESULT;
 #endif // MONGOCRYPT_HAVE_DECIMAL128_SUPPORT
 
+extern const int64_t mc_FLERangeSparsityDefault;
+
+uint32_t trimFactorDefault(size_t maxlen, mc_optional_uint32_t trimFactor, bool use_range_v2);
 #endif /* MC_RANGE_ENCODING_PRIVATE_H */
diff --git a/src/mc-range-encoding.c b/src/mc-range-encoding.c
@@ -616,3 +616,23 @@ bool mc_getTypeInfoDecimal128(mc_getTypeInfoDecimal128_args_t args,
 }
 
 #endif // defined MONGOCRYPT_HAVE_DECIMAL128_SUPPORT
+
+const int64_t mc_FLERangeSparsityDefault = 2;
+const uint32_t mc_FLERangeTrimFactorDefault = 6;
+
+uint32_t trimFactorDefault(size_t maxlen, mc_optional_uint32_t trimFactor, bool use_range_v2) {
+    if (trimFactor.set) {
+        return trimFactor.value;
+    }
+
+    if (!use_range_v2) {
+        // Preserve old default.
+        return 0;
+    }
+
+    if (mc_FLERangeTrimFactorDefault > maxlen - 1) {
+        return (uint32_t)maxlen - 1;
+    } else {
+        return mc_FLERangeTrimFactorDefault;
+    }
+}
diff --git a/src/mc-range-mincover-generator.template.h b/src/mc-range-mincover-generator.template.h
@@ -99,8 +99,9 @@ static inline DECORATE_NAME(MinCoverGenerator)
                                            UINT_T rangeMax,
                                            UINT_T max,
                                            size_t sparsity,
-                                           uint32_t trimFactor,
-                                           mongocrypt_status_t *status) {
+                                           mc_optional_uint32_t opt_trimFactor,
+                                           mongocrypt_status_t *status,
+                                           bool use_range_v2) {
     BSON_ASSERT_PARAM(status);
 
     if (UINT_COMPARE(rangeMin, rangeMax) > 0) {
@@ -122,6 +123,7 @@ static inline DECORATE_NAME(MinCoverGenerator)
         return NULL;
     }
     size_t maxlen = (size_t)BITS - DECORATE_NAME(mc_count_leading_zeros)(max);
+    uint32_t trimFactor = trimFactorDefault(maxlen, opt_trimFactor, use_range_v2);
     if (trimFactor != 0 && trimFactor >= maxlen) {
         CLIENT_ERR("Trim factor must be less than the number of bits (%zu) used to represent an element of the domain",
                    maxlen);

diff --git a/src/mc-range-mincover-private.h b/src/mc-range-mincover-private.h
@@ -44,13 +44,14 @@ typedef struct {
     mc_optional_int32_t min;
     mc_optional_int32_t max;
     size_t sparsity;
-    uint32_t trimFactor;
+    mc_optional_uint32_t trimFactor;
 } mc_getMincoverInt32_args_t;
 
 // mc_getMincoverInt32 implements the Mincover Generation algorithm described in
 // SERVER-68600 for int32_t.
 mc_mincover_t *mc_getMincoverInt32(mc_getMincoverInt32_args_t args,
-                                   mongocrypt_status_t *status) MONGOCRYPT_WARN_UNUSED_RESULT;
+                                   mongocrypt_status_t *status,
+                                   bool use_range_v2) MONGOCRYPT_WARN_UNUSED_RESULT;
 
 typedef struct {
     int64_t lowerBound;
@@ -60,13 +61,14 @@ typedef struct {
     mc_optional_int64_t min;
     mc_optional_int64_t max;
     size_t sparsity;
-    uint32_t trimFactor;
+    mc_optional_uint32_t trimFactor;
 } mc_getMincoverInt64_args_t;
 
 // mc_getMincoverInt64 implements the Mincover Generation algorithm described in
 // SERVER-68600 for int64_t.
 mc_mincover_t *mc_getMincoverInt64(mc_getMincoverInt64_args_t args,
-                                   mongocrypt_status_t *status) MONGOCRYPT_WARN_UNUSED_RESULT;
+                                   mongocrypt_status_t *status,
+                                   bool use_range_v2) MONGOCRYPT_WARN_UNUSED_RESULT;
 
 typedef struct {
     double lowerBound;
@@ -77,7 +79,7 @@ typedef struct {
     mc_optional_double_t min;
     mc_optional_double_t max;
     mc_optional_uint32_t precision;
-    uint32_t trimFactor;
+    mc_optional_uint32_t trimFactor;
 } mc_getMincoverDouble_args_t;
 
 // mc_getMincoverDouble implements the Mincover Generation algorithm described
@@ -95,7 +97,7 @@ typedef struct {
     size_t sparsity;
     mc_optional_dec128_t min, max;
     mc_optional_uint32_t precision;
-    uint32_t trimFactor;
+    mc_optional_uint32_t trimFactor;
 } mc_getMincoverDecimal128_args_t;
 
 // mc_getMincoverDecimal128 implements the Mincover Generation algorithm

diff --git a/src/mc-range-mincover.c b/src/mc-range-mincover.c
@@ -20,7 +20,7 @@
 
 #include "mc-array-private.h"
 #include "mc-range-edge-generation-private.h" // mc_count_leading_zeros_u32
-#include "mc-range-encoding-private.h"        // mc_getTypeInfo32
+#include "mc-range-encoding-private.h"        // mc_getTypeInfo32, trimFactorDefault
 #include "mc-range-mincover-private.h"
 #include "mongocrypt-private.h"
 
@@ -127,7 +127,7 @@ void mc_mincover_destroy(mc_mincover_t *mincover) {
     } else                                                                                                             \
         (void)0
 
-mc_mincover_t *mc_getMincoverInt32(mc_getMincoverInt32_args_t args, mongocrypt_status_t *status) {
+mc_mincover_t *mc_getMincoverInt32(mc_getMincoverInt32_args_t args, mongocrypt_status_t *status, bool use_range_v2) {
     BSON_ASSERT_PARAM(status);
     CHECK_BOUNDS(args, PRId32, IDENTITY, LESSTHAN);
     mc_OSTType_Int32 a, b;
@@ -150,7 +150,7 @@ mc_mincover_t *mc_getMincoverInt32(mc_getMincoverInt32_args_t args, mongocrypt_s
     }
 
     MinCoverGenerator_u32 *mcg =
-        MinCoverGenerator_new_u32(a.value, b.value, a.max, args.sparsity, args.trimFactor, status);
+        MinCoverGenerator_new_u32(a.value, b.value, a.max, args.sparsity, args.trimFactor, status, use_range_v2);
     if (!mcg) {
         return NULL;
     }
@@ -159,7 +159,7 @@ mc_mincover_t *mc_getMincoverInt32(mc_getMincoverInt32_args_t args, mongocrypt_s
     return mc;
 }
 
-mc_mincover_t *mc_getMincoverInt64(mc_getMincoverInt64_args_t args, mongocrypt_status_t *status) {
+mc_mincover_t *mc_getMincoverInt64(mc_getMincoverInt64_args_t args, mongocrypt_status_t *status, bool use_range_v2) {
     BSON_ASSERT_PARAM(status);
     CHECK_BOUNDS(args, PRId64, IDENTITY, LESSTHAN);
     mc_OSTType_Int64 a, b;
@@ -182,7 +182,7 @@ mc_mincover_t *mc_getMincoverInt64(mc_getMincoverInt64_args_t args, mongocrypt_s
     }
 
     MinCoverGenerator_u64 *mcg =
-        MinCoverGenerator_new_u64(a.value, b.value, a.max, args.sparsity, args.trimFactor, status);
+        MinCoverGenerator_new_u64(a.value, b.value, a.max, args.sparsity, args.trimFactor, status, use_range_v2);
     if (!mcg) {
         return NULL;
     }
@@ -225,7 +225,7 @@ mc_mincover_t *mc_getMincoverDouble(mc_getMincoverDouble_args_t args, mongocrypt
     }
 
     MinCoverGenerator_u64 *mcg =
-        MinCoverGenerator_new_u64(a.value, b.value, a.max, args.sparsity, args.trimFactor, status);
+        MinCoverGenerator_new_u64(a.value, b.value, a.max, args.sparsity, args.trimFactor, status, use_range_v2);
     if (!mcg) {
         return NULL;
     }
@@ -269,7 +269,7 @@ mc_getMincoverDecimal128(mc_getMincoverDecimal128_args_t args, mongocrypt_status
     }
 
     MinCoverGenerator_u128 *mcg =
-        MinCoverGenerator_new_u128(a.value, b.value, a.max, args.sparsity, args.trimFactor, status);
+        MinCoverGenerator_new_u128(a.value, b.value, a.max, args.sparsity, args.trimFactor, status, use_range_v2);
     if (!mcg) {
         return NULL;
     }

diff --git a/src/mc-rangeopts.c b/src/mc-rangeopts.c
@@ -119,11 +119,14 @@ bool mc_RangeOpts_parse(mc_RangeOpts_t *ro, const bson_t *in, bool use_range_v2,
     }
 
     // Do not error if min/max are not present. min/max are optional.
-    CHECK_HAS(sparsity);
     // Do not error if precision is not present. Precision is optional and only
     // applies to double/decimal128.
     // Do not error if trimFactor is not present. It is optional.
 
+    if (!has_sparsity && use_range_v2) {
+        ro->sparsity = mc_FLERangeSparsityDefault;
+    }
+
     // Expect precision only to be set for double or decimal128.
     if (has_precision) {
         if (!ro->min.set) {
@@ -491,10 +494,7 @@ bool mc_RangeOpts_appendTrimFactor(const mc_RangeOpts_t *ro,
     BSON_ASSERT(status || true);
 
     if (!ro->trimFactor.set) {
-        if (!BSON_APPEND_INT32(out, fieldName, 0)) {
-            CLIENT_ERR_PREFIXED("failed to append BSON");
-            return false;
-        }
+        // A default `trimFactor` will be selected later with `trimFactorDefault`
         return true;
     }
     BSON_ASSERT(ro->trimFactor.value <= INT32_MAX);