Fix comparison between int and float

cswinter · May 10, 2024 · 8e6cfab · 8e6cfab
1 parent af97463
commit 8e6cfab
Show file tree

Hide file tree

Showing 5 changed files with 171 additions and 0 deletions.
diff --git a/src/engine/operators/mod.rs b/src/engine/operators/mod.rs
@@ -57,6 +57,7 @@ mod pco_decode;
 mod propagate_nullability;
 mod scalar_f64;
 mod scalar_i64;
+mod scalar_i64_to_scalar_f64;
 mod scalar_str;
 mod select;
 mod slice_pack;

diff --git a/src/engine/operators/scalar_i64_to_scalar_f64.rs b/src/engine/operators/scalar_i64_to_scalar_f64.rs
@@ -0,0 +1,31 @@
+use ordered_float::OrderedFloat;
+
+use crate::engine::*;
+
+
+#[derive(Debug)]
+pub struct ScalarI64ToScalarF64 {
+    pub input: BufferRef<Scalar<i64>>,
+    pub output: BufferRef<Scalar<of64>>,
+}
+
+impl<'a> VecOperator<'a> for ScalarI64ToScalarF64 {
+    fn execute(&mut self, _: bool, _: &mut Scratchpad<'a>) -> Result<(), QueryError>{ Ok(()) }
+
+    fn init(&mut self, _: usize, _: usize, scratchpad: &mut Scratchpad<'a>) {
+        let input = scratchpad.get_scalar(&self.input);
+        scratchpad.set_const(self.output, OrderedFloat(input as f64));
+    }
+
+    fn inputs(&self) -> Vec<BufferRef<Any>> { vec![self.input.any()] }
+    fn inputs_mut(&mut self) -> Vec<&mut usize> { vec![&mut self.input.i] }
+    fn outputs(&self) -> Vec<BufferRef<Any>> { vec![self.output.any()] }
+    fn can_stream_input(&self, _: usize) -> bool { true }
+    fn can_stream_output(&self, _: usize) -> bool { true }
+    fn can_block_output(&self) -> bool { true }
+    fn allocates(&self) -> bool { false }
+
+    fn display_op(&self, _: bool) -> String {
+        format!("{} as f64", self.input)
+    }
+}
diff --git a/src/engine/operators/vector_operator.rs b/src/engine/operators/vector_operator.rs
@@ -173,6 +173,8 @@ fn short_type_name<T: ?Sized>() -> String {
 
 #[allow(clippy::branches_sharing_code)]
 pub mod operator {
+    use vector_operator::operators::scalar_i64_to_scalar_f64::ScalarI64ToScalarF64;
+
     use super::*;
 
     pub fn read_column_data<'a>(
@@ -1329,6 +1331,16 @@ pub mod operator {
                 output: NullablePrimitive;
                 Ok(Box::new(NullToVec { input: input.any(), output, batch_size: 0 }))
             }
+        } else if input.tag.is_constant() {
+            assert!(output.tag.is_constant(), "constant to non-constant conversion not supported");
+            if input.tag == EncodingType::ScalarI64 && output.tag == EncodingType::ScalarF64 {
+                return Ok(Box::new(ScalarI64ToScalarF64 {
+                    input: input.scalar_i64()?,
+                    output: output.scalar_f64()?,
+                }));
+            } else {
+                panic!("Scalar conversion from {:?} to {:?} is not implemented", input.tag, output.tag);
+            }
         } else {
             if input.tag == EncodingType::Str && output.tag == EncodingType::OptStr {
                 return Ok(Box::new(TypeConversionOperator {

diff --git a/src/engine/planning/query_plan.rs b/src/engine/planning/query_plan.rs
@@ -857,6 +857,27 @@ fn function2_registry() -> HashMap<Func2Type, Vec<Function2>> {
                     Box::new(|qp, lhs, rhs| qp.less_than(lhs, rhs)),
                     BasicType::String,
                 ),
+                Function2 {
+                    factory: Box::new(|qp, lhs, rhs| {
+                        // TODO: not strictly correct, casting int to float can lose precision, causing aliased values to comapre differently (value might be smaller but compares as equal)
+                        let rhs = int_to_float_cast(qp, rhs).unwrap();
+                        qp.less_than(lhs, rhs)
+                    }),
+                    type_lhs: BasicType::Float,
+                    type_rhs: BasicType::Integer,
+                    type_out: Type::unencoded(BasicType::Boolean).mutable(),
+                    encoding_invariance: true,
+                },
+                Function2 {
+                    factory: Box::new(|qp, lhs, rhs| {
+                        let lhs = int_to_float_cast(qp, lhs).unwrap();
+                        qp.less_than(lhs, rhs)
+                    }),
+                    type_lhs: BasicType::Integer,
+                    type_rhs: BasicType::Float,
+                    type_out: Type::unencoded(BasicType::Boolean).mutable(),
+                    encoding_invariance: true,
+                },
             ],
         ),
         (
@@ -891,6 +912,27 @@ fn function2_registry() -> HashMap<Func2Type, Vec<Function2>> {
                     Box::new(|qp, lhs, rhs| qp.less_than(rhs, lhs)),
                     BasicType::String,
                 ),
+                Function2 {
+                    factory: Box::new(|qp, lhs, rhs| {
+                        // TODO: not strictly correct, casting int to float can lose precision, causing aliased values to comapre differently (value might be smaller but compares as equal)
+                        let rhs = int_to_float_cast(qp, rhs).unwrap();
+                        qp.less_than(rhs, lhs)
+                    }),
+                    type_lhs: BasicType::Float,
+                    type_rhs: BasicType::Integer,
+                    type_out: Type::unencoded(BasicType::Boolean).mutable(),
+                    encoding_invariance: true,
+                },
+                Function2 {
+                    factory: Box::new(|qp, lhs, rhs| {
+                        let lhs = int_to_float_cast(qp, lhs).unwrap();
+                        qp.less_than(rhs, lhs)
+                    }),
+                    type_lhs: BasicType::Integer,
+                    type_rhs: BasicType::Float,
+                    type_out: Type::unencoded(BasicType::Boolean).mutable(),
+                    encoding_invariance: true,
+                },
             ],
         ),
         (
@@ -908,6 +950,27 @@ fn function2_registry() -> HashMap<Func2Type, Vec<Function2>> {
                     Box::new(|qp, lhs, rhs| qp.less_than_equals(rhs, lhs)),
                     BasicType::String,
                 ),
+                Function2 {
+                    factory: Box::new(|qp, lhs, rhs| {
+                        // TODO: not strictly correct, casting int to float can lose precision, causing aliased values to comapre differently (value might be smaller but compares as equal)
+                        let rhs = int_to_float_cast(qp, rhs).unwrap();
+                        qp.less_than_equals(rhs, lhs)
+                    }),
+                    type_lhs: BasicType::Float,
+                    type_rhs: BasicType::Integer,
+                    type_out: Type::unencoded(BasicType::Boolean).mutable(),
+                    encoding_invariance: true,
+                },
+                Function2 {
+                    factory: Box::new(|qp, lhs, rhs| {
+                        let lhs = int_to_float_cast(qp, lhs).unwrap();
+                        qp.less_than_equals(rhs, lhs)
+                    }),
+                    type_lhs: BasicType::Integer,
+                    type_rhs: BasicType::Float,
+                    type_out: Type::unencoded(BasicType::Boolean).mutable(),
+                    encoding_invariance: true,
+                },
             ],
         ),
         (
@@ -925,6 +988,27 @@ fn function2_registry() -> HashMap<Func2Type, Vec<Function2>> {
                     Box::new(|qp, lhs, rhs| qp.equals(lhs, rhs)),
                     BasicType::String,
                 ),
+                Function2 {
+                    factory: Box::new(|qp, lhs, rhs| {
+                        // TODO: not strictly correct, casting int to float can lose precision, causing aliased values to comapre differently (value might be smaller but compares as equal)
+                        let rhs = int_to_float_cast(qp, rhs).unwrap();
+                        qp.equals(lhs, rhs)
+                    }),
+                    type_lhs: BasicType::Float,
+                    type_rhs: BasicType::Integer,
+                    type_out: Type::unencoded(BasicType::Boolean).mutable(),
+                    encoding_invariance: true,
+                },
+                Function2 {
+                    factory: Box::new(|qp, lhs, rhs| {
+                        let lhs = int_to_float_cast(qp, lhs).unwrap();
+                        qp.equals(lhs, rhs)
+                    }),
+                    type_lhs: BasicType::Integer,
+                    type_rhs: BasicType::Float,
+                    type_out: Type::unencoded(BasicType::Boolean).mutable(),
+                    encoding_invariance: true,
+                },
             ],
         ),
         (
@@ -942,6 +1026,27 @@ fn function2_registry() -> HashMap<Func2Type, Vec<Function2>> {
                     Box::new(|qp, lhs, rhs| qp.not_equals(lhs, rhs)),
                     BasicType::String,
                 ),
+                Function2 {
+                    factory: Box::new(|qp, lhs, rhs| {
+                        // TODO: not strictly correct, casting int to float can lose precision, causing aliased values to comapre differently (value might be smaller but compares as equal)
+                        let rhs = int_to_float_cast(qp, rhs).unwrap();
+                        qp.not_equals(lhs, rhs)
+                    }),
+                    type_lhs: BasicType::Float,
+                    type_rhs: BasicType::Integer,
+                    type_out: Type::unencoded(BasicType::Boolean).mutable(),
+                    encoding_invariance: true,
+                },
+                Function2 {
+                    factory: Box::new(|qp, lhs, rhs| {
+                        let lhs = int_to_float_cast(qp, lhs).unwrap();
+                        qp.not_equals(lhs, rhs)
+                    }),
+                    type_lhs: BasicType::Integer,
+                    type_rhs: BasicType::Float,
+                    type_out: Type::unencoded(BasicType::Boolean).mutable(),
+                    encoding_invariance: true,
+                },
             ],
         ),
     ]
@@ -2078,3 +2183,18 @@ pub(super) fn prepare<'a>(
     result.push(operation);
     Ok(result.last_buffer())
 }
+
+fn int_to_float_cast(
+    planner: &mut QueryPlanner,
+    plan: TypedBufferRef,
+) -> Result<TypedBufferRef, QueryError> {
+    let target_type = match plan.tag {
+        EncodingType::I64 => EncodingType::F64,
+        EncodingType::ScalarI64 => EncodingType::ScalarF64,
+        _ => Err(QueryError::TypeError(format!(
+            "Cannot cast {:?} to float",
+            plan.tag
+        )))?,
+    };
+    Ok(planner.cast(plan, target_type))
+}
diff --git a/tests/query_tests.rs b/tests/query_tests.rs
@@ -1693,6 +1693,13 @@ fn test_float_greater_than() {
     );
 }
 
+#[test]
+fn test_float_greater_than_int() {
+    test_query_ec(
+        "SELECT id, float FROM default WHERE float > 0 ORDER BY id LIMIT 2;",
+        &[vec![Int(0), Float(0.123412)], vec![Int(1), Float(0.0003)]],
+    );
+}
 
 // #[test]
 // fn test_missing_count() {