Update ion tests and add null & nan comparisions

extension/partiql-extension-ion/src/boxed_ion.rs

@@ -1,9 +1,10 @@
 use crate::util::{PartiqlValueTarget, ToPartiqlValue};
 use ion_rs::{
     AnyEncoding, Element, ElementReader, IonResult, IonType, OwnedSequenceIterator, Reader,
-    Sequence,
+    Sequence, Str, Struct,
 };
 use ion_rs_old::IonReader;
+use itertools::Itertools;
 use partiql_value::boxed_variant::{
     BoxedVariant, BoxedVariantResult, BoxedVariantType, BoxedVariantTypeTag,
     BoxedVariantValueIntoIterator, DynBoxedVariant,
@@ -13,7 +14,7 @@ use partiql_value::datum::{
     DatumSeqRef, DatumTupleOwned, DatumTupleRef, DatumValueOwned, DatumValueRef, OwnedSequenceView,
     OwnedTupleView, RefSequenceView, RefTupleView, SequenceDatum, TupleDatum,
 };
-use partiql_value::{Bag, BindingsName, List, Tuple, Value, Variant};
+use partiql_value::{Bag, BindingsName, EqualityValue, List, NullableEq, Tuple, Value, Variant};
 use peekmore::{PeekMore, PeekMoreIterator};
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
@@ -41,16 +42,33 @@ impl BoxedVariantType for BoxedIonType {
     }
 
     fn value_eq(&self, l: &DynBoxedVariant, r: &DynBoxedVariant) -> bool {
-        let (l, r) = get_values(l, r);
-
-        l.eq(r)
+        wrap_eq::<true, false>(l, r) == Value::Boolean(true)
+    }
+
+    fn value_eq_param(
+        &self,
+        l: &DynBoxedVariant,
+        r: &DynBoxedVariant,
+        nulls_eq: bool,
+        nans_eq: bool,
+    ) -> bool {
+        let res = match (nulls_eq, nans_eq) {
+            (true, true) => wrap_eq::<true, true>(l, r),
+            (true, false) => wrap_eq::<true, false>(l, r),
+            (false, true) => wrap_eq::<false, true>(l, r),
+            (false, false) => wrap_eq::<false, false>(l, r),
+        };
+        res == Value::Boolean(true)
     }
+}
 
-    fn value_cmp(&self, l: &DynBoxedVariant, r: &DynBoxedVariant) -> Ordering {
-        let (l, r) = get_values(l, r);
-
-        l.cmp(r)
-    }
+fn wrap_eq<const NULLS_EQUAL: bool, const NAN_EQUAL: bool>(
+    l: &DynBoxedVariant,
+    r: &DynBoxedVariant,
+) -> Value {
+    let (l, r) = get_values(l, r);
+    let wrap = IonEqualityValue::<'_, { NULLS_EQUAL }, { NAN_EQUAL }, _>;
+    NullableEq::eq(&wrap(l), &wrap(r))
 }
 
 #[inline]
@@ -141,7 +159,7 @@ impl<'de> Deserialize<'de> for BoxedIon {
 
 impl Hash for BoxedIon {
     fn hash<H: Hasher>(&self, state: &mut H) {
-        todo!("BoxedIon.hash")
+        self.doc.hash(state);
     }
 }
 
@@ -207,23 +225,16 @@ impl BoxedVariant for BoxedIon {
     }
 }
 
-impl PartialEq<Self> for BoxedIon {
-    fn eq(&self, other: &Self) -> bool {
-        self.doc.eq(&other.doc)
-    }
-}
-
-impl Eq for BoxedIon {}
+/// A wrapper on [`T`] that specifies if missing and null values should be equal.
+#[derive(Eq, PartialEq)]
+pub struct IonEqualityValue<'a, const NULLS_EQUAL: bool, const NAN_EQUAL: bool, T>(pub &'a T);
 
-impl PartialOrd for BoxedIon {
-    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        Some(self.cmp(other))
-    }
-}
-impl Ord for BoxedIon {
-    fn cmp(&self, other: &Self) -> Ordering {
-        // TODO lowering just to compare is costly... Either find a better way, or lift this out of the extension
-        self.lower().unwrap().cmp(&other.lower().unwrap())
+impl<'a, const NULLS_EQUAL: bool, const NAN_EQUAL: bool> NullableEq
+    for IonEqualityValue<'a, NULLS_EQUAL, NAN_EQUAL, BoxedIon>
+{
+    fn eq(&self, rhs: &Self) -> Value {
+        let wrap = IonEqualityValue::<'a, { NULLS_EQUAL }, { NAN_EQUAL }, _>;
+        wrap(&self.0.doc).eq(&wrap(&rhs.0.doc))
     }
 }
 
@@ -532,6 +543,118 @@ enum BoxedIonValue {
     Sequence(Sequence),
 }
 
+impl Hash for BoxedIonValue {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        match self {
+            BoxedIonValue::Stream() => {
+                todo!("stream not hashable? ")
+            }
+            BoxedIonValue::Value(val) => {
+                let sha = ion_rs::ion_hash::sha256(val).expect("ion hash");
+                state.write(&sha);
+            }
+            BoxedIonValue::Sequence(seq) => todo!("ion seq hash"),
+        }
+    }
+}
+
+impl<'a, const NULLS_EQUAL: bool, const NAN_EQUAL: bool> NullableEq
+    for IonEqualityValue<'a, NULLS_EQUAL, NAN_EQUAL, BoxedIonValue>
+{
+    #[inline(always)]
+    fn eq(&self, other: &Self) -> Value {
+        let wrap = IonEqualityValue::<'a, { NULLS_EQUAL }, { NAN_EQUAL }, Element>;
+        let wrap_seq = IonEqualityValue::<'a, { NULLS_EQUAL }, { NAN_EQUAL }, Sequence>;
+        match (self.0, other.0) {
+            (BoxedIonValue::Value(l), BoxedIonValue::Value(r)) => {
+                NullableEq::eq(&wrap(l), &wrap(r))
+            }
+            (BoxedIonValue::Sequence(l), BoxedIonValue::Sequence(r)) => {
+                NullableEq::eq(&wrap_seq(l), &wrap_seq(r))
+            }
+            _ => Value::Boolean(false),
+        }
+    }
+}
+
+impl<'a, const NULLS_EQUAL: bool, const NAN_EQUAL: bool> NullableEq
+    for IonEqualityValue<'a, NULLS_EQUAL, NAN_EQUAL, Element>
+{
+    fn eq(&self, other: &Self) -> Value {
+        let wrap_seq = IonEqualityValue::<'a, { NULLS_EQUAL }, { NAN_EQUAL }, Sequence>;
+        let wrap_struct = IonEqualityValue::<'a, { NULLS_EQUAL }, { NAN_EQUAL }, Struct>;
+        let (l, r) = (self.0, other.0);
+        let (lty, rty) = (l.ion_type(), r.ion_type());
+
+        let result = if l.is_null() && r.is_null() {
+            NULLS_EQUAL
+        } else {
+            match (lty, rty) {
+                (IonType::Float, IonType::Float) => {
+                    let (l, r) = (l.as_float().unwrap(), r.as_float().unwrap());
+                    if l.is_nan() && r.is_nan() {
+                        NAN_EQUAL
+                    } else {
+                        l == r
+                    }
+                }
+
+                (IonType::List, IonType::List) => {
+                    let (ls, rs) = (l.as_list().unwrap(), r.as_list().unwrap());
+                    l.annotations().eq(r.annotations())
+                        && NullableEq::eq(&wrap_seq(ls), &wrap_seq(rs)) == Value::Boolean(true)
+                }
+                (IonType::SExp, IonType::SExp) => {
+                    let (ls, rs) = (l.as_sexp().unwrap(), r.as_sexp().unwrap());
+                    l.annotations().eq(r.annotations())
+                        && NullableEq::eq(&wrap_seq(ls), &wrap_seq(rs)) == Value::Boolean(true)
+                }
+
+                (IonType::Struct, IonType::Struct) => {
+                    let (ls, rs) = (l.as_struct().unwrap(), r.as_struct().unwrap());
+                    l.annotations().eq(r.annotations())
+                        && NullableEq::eq(&wrap_struct(ls), &wrap_struct(rs))
+                            == Value::Boolean(true)
+                }
+
+                _ => l == r,
+            }
+        };
+
+        Value::Boolean(result)
+    }
+}
+
+impl<'a, const NULLS_EQUAL: bool, const NAN_EQUAL: bool> NullableEq
+    for IonEqualityValue<'a, NULLS_EQUAL, NAN_EQUAL, Sequence>
+{
+    fn eq(&self, other: &Self) -> Value {
+        let wrap = IonEqualityValue::<'a, { NULLS_EQUAL }, { NAN_EQUAL }, _>;
+        let (l, r) = (self.0, other.0);
+        let l = l.iter().map(wrap);
+        let r = r.iter().map(wrap);
+        let res = l.zip(r).all(|(l, r)| l == r);
+        Value::Boolean(res)
+    }
+}
+
+impl<'a, const NULLS_EQUAL: bool, const NAN_EQUAL: bool> NullableEq
+    for IonEqualityValue<'a, NULLS_EQUAL, NAN_EQUAL, Struct>
+{
+    fn eq(&self, other: &Self) -> Value {
+        let wrap = IonEqualityValue::<'a, { NULLS_EQUAL }, { NAN_EQUAL }, _>;
+        let (l, r) = (self.0, other.0);
+        let l = l.iter().map(|(s, elt)| (s, wrap(elt)));
+        let r = r.iter().map(|(s, elt)| (s, wrap(elt)));
+        let res = l.zip(r).all(|((ls, lelt), (rs, relt))| {
+            ls == rs && NullableEq::eq(&lelt, &relt) == Value::Boolean(true)
+        });
+        Value::Boolean(res)
+    }
+}
+
+/*
+
 impl PartialEq<Self> for BoxedIonValue {
     fn eq(&self, other: &Self) -> bool {
         match (self, other) {
@@ -544,6 +667,8 @@ impl PartialEq<Self> for BoxedIonValue {
 
 impl Eq for BoxedIonValue {}
 
+ */
+
 impl From<Element> for BoxedIonValue {
     fn from(value: Element) -> Self {
         BoxedIonValue::Value(value)

partiql-conformance-tests/tests/mod.rs

@@ -210,7 +210,9 @@ pub(crate) fn pass_eval(
     expected: &TestValue,
 ) {
     match eval(statement, mode, env) {
-        Ok(v) => assert_eq!(v.result, expected.value),
+        Ok(v) => {
+            assert_eq!(&TestValue::from(v), expected)
+        },
         Err(TestError::Parse(_)) => {
             panic!("When evaluating (mode = {mode:#?}) `{statement}`, unexpected parse error")
         }

partiql-conformance-tests/tests/test_value.rs

@@ -1,19 +1,41 @@
-use partiql_value::Value;
+use partiql_eval::eval::Evaluated;
+use partiql_value::{Bag, EqualityValue, List, NullSortedValue, NullableEq, Tuple, Value, Variant};
+use std::cmp::Ordering;
 
 use partiql_extension_ion::decode::{IonDecoderBuilder, IonDecoderConfig};
 use partiql_extension_ion::Encoding;
+use partiql_value::datum::DatumLower;
 
 #[allow(dead_code)]
-#[derive(Debug, Eq, PartialEq, Ord, PartialOrd)]
+#[derive(Debug, Ord, PartialOrd)]
 pub(crate) struct TestValue {
     pub value: Value,
 }
 
+impl Eq for TestValue {}
+
+impl PartialEq for TestValue {
+    fn eq(&self, other: &Self) -> bool {
+        let wrap_value = EqualityValue::<'_, true, true, Value>;
+        NullableEq::eq(&wrap_value(&self.value), &wrap_value(&other.value)) == Value::Boolean(true)
+    }
+}
+
+impl From<Value> for TestValue {
+    fn from(value: Value) -> Self {
+        TestValue { value }
+    }
+}
+
+impl From<Evaluated> for TestValue {
+    fn from(value: Evaluated) -> Self {
+        value.result.into()
+    }
+}
+
 impl From<&str> for TestValue {
     fn from(contents: &str) -> Self {
-        TestValue {
-            value: parse_test_value_str(contents),
-        }
+        parse_test_value_str(contents).into()
     }
 }
 

partiql-eval/src/eval/expr/operators.rs

@@ -230,11 +230,11 @@ impl BindEvalExpr for EvalOpBinary {
             EvalOpBinary::And => logical!(AndCheck, partiql_value::BinaryAnd::and),
             EvalOpBinary::Or => logical!(OrCheck, partiql_value::BinaryOr::or),
             EvalOpBinary::Eq => equality!(|lhs, rhs| {
-                let wrap = EqualityValue::<false, Value>;
+                let wrap = EqualityValue::<false, false, Value>;
                 NullableEq::eq(&wrap(lhs), &wrap(rhs))
             }),
             EvalOpBinary::Neq => equality!(|lhs, rhs| {
-                let wrap = EqualityValue::<false, Value>;
+                let wrap = EqualityValue::<false, false, Value>;
                 NullableEq::neq(&wrap(lhs), &wrap(rhs))
             }),
             EvalOpBinary::Gt => comparison!(NullableOrd::gt),

partiql-value/src/bag.rs

@@ -178,16 +178,32 @@ impl Debug for Bag {
 
 impl PartialEq for Bag {
     fn eq(&self, other: &Self) -> bool {
-        if self.len() != other.len() {
-            return false;
+        let wrap = EqualityValue::<true, false, _>;
+        NullableEq::eq(&wrap(self), &wrap(other)) == Value::Boolean(true)
+    }
+}
+
+impl<const NULLS_EQUAL: bool, const NAN_EQUAL: bool> NullableEq
+    for EqualityValue<'_, NULLS_EQUAL, NAN_EQUAL, Bag>
+{
+    #[inline(always)]
+    fn eq(&self, other: &Self) -> Value {
+        let ord_wrap = NullSortedValue::<'_, false, _>;
+        let (l, r) = (self.0, other.0);
+        if l.len() != r.len() {
+            return Value::Boolean(false);
         }
-        for (v1, v2) in self.0.iter().sorted().zip(other.0.iter().sorted()) {
-            let wrap = EqualityValue::<true, Value>;
+
+        let li = l.iter().map(ord_wrap).sorted().map(|nsv| nsv.0);
+        let ri = r.iter().map(ord_wrap).sorted().map(|nsv| nsv.0);
+
+        for (v1, v2) in li.zip(ri) {
+            let wrap = EqualityValue::<{ NULLS_EQUAL }, { NAN_EQUAL }, Value>;
             if NullableEq::eq(&wrap(v1), &wrap(v2)) != Value::Boolean(true) {
-                return false;
+                return Value::Boolean(false);
             }
         }
-        true
+        Value::Boolean(true)
     }
 }
 

partiql-value/src/boxed_variant.rs

@@ -2,11 +2,12 @@ use dyn_clone::DynClone;
 use dyn_hash::DynHash;
 use partiql_common::pretty::PrettyDoc;
 use std::any::Any;
+use std::borrow::Cow;
 use std::cmp::Ordering;
 use std::error::Error;
 
 use crate::datum::{Datum, DatumCategoryOwned, DatumCategoryRef, DatumLower};
-use crate::Value;
+use crate::{EqualityValue, Value};
 use pretty::{DocAllocator, DocBuilder};
 use std::fmt::{Debug, Display};
 
@@ -28,7 +29,14 @@ pub trait BoxedVariantType: Debug + DynClone {
     fn name(&self) -> &'static str;
 
     fn value_eq(&self, l: &DynBoxedVariant, r: &DynBoxedVariant) -> bool;
-    fn value_cmp(&self, l: &DynBoxedVariant, r: &DynBoxedVariant) -> Ordering;
+
+    fn value_eq_param(
+        &self,
+        l: &DynBoxedVariant,
+        r: &DynBoxedVariant,
+        nulls_eq: bool,
+        nans_eq: bool,
+    ) -> bool;
 }
 
 dyn_clone::clone_trait_object!(BoxedVariantType);
@@ -90,9 +98,12 @@ impl PartialOrd for DynBoxedVariant {
 }
 impl Ord for DynBoxedVariant {
     fn cmp(&self, other: &Self) -> Ordering {
-        self.type_tag()
-            .cmp(&other.type_tag())
-            .then_with(|| self.type_tag().value_cmp(self, other))
+        let missing = |_| Cow::Owned(Value::Missing);
+        self.type_tag().cmp(&other.type_tag()).then_with(|| {
+            self.lower()
+                .unwrap_or_else(missing)
+                .cmp(&other.lower().unwrap_or_else(missing))
+        })
     }
 }