diff --git a/extension/partiql-extension-ion/Cargo.toml b/extension/partiql-extension-ion/Cargo.toml index af50e085..110595a8 100644 --- a/extension/partiql-extension-ion/Cargo.toml +++ b/extension/partiql-extension-ion/Cargo.toml @@ -34,7 +34,7 @@ unicase = "2.7" rust_decimal = { version = "1.36.0", default-features = false, features = ["std"] } rust_decimal_macros = "1.36" ion-rs_old = { version = "0.18", package = "ion-rs" } -ion-rs = { version = "1.0.0-rc.10", features = ["experimental"], git = "https://github.com/amazon-ion/ion-rust", branch = "feat-decon-owned-elt" } +ion-rs = { version = "1.0.0-rc.10", features = ["experimental"], git = "https://github.com/amazon-ion/ion-rust" } time = { version = "0.3", features = ["macros"] } once_cell = "1" diff --git a/extension/partiql-extension-ion/src/boxed_ion.rs b/extension/partiql-extension-ion/src/boxed_ion.rs index 68137600..77068a35 100644 --- a/extension/partiql-extension-ion/src/boxed_ion.rs +++ b/extension/partiql-extension-ion/src/boxed_ion.rs @@ -5,7 +5,8 @@ use ion_rs::{ }; use ion_rs_old::IonReader; use partiql_value::boxed_variant::{ - BoxedVariant, BoxedVariantResult, BoxedVariantType, BoxedVariantValueIntoIterator, + BoxedVariant, BoxedVariantResult, BoxedVariantType, BoxedVariantTypeTag, + BoxedVariantValueIntoIterator, DynBoxedVariant, }; use partiql_value::datum::{ Datum, DatumCategoryOwned, DatumCategoryRef, DatumLower, DatumLowerResult, DatumSeqOwned, @@ -16,8 +17,10 @@ use partiql_value::{Bag, BindingsName, List, Tuple, Value, Variant}; use peekmore::{PeekMore, PeekMoreIterator}; #[cfg(feature = "serde")] use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use std::any::Any; use std::borrow::Cow; use std::cell::RefCell; +use std::cmp::Ordering; use std::fmt::{Debug, Display, Formatter}; use std::hash::{Hash, Hasher}; use std::ops::DerefMut; @@ -27,15 +30,42 @@ use thiserror::Error; #[derive(Default, Debug, Copy, Clone)] pub struct BoxedIonType {} impl BoxedVariantType for BoxedIonType { - type Doc = BoxedIon; - - fn construct(&self, bytes: Vec) -> BoxedVariantResult { - BoxedIon::parse(bytes, BoxedIonStreamType::SingleTLV).map_err(Into::into) + fn construct(&self, bytes: Vec) -> BoxedVariantResult { + BoxedIon::parse(bytes, BoxedIonStreamType::SingleTLV) + .map_err(Into::into) + .map(|b| Box::new(b) as DynBoxedVariant) } fn name(&self) -> &'static str { "ion" } + + fn value_eq(&self, l: &DynBoxedVariant, r: &DynBoxedVariant) -> bool { + let (l, r) = get_values(l, r); + + l.eq(r) + } + + fn value_cmp(&self, l: &DynBoxedVariant, r: &DynBoxedVariant) -> Ordering { + let (l, r) = get_values(l, r); + + l.cmp(r) + } +} + +#[inline] +fn get_value(l: &DynBoxedVariant) -> &BoxedIon { + l.as_any().downcast_ref::().expect("IonValue") +} + +#[inline] +fn get_values<'a, 'b>( + l: &'a DynBoxedVariant, + r: &'b DynBoxedVariant, +) -> (&'a BoxedIon, &'b BoxedIon) { + debug_assert_eq!(*l.type_tag(), *r.type_tag()); + + (get_value(l), get_value(r)) } /// Errors in boxed Ion. @@ -117,6 +147,14 @@ impl Hash for BoxedIon { #[cfg_attr(feature = "serde", typetag::serde)] impl BoxedVariant for BoxedIon { + fn type_tag(&self) -> BoxedVariantTypeTag { + Box::new(BoxedIonType {}) + } + + fn as_any(&self) -> &dyn Any { + self + } + fn into_dyn_iter(self: Box) -> BoxedVariantResult { let iter = self.try_into_iter()?; @@ -128,13 +166,19 @@ impl BoxedVariant for BoxedIon { match &self.doc { BoxedIonValue::Stream() => DatumCategoryRef::Sequence(DatumSeqRef::Dynamic(self)), BoxedIonValue::Sequence(seq) => DatumCategoryRef::Sequence(DatumSeqRef::Dynamic(self)), - BoxedIonValue::Value(elt) => match elt.ion_type() { - IonType::List => DatumCategoryRef::Sequence(DatumSeqRef::Dynamic(self)), - IonType::SExp => DatumCategoryRef::Sequence(DatumSeqRef::Dynamic(self)), - IonType::Null => DatumCategoryRef::Null, - IonType::Struct => DatumCategoryRef::Tuple(DatumTupleRef::Dynamic(self)), - _ => DatumCategoryRef::Scalar(DatumValueRef::Lower(self)), - }, + BoxedIonValue::Value(elt) => { + if elt.is_null() { + DatumCategoryRef::Null + } else { + match elt.ion_type() { + IonType::List => DatumCategoryRef::Sequence(DatumSeqRef::Dynamic(self)), + IonType::SExp => DatumCategoryRef::Sequence(DatumSeqRef::Dynamic(self)), + IonType::Null => DatumCategoryRef::Null, + IonType::Struct => DatumCategoryRef::Tuple(DatumTupleRef::Dynamic(self)), + _ => DatumCategoryRef::Scalar(DatumValueRef::Lower(self)), + } + } + } } } @@ -144,44 +188,70 @@ impl BoxedVariant for BoxedIon { BoxedIonValue::Sequence(seq) => { DatumCategoryOwned::Sequence(DatumSeqOwned::Dynamic(self)) } - BoxedIonValue::Value(elt) => match elt.ion_type() { - IonType::List => DatumCategoryOwned::Sequence(DatumSeqOwned::Dynamic(self)), - IonType::SExp => DatumCategoryOwned::Sequence(DatumSeqOwned::Dynamic(self)), - IonType::Null => DatumCategoryOwned::Null, - IonType::Struct => DatumCategoryOwned::Tuple(DatumTupleOwned::Dynamic(self)), - _ => DatumCategoryOwned::Scalar(DatumValueOwned::Value(self.into_value())), - }, + BoxedIonValue::Value(elt) => { + if elt.is_null() { + DatumCategoryOwned::Null + } else { + match elt.ion_type() { + IonType::List => DatumCategoryOwned::Sequence(DatumSeqOwned::Dynamic(self)), + IonType::SExp => DatumCategoryOwned::Sequence(DatumSeqOwned::Dynamic(self)), + IonType::Null => DatumCategoryOwned::Null, + IonType::Struct => { + DatumCategoryOwned::Tuple(DatumTupleOwned::Dynamic(self)) + } + _ => DatumCategoryOwned::Scalar(DatumValueOwned::Value(self.into_value())), + } + } + } } } } +impl PartialEq for BoxedIon { + fn eq(&self, other: &Self) -> bool { + self.doc.eq(&other.doc) + } +} + +impl Eq for BoxedIon {} + +impl PartialOrd for BoxedIon { + fn partial_cmp(&self, other: &Self) -> Option { + 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 DatumLower for BoxedIon { fn into_lower(self) -> DatumLowerResult { let Self { ctx, doc } = self; - match doc { + let pval = match doc { BoxedIonValue::Stream() => todo!("into_lower stream"), - BoxedIonValue::Sequence(seq) => todo!("into_lower seq"), - BoxedIonValue::Value(elt) => { - let pval = elt.into_partiql_value()?; - Ok(match pval { - PartiqlValueTarget::Atom(val) => val, - PartiqlValueTarget::List(l) => { - let vals = l.into_iter().map(|elt| Self::new_value(elt, ctx.clone())); - List::from_iter(vals).into() - } - PartiqlValueTarget::Bag(b) => { - let vals = b.into_iter().map(|elt| Self::new_value(elt, ctx.clone())); - Bag::from_iter(vals).into() - } - PartiqlValueTarget::Struct(s) => { - let vals = s - .into_iter() - .map(|(key, elt)| (key, Self::new_value(elt, ctx.clone()))); - Tuple::from_iter(vals).into() - } - }) + BoxedIonValue::Sequence(seq) => seq.into_partiql_value()?, + BoxedIonValue::Value(elt) => elt.into_partiql_value()?, + }; + Ok(match pval { + PartiqlValueTarget::Atom(val) => val, + PartiqlValueTarget::List(l) => { + let vals = l.into_iter().map(|elt| Self::new_value(elt, ctx.clone())); + List::from_iter(vals).into() } - } + PartiqlValueTarget::Bag(b) => { + let vals = b.into_iter().map(|elt| Self::new_value(elt, ctx.clone())); + Bag::from_iter(vals).into() + } + PartiqlValueTarget::Struct(s) => { + let vals = s + .into_iter() + .map(|(key, elt)| (key, Self::new_value(elt, ctx.clone()))); + Tuple::from_iter(vals).into() + } + }) } fn into_lower_boxed(self: Box) -> DatumLowerResult { @@ -462,6 +532,18 @@ enum BoxedIonValue { Sequence(Sequence), } +impl PartialEq for BoxedIonValue { + fn eq(&self, other: &Self) -> bool { + match (self, other) { + (BoxedIonValue::Value(l), BoxedIonValue::Value(r)) => l == r, + (BoxedIonValue::Sequence(l), BoxedIonValue::Sequence(r)) => l == r, + _ => false, + } + } +} + +impl Eq for BoxedIonValue {} + impl From for BoxedIonValue { fn from(value: Element) -> Self { BoxedIonValue::Value(value) diff --git a/extension/partiql-extension-ion/src/common.rs b/extension/partiql-extension-ion/src/common.rs index ae6b408f..a4fff064 100644 --- a/extension/partiql-extension-ion/src/common.rs +++ b/extension/partiql-extension-ion/src/common.rs @@ -10,6 +10,8 @@ pub enum Encoding { PartiqlEncodedAsIon, } +pub(crate) const BOXED_ION_ANNOT: &str = "$ion"; + pub(crate) const BAG_ANNOT: &str = "$bag"; pub(crate) const TIME_ANNOT: &str = "$time"; pub(crate) const DATE_ANNOT: &str = "$date"; diff --git a/extension/partiql-extension-ion/src/decode.rs b/extension/partiql-extension-ion/src/decode.rs index 6b63f814..55401360 100644 --- a/extension/partiql-extension-ion/src/decode.rs +++ b/extension/partiql-extension-ion/src/decode.rs @@ -1,21 +1,21 @@ use delegate::delegate; use ion_rs_old::{Decimal, Int, IonError, IonReader, IonType, StreamItem, Symbol}; use once_cell::sync::Lazy; -use partiql_value::{Bag, DateTime, List, Tuple, Value}; +use partiql_value::{Bag, DateTime, List, Tuple, Value, Variant}; use regex::RegexSet; use rust_decimal::prelude::ToPrimitive; +use crate::boxed_ion::BoxedIonType; +use crate::common::{ + Encoding, BAG_ANNOT, BOXED_ION_ANNOT, DATE_ANNOT, MISSING_ANNOT, RE_SET_TIME_PARTS, TIME_ANNOT, + TIME_PARTS_HOUR, TIME_PARTS_MINUTE, TIME_PARTS_SECOND, TIME_PARTS_TZ_HOUR, + TIME_PARTS_TZ_MINUTE, +}; use std::num::NonZeroU8; use std::str::FromStr; - use thiserror::Error; use time::Duration; -use crate::common::{ - Encoding, BAG_ANNOT, DATE_ANNOT, MISSING_ANNOT, RE_SET_TIME_PARTS, TIME_ANNOT, TIME_PARTS_HOUR, - TIME_PARTS_MINUTE, TIME_PARTS_SECOND, TIME_PARTS_TZ_HOUR, TIME_PARTS_TZ_MINUTE, -}; - /// Errors in ion decoding. /// /// ### Notes @@ -161,6 +161,28 @@ where } } } +#[inline] +fn dispatch_decode_value(decoder: &D, reader: &mut R, typ: IonType) -> IonDecodeResult +where + R: IonReader, + D: IonValueDecoder + ?Sized, +{ + match typ { + IonType::Null => decoder.decode_null(reader), + IonType::Bool => decoder.decode_bool(reader), + IonType::Int => decoder.decode_int(reader), + IonType::Float => decoder.decode_float(reader), + IonType::Decimal => decoder.decode_decimal(reader), + IonType::Timestamp => decoder.decode_timestamp(reader), + IonType::Symbol => decoder.decode_symbol(reader), + IonType::String => decoder.decode_string(reader), + IonType::Clob => decoder.decode_clob(reader), + IonType::Blob => decoder.decode_blob(reader), + IonType::List => decoder.decode_list(reader), + IonType::SExp => decoder.decode_sexp(reader), + IonType::Struct => decoder.decode_struct(reader), + } +} trait IonValueDecoder where @@ -168,21 +190,7 @@ where { #[inline] fn decode_value(&self, reader: &mut R, typ: IonType) -> IonDecodeResult { - match typ { - IonType::Null => self.decode_null(reader), - IonType::Bool => self.decode_bool(reader), - IonType::Int => self.decode_int(reader), - IonType::Float => self.decode_float(reader), - IonType::Decimal => self.decode_decimal(reader), - IonType::Timestamp => self.decode_timestamp(reader), - IonType::Symbol => self.decode_symbol(reader), - IonType::String => self.decode_string(reader), - IonType::Clob => self.decode_clob(reader), - IonType::Blob => self.decode_blob(reader), - IonType::List => self.decode_list(reader), - IonType::SExp => self.decode_sexp(reader), - IonType::Struct => self.decode_struct(reader), - } + dispatch_decode_value(self, reader, typ) } fn decode_null(&self, reader: &mut R) -> IonDecodeResult; @@ -500,15 +508,38 @@ impl PartiqlEncodedIonValueDecoder { ); Ok(datetime.into()) } + + fn decode_boxed(&self, reader: &mut R) -> IonDecodeResult + where + R: IonReader, + { + let mut loader = ion_elt::ElementLoader::for_reader(reader); + let elt = loader.materialize_current()?.unwrap(); + let ion_ctor = Box::new(BoxedIonType {}); + let contents = elt.to_string(); + Ok(Value::from( + Variant::new(contents, ion_ctor).expect("variant"), + )) + } } impl IonValueDecoder for PartiqlEncodedIonValueDecoder where R: IonReader, { + fn decode_value(&self, reader: &mut R, typ: IonType) -> IonDecodeResult { + if has_annotation(reader, BOXED_ION_ANNOT) { + self.decode_boxed(reader) + } else { + dispatch_decode_value(self, reader, typ) + } + } + #[inline] fn decode_null(&self, reader: &mut R) -> IonDecodeResult { - if has_annotation(reader, MISSING_ANNOT) { + if has_annotation(reader, BOXED_ION_ANNOT) { + self.decode_boxed(reader) + } else if has_annotation(reader, MISSING_ANNOT) { Ok(Value::Missing) } else { Ok(Value::Null) @@ -558,3 +589,106 @@ where } } } + +// Code in this module is copied from ion-rs v0.18, in order to make use of `materialize_current`, +// which is not exposed there. +mod ion_elt { + use ion_rs_old::element::{Element, Sequence, Struct, Value}; + use ion_rs_old::{IonReader, IonResult, StreamItem, Symbol}; + + /// Helper type; wraps an [ElementReader] and recursively materializes the next value in the + /// reader's input, reporting any errors that might occur along the way. + pub(crate) struct ElementLoader<'a, R: ?Sized> { + reader: &'a mut R, + } + + impl<'a, R: IonReader + ?Sized> ElementLoader<'a, R> { + pub(crate) fn for_reader(reader: &'a mut R) -> ElementLoader<'a, R> { + ElementLoader { reader } + } + + /// Advances the reader to the next value in the stream and uses [Self::materialize_current] + /// to materialize it. + pub(crate) fn materialize_next(&mut self) -> IonResult> { + // Advance the reader to the next value + let _ = self.reader.next()?; + self.materialize_current() + } + + /// Recursively materialize the reader's current Ion value and returns it as `Ok(Some(value))`. + /// If there are no more values at this level, returns `Ok(None)`. + /// If an error occurs while materializing the value, returns an `Err`. + /// Calling this method advances the reader and consumes the current value. + pub(crate) fn materialize_current(&mut self) -> IonResult> { + // Collect this item's annotations into a Vec. We have to do this before materializing the + // value itself because materializing a collection requires advancing the reader further. + let mut annotations = Vec::new(); + // Current API limitations require `self.reader.annotations()` to heap allocate its + // iterator even if there aren't annotations. `self.reader.has_annotations()` is trivial + // and allows us to skip the heap allocation in the common case. + if self.reader.has_annotations() { + for annotation in self.reader.annotations() { + annotations.push(annotation?); + } + } + + let value = match self.reader.current() { + // No more values at this level of the stream + StreamItem::Nothing => return Ok(None), + // This is a typed null + StreamItem::Null(ion_type) => Value::Null(ion_type), + // This is a non-null value + StreamItem::Value(ion_type) => { + use ion_rs_old::IonType::*; + match ion_type { + Null => unreachable!("non-null value had IonType::Null"), + Bool => Value::Bool(self.reader.read_bool()?), + Int => Value::Int(self.reader.read_int()?), + Float => Value::Float(self.reader.read_f64()?), + Decimal => Value::Decimal(self.reader.read_decimal()?), + Timestamp => Value::Timestamp(self.reader.read_timestamp()?), + Symbol => Value::Symbol(self.reader.read_symbol()?), + String => Value::String(self.reader.read_string()?), + Clob => Value::Clob(self.reader.read_clob()?.into()), + Blob => Value::Blob(self.reader.read_blob()?.into()), + // It's a collection; recursively materialize all of this value's children + List => Value::List(self.materialize_sequence()?), + SExp => Value::SExp(self.materialize_sequence()?), + Struct => Value::Struct(self.materialize_struct()?), + } + } + }; + Ok(Some(Element::from(value))) + } + + /// Steps into the current sequence and materializes each of its children to construct + /// an [`Vec`]. When all of the the children have been materialized, steps out. + /// The reader MUST be positioned over a list or s-expression when this is called. + fn materialize_sequence(&mut self) -> IonResult { + let mut child_elements = Vec::new(); + self.reader.step_in()?; + while let Some(element) = self.materialize_next()? { + child_elements.push(element); + } + self.reader.step_out()?; + Ok(child_elements.into()) + } + + /// Steps into the current struct and materializes each of its fields to construct + /// an [`Struct`]. When all of the the fields have been materialized, steps out. + /// The reader MUST be positioned over a struct when this is called. + fn materialize_struct(&mut self) -> IonResult { + let mut child_elements = Vec::new(); + self.reader.step_in()?; + while let StreamItem::Value(_) | StreamItem::Null(_) = self.reader.next()? { + let field_name = self.reader.field_name()?; + let value = self + .materialize_current()? + .expect("materialize_current() returned None for user data"); + child_elements.push((field_name, value)); + } + self.reader.step_out()?; + Ok(Struct::from_iter(child_elements)) + } + } +} diff --git a/partiql-conformance-tests/tests/test_value.rs b/partiql-conformance-tests/tests/test_value.rs index f46d46cf..d7e7ccaf 100644 --- a/partiql-conformance-tests/tests/test_value.rs +++ b/partiql-conformance-tests/tests/test_value.rs @@ -4,6 +4,7 @@ use partiql_extension_ion::decode::{IonDecoderBuilder, IonDecoderConfig}; use partiql_extension_ion::Encoding; #[allow(dead_code)] +#[derive(Debug, Eq, PartialEq, Ord, PartialOrd)] pub(crate) struct TestValue { pub value: Value, } diff --git a/partiql-eval/src/eval/evaluable.rs b/partiql-eval/src/eval/evaluable.rs index 4fb6853d..e455e490 100644 --- a/partiql-eval/src/eval/evaluable.rs +++ b/partiql-eval/src/eval/evaluable.rs @@ -15,7 +15,7 @@ use std::collections::hash_map::Entry; use std::collections::HashSet; use std::fmt::{Debug, Formatter}; -use partiql_value::datum::Datum; +use partiql_value::datum::{Datum, DatumLower, DatumLowerResult}; use std::rc::Rc; #[macro_export] @@ -1008,7 +1008,10 @@ impl Evaluable for EvalOrderBy { fn evaluate<'a, 'c>(&mut self, ctx: &'c dyn EvalContext<'c>) -> Value { let input_value = take_input!(self.input.take(), ctx); - let mut values = input_value.into_iter().collect_vec(); + let values: DatumLowerResult> = + input_value.into_iter().map(|v| v.into_lower()).collect(); + // TODO handle lowering error + let mut values = values.expect("lower"); values.sort_by(|l, r| self.compare(l, r, ctx)); Value::from(List::from(values)) } diff --git a/partiql-eval/src/eval/expr/data_types.rs b/partiql-eval/src/eval/expr/data_types.rs index e1dfa114..e03b1dd6 100644 --- a/partiql-eval/src/eval/expr/data_types.rs +++ b/partiql-eval/src/eval/expr/data_types.rs @@ -3,12 +3,13 @@ use crate::error::EvaluationError; use crate::eval::expr::EvalExpr; use crate::eval::EvalContext; -use partiql_value::Value::{Missing, Null}; +use partiql_value::Value::Missing; use partiql_value::{Bag, List, Tuple, Value}; use std::borrow::Cow; use std::fmt::Debug; use partiql_logical::Type; +use partiql_value::datum::{DatumCategory, DatumCategoryRef}; use std::ops::Not; /// Represents an evaluation operator for Tuple expressions such as `{t1.a: t1.b * 2}` in @@ -119,10 +120,11 @@ impl EvalExpr for EvalIsTypeExpr { 'c: 'a, { let expr = self.expr.evaluate(bindings, ctx); - let expr = expr.as_ref(); - let result = match self.is_type { - Type::NullType => matches!(expr, Missing | Null), - Type::MissingType => matches!(expr, Missing), + let result = match (&self.is_type, expr.category()) { + (Type::NullType, DatumCategoryRef::Null | DatumCategoryRef::Missing) => true, + (Type::MissingType, DatumCategoryRef::Missing) => true, + (Type::NullType, _) => false, + (Type::MissingType, _) => false, _ => { ctx.add_error(EvaluationError::NotYetImplemented( "`IS` for other types".to_string(), diff --git a/partiql-value/src/boxed_variant.rs b/partiql-value/src/boxed_variant.rs index 89144b44..4b0f5fb6 100644 --- a/partiql-value/src/boxed_variant.rs +++ b/partiql-value/src/boxed_variant.rs @@ -1,6 +1,8 @@ use dyn_clone::DynClone; use dyn_hash::DynHash; use partiql_common::pretty::PrettyDoc; +use std::any::Any; +use std::cmp::Ordering; use std::error::Error; use crate::datum::{Datum, DatumCategoryOwned, DatumCategoryRef, DatumLower}; @@ -16,29 +18,45 @@ pub type BoxedVariantValueIntoIterator = Box = Box>>; -// dyn - -pub type DynBoxedVariantTypeTag = Box; - -pub trait DynBoxedVariantType: Debug + DynClone { - fn construct(&self, bytes: Vec) -> BoxedVariantResult>; - fn name(&self) -> &'static str; +pub trait DynBoxedVariantTypeFactory { + fn to_dyn_type_tag(self) -> BoxedVariantTypeTag; } -dyn_clone::clone_trait_object!(DynBoxedVariantType); +pub type BoxedVariantTypeTag = Box; +pub trait BoxedVariantType: Debug + DynClone { + fn construct(&self, bytes: Vec) -> BoxedVariantResult; + fn name(&self) -> &'static str; -pub trait DynBoxedVariantTypeFactory { - fn to_dyn_type_tag(self) -> DynBoxedVariantTypeTag; + fn value_eq(&self, l: &DynBoxedVariant, r: &DynBoxedVariant) -> bool; + fn value_cmp(&self, l: &DynBoxedVariant, r: &DynBoxedVariant) -> Ordering; } -// typed +dyn_clone::clone_trait_object!(BoxedVariantType); -pub type BoxedVariantTypeTag = Box>; -pub trait BoxedVariantType: Debug + Clone { - type Doc: BoxedVariant + 'static; +impl Eq for dyn BoxedVariantType {} +impl PartialEq for dyn BoxedVariantType { + fn eq(&self, other: &Self) -> bool { + self.name() == other.name() + } +} +impl PartialOrd for dyn BoxedVariantType { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} +impl Ord for dyn BoxedVariantType { + fn cmp(&self, other: &Self) -> Ordering { + self.name().cmp(other.name()) + } +} - fn construct(&self, bytes: Vec) -> BoxedVariantResult; - fn name(&self) -> &'static str; +impl DynBoxedVariantTypeFactory for T +where + T: BoxedVariantType + 'static, +{ + fn to_dyn_type_tag(self) -> BoxedVariantTypeTag { + Box::new(self) + } } pub type DynBoxedVariant = Box; @@ -46,6 +64,9 @@ pub type DynBoxedVariant = Box; pub trait BoxedVariant: Display + Debug + DynHash + DynClone + Datum + DatumLower { + fn type_tag(&self) -> BoxedVariantTypeTag; + + fn as_any(&self) -> &dyn Any; fn into_dyn_iter(self: Box) -> BoxedVariantResult; fn category(&self) -> DatumCategoryRef<'_>; @@ -56,6 +77,25 @@ pub trait BoxedVariant: dyn_hash::hash_trait_object!(BoxedVariant); dyn_clone::clone_trait_object!(BoxedVariant); +impl Eq for DynBoxedVariant {} +impl PartialEq for DynBoxedVariant { + fn eq(&self, other: &Self) -> bool { + self.type_tag() == other.type_tag() && self.type_tag().value_eq(self, other) + } +} +impl PartialOrd for DynBoxedVariant { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} +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)) + } +} + impl PrettyDoc for DynBoxedVariant { fn pretty_doc<'b, D, A>(&'b self, arena: &'b D) -> DocBuilder<'b, D, A> where @@ -67,27 +107,3 @@ impl PrettyDoc for DynBoxedVariant { arena.text(format!("{}", self)) } } - -impl DynBoxedVariantType for T -where - T: BoxedVariantType, - D: BoxedVariant + 'static, -{ - fn construct(&self, bytes: Vec) -> BoxedVariantResult> { - BoxedVariantType::construct(self, bytes).map(|d| Box::new(d) as Box) - } - - fn name(&self) -> &'static str { - BoxedVariantType::name(self) - } -} - -impl DynBoxedVariantTypeFactory for T -where - T: BoxedVariantType + 'static, - D: BoxedVariant + 'static, -{ - fn to_dyn_type_tag(self) -> DynBoxedVariantTypeTag { - Box::new(self) - } -} diff --git a/partiql-value/src/sort.rs b/partiql-value/src/sort.rs index 96faa0cf..8d4455a2 100644 --- a/partiql-value/src/sort.rs +++ b/partiql-value/src/sort.rs @@ -1,4 +1,4 @@ -use crate::{Bag, List, Tuple, Value}; +use crate::{Bag, List, Tuple, Value, Variant}; use std::cmp::Ordering; /// A wrapper on [`T`] that specifies if a null or missing value should be ordered before @@ -18,9 +18,10 @@ where impl Ord for NullSortedValue<'_, NULLS_FIRST, Value> { fn cmp(&self, other: &Self) -> Ordering { - let wrap_list = NullSortedValue::<{ NULLS_FIRST }, List>; - let wrap_tuple = NullSortedValue::<{ NULLS_FIRST }, Tuple>; - let wrap_bag = NullSortedValue::<{ NULLS_FIRST }, Bag>; + let wrap_list = NullSortedValue::<'_, { NULLS_FIRST }, List>; + let wrap_tuple = NullSortedValue::<'_, { NULLS_FIRST }, Tuple>; + let wrap_bag = NullSortedValue::<'_, { NULLS_FIRST }, Bag>; + let wrap_var = NullSortedValue::<'_, { NULLS_FIRST }, Variant>; let null_cond = |order: Ordering| { if NULLS_FIRST { order @@ -48,6 +49,11 @@ impl Ord for NullSortedValue<'_, NULLS_FIRST, Value> { } (Value::Bag(l), Value::Bag(r)) => wrap_bag(l.as_ref()).cmp(&wrap_bag(r.as_ref())), + + (Value::Variant(l), Value::Variant(r)) => wrap_var(l).cmp(&wrap_var(r)), + (Value::Variant(_), _) => Ordering::Less, + (_, Value::Variant(_)) => Ordering::Greater, + (l, r) => l.cmp(r), } } diff --git a/partiql-value/src/value.rs b/partiql-value/src/value.rs index 1d86134c..5f60e525 100644 --- a/partiql-value/src/value.rs +++ b/partiql-value/src/value.rs @@ -265,7 +265,13 @@ impl Debug for Value { Value::Real(r) => write!(f, "{}", r.0), Value::Decimal(d) => write!(f, "{d}"), Value::String(s) => write!(f, "'{s}'"), - Value::Blob(s) => write!(f, "'{s:?}'"), + Value::Blob(blob) => { + write!(f, "x'")?; + for byte in blob.as_ref() { + f.write_str(&format!("{:02x}", byte))?; + } + write!(f, "'") + } Value::DateTime(t) => t.fmt(f), Value::List(l) => l.fmt(f), Value::Bag(b) => b.fmt(f), @@ -285,10 +291,8 @@ impl PartialOrd for Value { /// TODO: more tests for Ord on Value impl Ord for Value { fn cmp(&self, other: &Self) -> Ordering { + // **NOTE** The Order of these match arms defines the proper comparisons; Do not reorder match (self, other) { - (Value::Variant(_), _) => todo!("Variant Ord"), - (_, Value::Variant(_)) => todo!("Variant Ord"), - (Value::Null, Value::Null) => Ordering::Equal, (Value::Missing, Value::Null) => Ordering::Equal, @@ -393,6 +397,10 @@ impl Ord for Value { (_, Value::Tuple(_)) => Ordering::Greater, (Value::Bag(l), Value::Bag(r)) => l.cmp(r), + (Value::Bag(_), _) => Ordering::Less, + (_, Value::Bag(_)) => Ordering::Greater, + + (Value::Variant(l), Value::Variant(r)) => l.cmp(r), } } } diff --git a/partiql-value/src/variant.rs b/partiql-value/src/variant.rs index 6ab456cf..fbc31958 100644 --- a/partiql-value/src/variant.rs +++ b/partiql-value/src/variant.rs @@ -1,24 +1,25 @@ use crate::boxed_variant::{ - BoxedVariant, BoxedVariantError, BoxedVariantResult, BoxedVariantValueIntoIterator, - BoxedVariantValueIter, DynBoxedVariant, DynBoxedVariantTypeTag, + BoxedVariant, BoxedVariantError, BoxedVariantResult, BoxedVariantTypeTag, + BoxedVariantValueIntoIterator, BoxedVariantValueIter, DynBoxedVariant, }; use crate::datum::{ Datum, DatumCategory, DatumCategoryOwned, DatumCategoryRef, DatumLower, DatumLowerResult, DatumValue, }; -use crate::Value; +use crate::{NullSortedValue, Value}; use delegate::delegate; -use partiql_common::pretty::{pretty_surrounded_doc, PrettyDoc}; +use partiql_common::pretty::{pretty_surrounded_doc, PrettyDoc, ToPretty}; use pretty::{DocAllocator, DocBuilder}; #[cfg(feature = "serde")] use serde::{Deserialize, Deserializer, Serialize, Serializer}; use std::borrow::Cow; -use std::fmt::{Debug, Formatter}; +use std::cmp::Ordering; +use std::fmt::{Debug, Formatter, Write}; use std::hash::{Hash, Hasher}; use thiserror::Error; -#[derive(Clone, Debug)] +#[derive(Clone)] pub struct Variant { variant: DynBoxedVariant, } @@ -26,13 +27,19 @@ pub struct Variant { impl Variant { pub fn new>>( contents: B, - type_tag: DynBoxedVariantTypeTag, + type_tag: BoxedVariantTypeTag, ) -> BoxedVariantResult { let variant = Unparsed::new(contents, type_tag)?.parse()?; Ok(Self { variant }) } } +impl std::fmt::Debug for Variant { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.to_pretty_string(80).expect("pretty")) + } +} + impl From for Variant where T: BoxedVariant + 'static, @@ -78,13 +85,13 @@ impl<'a> DatumCategory<'a> for Variant { #[derive(Debug, Clone)] pub struct Unparsed { contents: Vec, - type_tag: DynBoxedVariantTypeTag, + type_tag: BoxedVariantTypeTag, } impl Unparsed { pub fn new>>( contents: B, - type_tag: DynBoxedVariantTypeTag, + type_tag: BoxedVariantTypeTag, ) -> BoxedVariantResult { Ok(Unparsed { contents: contents.into(), @@ -96,7 +103,7 @@ impl Unparsed { #[derive(Error, Debug)] pub enum VariantError { #[error("Latent Type Error for Boxed Document {0}")] - LatentTypeError(BoxedVariantError, DynBoxedVariantTypeTag), + LatentTypeError(BoxedVariantError, BoxedVariantTypeTag), } pub type VariantResult = Result; @@ -169,9 +176,25 @@ impl Hash for Variant { } } +impl PartialOrd for Variant { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for Variant { + fn cmp(&self, other: &Self) -> Ordering { + let l = &self.variant; + let r = &other.variant; + l.type_tag().cmp(&r.type_tag()).then_with(|| l.cmp(r)) + } +} + impl PartialEq for Variant { - fn eq(&self, _other: &Self) -> bool { - todo!() + fn eq(&self, other: &Self) -> bool { + let lty = self.variant.type_tag(); + let rty = other.variant.type_tag(); + lty.eq(&rty) && self.variant.eq(&other.variant) } } @@ -207,9 +230,23 @@ impl PrettyDoc for Variant { // TODO [EMBDOC] write out type tag? // TODO [EMBDOC] handle backticks more generally. let doc = self.variant.pretty_doc(arena); + let ty = self.variant.type_tag().name(); pretty_surrounded_doc(doc, "`", "`", arena) .append(arena.text("::")) - .append(arena.text("ion")) + .append(arena.text(ty)) + } +} + +impl Ord for NullSortedValue<'_, NULLS_FIRST, Variant> { + fn cmp(&self, other: &Self) -> Ordering { + let wrap = NullSortedValue::<{ NULLS_FIRST }, _>; + + let l = self.0.lower().expect("lower"); + let l = wrap(l.as_ref()); + let r = other.0.lower().expect("lower"); + let r = wrap(r.as_ref()); + + l.cmp(&r) } } diff --git a/partiql/tests/comparisons.rs b/partiql/tests/comparisons.rs index a1962d93..926d13fe 100644 --- a/partiql/tests/comparisons.rs +++ b/partiql/tests/comparisons.rs @@ -33,7 +33,6 @@ pub fn eval_fail(statement: &str) { #[inline] pub fn eval_success(statement: &str) { let (permissive, strict) = eval_modes(statement); - dbg!(&permissive); assert_matches!(permissive, Ok(_)); assert_matches!(strict, Ok(_)); assert_eq!(permissive.unwrap().result, strict.unwrap().result); diff --git a/partiql/tests/ion.rs b/partiql/tests/ion.rs index 9d50fb6f..c7f803ea 100644 --- a/partiql/tests/ion.rs +++ b/partiql/tests/ion.rs @@ -56,3 +56,14 @@ fn ion_iter() { dbg!(&items); assert_eq!(items.len(), 4); } + +#[test] +fn ion_cmp() { + let query = "`foo` < `bar` OR `1.2` > `1`"; + + let res = eval(query, EvaluationMode::Permissive); + assert_matches!(res, Ok(_)); + let result = res.unwrap().result; + + insta::assert_snapshot!(result.to_pretty_string(25).expect("pretty")); +} diff --git a/partiql/tests/snapshots/ion__ion_cmp.snap b/partiql/tests/snapshots/ion__ion_cmp.snap new file mode 100644 index 00000000..370930b1 --- /dev/null +++ b/partiql/tests/snapshots/ion__ion_cmp.snap @@ -0,0 +1,5 @@ +--- +source: partiql/tests/ion.rs +expression: "result.to_pretty_string(25).expect(\"pretty\")" +--- +true