clean up scoring logic

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>

Author: connortsui20

vortex-compressor/src/compressor.rs

@@ -1109,6 +1109,23 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn finite_ratio_does_not_displace_zero_byte_sample() -> VortexResult<()> {
+        let compressor = CascadingCompressor::new(vec![&ZeroBytesSamplingScheme, &HugeRatioScheme]);
+        let schemes: [&'static dyn Scheme; 2] = [&ZeroBytesSamplingScheme, &HugeRatioScheme];
+        let mut data = estimate_test_data();
+
+        let winner =
+            compressor.choose_best_scheme(&schemes, &mut data, CompressorContext::new())?;
+
+        assert!(matches!(
+            winner,
+            Some((scheme, WinnerEstimate::Score(EstimateScore::ZeroBytes)))
+                if scheme.id() == ZeroBytesSamplingScheme.id()
+        ));
+        Ok(())
+    }
+
     #[test]
     fn all_null_array_compresses_to_constant() -> VortexResult<()> {
         let array = PrimitiveArray::new(

vortex-compressor/src/estimate.rs

@@ -92,6 +92,10 @@ pub(super) enum EstimateScore {
     /// A finite compression ratio.
     Finite(f64),
     /// Trial compression produced a 0-byte output.
+    ///
+    /// This is treated as an unbounded score for scheme selection, but has no finite trace ratio.
+    /// Final full-array compression still has to pass the normal before/after acceptance check, so
+    /// a zero-byte sample can choose a scheme but cannot force a larger output to be returned.
     ZeroBytes,
 }
 
@@ -112,6 +116,24 @@ impl EstimateScore {
             Self::ZeroBytes => None,
         }
     }
+
+    /// Returns whether this estimate is eligible to compete.
+    fn is_valid(self) -> bool {
+        match self {
+            Self::Finite(ratio) => ratio.is_finite() && !ratio.is_subnormal() && ratio > 1.0,
+            Self::ZeroBytes => true,
+        }
+    }
+
+    /// Returns whether this estimate beats another valid estimate.
+    fn beats(self, other: Self) -> bool {
+        match (self, other) {
+            (Self::ZeroBytes, Self::Finite(_)) => true,
+            (Self::ZeroBytes, Self::ZeroBytes) => false,
+            (Self::Finite(_), Self::ZeroBytes) => false,
+            (Self::Finite(ratio), Self::Finite(best_ratio)) => ratio > best_ratio,
+        }
+    }
 }
 
 /// Winner estimate carried from scheme selection into result tracing.
@@ -138,20 +160,7 @@ pub(super) fn is_better_score(
     score: EstimateScore,
     best: &Option<(&'static dyn Scheme, EstimateScore)>,
 ) -> bool {
-    match score {
-        EstimateScore::ZeroBytes => {
-            best.is_none_or(|(_, best_score)| !matches!(best_score, EstimateScore::ZeroBytes))
-        }
-        EstimateScore::Finite(ratio) => {
-            ratio.is_finite()
-                && !ratio.is_subnormal()
-                && ratio > 1.0
-                && best.is_none_or(|(_, best_score)| match best_score {
-                    EstimateScore::Finite(best_ratio) => ratio > best_ratio,
-                    EstimateScore::ZeroBytes => false,
-                })
-        }
-    }
+    score.is_valid() && best.is_none_or(|(_, best_score)| score.beats(best_score))
 }
 
 /// Estimates compression ratio by compressing a ~1% sample of the data.