Apply suggestions from code review

Co-authored-by: Paul Masurel <paul@quickwit.io>

fastfield_codecs/src/monotonic_mapping.rs

@@ -36,6 +36,11 @@ pub trait StrictlyMonotonicFn<External, Internal> {
 
 /// Inverts a strictly monotonic mapping from `StrictlyMonotonicFn<A, B>` to
 /// `StrictlyMonotonicFn<B, A>`.
+///
+/// # Warning
+///
+/// This type comes with a footgun. A type being strictly monotonic does not impose that the inverse mapping is strictly monotonic over the entire space External. e.g. a -> a * 2.
+/// Use at your own risks.
 pub(crate) struct StrictlyMonotonicMappingInverter<T> {
     orig_mapping: T,
 }
@@ -96,7 +101,10 @@ where T: MonotonicallyMappableToU64
     }
 }
 
-/// Strictly monotonic mapping with a gcd and a base value.
+/// Mapping dividing by  gcd and a base value.
+/// 
+/// The function is assumed to be only called on values divided by passed
+/// gcd value. (It is necessary for the function to be monotonic.)
 pub(crate) struct StrictlyMonotonicMappingToInternalGCDBaseval {
     gcd_divider: DividerU64,
     gcd: u64,

fastfield_codecs/src/serialize.rs

@@ -102,7 +102,6 @@ pub fn normalize_column<C: Column>(
     gcd: Option<NonZeroU64>,
 ) -> impl Column {
     let gcd = gcd.map(|gcd| gcd.get()).unwrap_or(1);
-
     let mapping = StrictlyMonotonicMappingToInternalGCDBaseval::new(gcd, min_value);
     monotonic_map_column(from_column, mapping)
 }

src/fastfield/readers.rs

@@ -177,7 +177,7 @@ impl FastFieldReaders {
     /// Returns the `u128` fast field reader reader associated to `field`.
     ///
     /// If `field` is not a u128 fast field, this method returns an Error.
-    pub fn u128(&self, field: Field) -> crate::Result<Arc<dyn Column<u128>>> {
+    pub(crate) fn u128(&self, field: Field) -> crate::Result<Arc<dyn Column<u128>>> {
         self.check_type(field, FastType::U128, Cardinality::SingleValue)?;
         let bytes = self.fast_field_data(field, 0)?.read_bytes()?;
         Ok(open_u128::<u128>(bytes)?)