Fix events ordering

Author: gatesn

vortex-expr/src/exprs/between.rs

@@ -1,12 +1,14 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
+use prost::Message;
 use std::fmt::Formatter;
 use vortex_array::compute::{between as between_compute, BetweenOptions};
 use vortex_array::ArrayRef;
 use vortex_dtype::DType;
 use vortex_dtype::DType::Bool;
 use vortex_error::{vortex_bail, VortexExpect, VortexResult};
+use vortex_proto::expr as pb;
 
 use crate::exprs::binary::Binary;
 use crate::exprs::operators::Operator;
@@ -33,6 +35,32 @@ impl VTable for Between {
         ExprId::from("vortex.between")
     }
 
+    fn serialize(&self, instance: &Self::Instance) -> VortexResult<Option<Vec<u8>>> {
+        Ok(Some(
+            pb::BetweenOpts {
+                lower_strict: instance.lower_strict.is_strict(),
+                upper_strict: instance.upper_strict.is_strict(),
+            }
+            .encode_to_vec(),
+        ))
+    }
+
+    fn deserialize(&self, metadata: &[u8]) -> VortexResult<Option<Self::Instance>> {
+        let opts = pb::BetweenOpts::decode(metadata)?;
+        Ok(Some(BetweenOptions {
+            lower_strict: if opts.lower_strict {
+                vortex_array::compute::StrictComparison::Strict
+            } else {
+                vortex_array::compute::StrictComparison::NonStrict
+            },
+            upper_strict: if opts.upper_strict {
+                vortex_array::compute::StrictComparison::Strict
+            } else {
+                vortex_array::compute::StrictComparison::NonStrict
+            },
+        }))
+    }
+
     fn validate(&self, expr: &ExprInstance<Self>) -> VortexResult<()> {
         if expr.children().len() != 3 {
             vortex_bail!(

vortex-expr/src/exprs/binary.rs

@@ -52,9 +52,9 @@ impl VTable for Binary {
 
     fn fmt_sql(&self, expr: &ExprInstance<Self>, f: &mut Formatter<'_>) -> std::fmt::Result {
         write!(f, "(")?;
-        expr.lhs().fmt(f)?;
+        expr.lhs().fmt_sql(f)?;
         write!(f, " {} ", expr.operator())?;
-        expr.rhs().fmt(f)?;
+        expr.rhs().fmt_sql(f)?;
         write!(f, ")")
     }
 

vortex-expr/src/exprs/cast.rs

@@ -172,7 +172,7 @@ mod tests {
             get_item("value", root()),
             DType::Primitive(PType::I64, Nullability::NonNullable),
         );
-        assert_eq!(expr.to_string(), "cast($.value, i64)");
+        assert_eq!(expr.to_string(), "cast($.value as i64)");
 
         let expr2 = cast(root(), DType::Bool(Nullability::Nullable));
         assert_eq!(expr2.to_string(), "cast($, bool?)");

vortex-expr/src/exprs/get_item.rs

@@ -58,6 +58,11 @@ impl VTable for GetItem {
         write!(f, ".{}", expr.data())
     }
 
+    fn fmt_data(&self, instance: &Self::Instance, f: &mut Formatter<'_>) -> std::fmt::Result {
+        // We use debug format to include the quotes around the field name.
+        write!(f, "{:?}", instance.inner().as_ref())
+    }
+
     fn return_dtype(&self, expr: &ExprInstance<Self>, scope: &DType) -> VortexResult<DType> {
         let input = expr.children()[0].return_dtype(scope)?;
         input

vortex-expr/src/exprs/list_contains.rs

@@ -21,6 +21,14 @@ impl VTable for ListContains {
         ExprId::from("vortex.list.contains")
     }
 
+    fn serialize(&self, _instance: &Self::Instance) -> VortexResult<Option<Vec<u8>>> {
+        Ok(Some(vec![]))
+    }
+
+    fn deserialize(&self, _metadata: &[u8]) -> VortexResult<Option<Self::Instance>> {
+        Ok(Some(()))
+    }
+
     fn validate(&self, expr: &ExprInstance<Self>) -> VortexResult<()> {
         if expr.children().len() != 2 {
             vortex_bail!(

vortex-expr/src/exprs/merge.rs

@@ -29,6 +29,24 @@ impl VTable for Merge {
         ExprId::new_ref("vortex.merge")
     }
 
+    fn serialize(&self, instance: &Self::Instance) -> VortexResult<Option<Vec<u8>>> {
+        Ok(Some(match instance {
+            DuplicateHandling::RightMost => vec![0x00],
+            DuplicateHandling::Error => vec![0x01],
+        }))
+    }
+
+    fn deserialize(&self, metadata: &[u8]) -> VortexResult<Option<Self::Instance>> {
+        let instance = match metadata {
+            [0x00] => DuplicateHandling::RightMost,
+            [0x01] => DuplicateHandling::Error,
+            _ => {
+                vortex_bail!("invalid metadata for Merge expression");
+            }
+        };
+        Ok(Some(instance))
+    }
+
     fn validate(&self, _expr: &ExprInstance<Self>) -> VortexResult<()> {
         Ok(())
     }
@@ -428,9 +446,9 @@ mod tests {
     #[test]
     pub fn test_display() {
         let expr = merge([get_item("struct1", root()), get_item("struct2", root())]);
-        assert_eq!(expr.to_string(), "merge[error]($.struct1, $.struct2)");
+        assert_eq!(expr.to_string(), "merge($.struct1, $.struct2)");
 
         let expr2 = merge(vec![get_item("a", root())]);
-        assert_eq!(expr2.to_string(), "merge[error]($.a)");
+        assert_eq!(expr2.to_string(), "merge($.a)");
     }
 }

vortex-expr/src/exprs/not.rs

@@ -47,7 +47,7 @@ impl VTable for Not {
     }
 
     fn fmt_sql(&self, expr: &ExprInstance<Self>, f: &mut Formatter<'_>) -> std::fmt::Result {
-        write!(f, "~(")?;
+        write!(f, "not(")?;
         expr.child(0).fmt_sql(f)?;
         write!(f, ")")
     }
@@ -113,8 +113,8 @@ mod tests {
         let a = not(get_item("a", root()));
         let b = get_item("a", not(root()));
         assert_ne!(a.to_string(), b.to_string());
-        assert_eq!(a.to_string(), "(!$.a)");
-        assert_eq!(b.to_string(), "(!$).a");
+        assert_eq!(a.to_string(), "not($.a)");
+        assert_eq!(b.to_string(), "not($).a");
     }
 
     #[test]

vortex-expr/src/exprs/root.rs

@@ -50,6 +50,10 @@ impl VTable for Root {
         write!(f, "$")
     }
 
+    fn fmt_data(&self, _instance: &Self::Instance, _f: &mut Formatter<'_>) -> std::fmt::Result {
+        Ok(()) // No data
+    }
+
     fn return_dtype(&self, _expr: &ExprInstance<Self>, scope: &DType) -> VortexResult<DType> {
         Ok(scope.clone())
     }

vortex-expr/src/lib.rs

@@ -197,7 +197,7 @@ mod tests {
             "(($.col1 < $.col2) or ($.col1 != $.col2))"
         );
 
-        assert_eq!(not(col1.clone()).to_string(), "(!$.col1)");
+        assert_eq!(not(col1.clone()).to_string(), "not($.col1)");
 
         assert_eq!(
             select(vec![FieldName::from("col1")], root()).to_string(),