Replace BTreeSet with IndexSet in type to order more intuitively (#477)

Author: jpschorr

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - *BREAKING:* partiql-ast: changed modeling of `GroupByExpr` `strategy` field to be an `Option`
 - *BREAKING:* partiql-ast: changed modeling of `PathStep` to split `PathExpr` to `PathIndex` (e.g., `[2]`) and `PathProject` (e.g., `.a`)
 - *BREAKING:* partiql-ast: changed modeling of `PathStep` to rename `PathWildcard` to `PathForEach` (for `[*]`)
+- *BREAKING:* partiql-types: changed type ordering to match specification order
 
 ### Added
 - partiql-ast: Pretty-printing of AST via `ToPretty` trait

extension/partiql-extension-ddl/Cargo.toml

@@ -28,6 +28,8 @@ thiserror = "1.0"
 miette = { version = "7.2", features = ["fancy"] }
 time = { version = "0.3", features = ["formatting", "parsing", "serde"] }
 
+indexmap = "2.2"
+
 [dev-dependencies]
 criterion = "0.4"
 

extension/partiql-extension-ddl/src/ddl.rs

@@ -224,8 +224,8 @@ impl PartiqlDdlEncoder for PartiqlBasicDdlEncoder {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use indexmap::IndexSet;
     use partiql_types::{array, bag, f64, int8, r#struct, str, struct_fields, StructConstraint};
-    use std::collections::BTreeSet;
 
     #[test]
     fn ddl_test() {
@@ -240,7 +240,7 @@ mod tests {
             ("b", array![str![]]),
             ("c", f64!()),
         ];
-        let details = r#struct![BTreeSet::from([nested_attrs])];
+        let details = r#struct![IndexSet::from([nested_attrs])];
 
         let fields = struct_fields![
             ("employee_id", int8![]),
@@ -249,17 +249,17 @@ mod tests {
             ("details", details),
             ("dependents", array![str![]])
         ];
-        let ty = bag![r#struct![BTreeSet::from([
+        let ty = bag![r#struct![IndexSet::from([
             fields,
             StructConstraint::Open(false)
         ])]];
 
-        let expected_compact = r#""dependents" ARRAY<VARCHAR>,"details" STRUCT<"a": UNION<TINYINT,DECIMAL(5, 4)>,"b": ARRAY<VARCHAR>,"c": DOUBLE>,"employee_id" TINYINT,"full_name" VARCHAR,"salary" DECIMAL(8, 2)"#;
-        let expected_pretty = r#""dependents" ARRAY<VARCHAR>,
-"details" STRUCT<"a": UNION<TINYINT,DECIMAL(5, 4)>,"b": ARRAY<VARCHAR>,"c": DOUBLE>,
-"employee_id" TINYINT,
+        let expected_compact = r#""employee_id" TINYINT,"full_name" VARCHAR,"salary" DECIMAL(8, 2),"details" STRUCT<"a": UNION<DECIMAL(5, 4),TINYINT>,"b": ARRAY<VARCHAR>,"c": DOUBLE>,"dependents" ARRAY<VARCHAR>"#;
+        let expected_pretty = r#""employee_id" TINYINT,
 "full_name" VARCHAR,
-"salary" DECIMAL(8, 2)"#;
+"salary" DECIMAL(8, 2),
+"details" STRUCT<"a": UNION<DECIMAL(5, 4),TINYINT>,"b": ARRAY<VARCHAR>,"c": DOUBLE>,
+"dependents" ARRAY<VARCHAR>"#;
 
         let ddl_compact = PartiqlBasicDdlEncoder::new(DdlFormat::Compact);
         assert_eq!(ddl_compact.ddl(&ty).expect("write shape"), expected_compact);

extension/partiql-extension-ddl/tests/ddl-tests.rs

@@ -1,27 +1,27 @@
+use indexmap::IndexSet;
 use partiql_extension_ddl::ddl::{DdlFormat, PartiqlBasicDdlEncoder, PartiqlDdlEncoder};
 use partiql_types::{bag, int, r#struct, str, struct_fields, StructConstraint, StructField};
 use partiql_types::{BagType, PartiqlShape, Static, StructType};
-use std::collections::BTreeSet;
 
 #[test]
 fn basic_ddl_test() {
     let details_fields = struct_fields![("age", int!())];
-    let details = r#struct![BTreeSet::from([details_fields])];
+    let details = r#struct![IndexSet::from([details_fields])];
     let fields = [
         StructField::new("id", int!()),
         StructField::new("name", str!()),
         StructField::new("address", PartiqlShape::new_non_nullable(Static::String)),
         StructField::new_optional("details", details.clone()),
     ]
     .into();
-    let shape = bag![r#struct![BTreeSet::from([
+    let shape = bag![r#struct![IndexSet::from([
         StructConstraint::Fields(fields),
         StructConstraint::Open(false)
     ])]];
 
     let ddl_compact = PartiqlBasicDdlEncoder::new(DdlFormat::Compact);
     let actual = ddl_compact.ddl(&shape).expect("ddl_output");
-    let expected = r#""address" VARCHAR NOT NULL,"id" INT,"name" VARCHAR,"details" OPTIONAL STRUCT<"age": INT>"#;
+    let expected = r#""id" INT,"name" VARCHAR,"address" VARCHAR NOT NULL,"details" OPTIONAL STRUCT<"age": INT>"#;
 
     println!("Actual: {actual}");
     println!("Expected: {expected}");

partiql-ast-passes/Cargo.toml

@@ -26,7 +26,7 @@ partiql-types = { path = "../partiql-types", version = "0.8.*" }
 
 assert_matches = "1.5.*"
 fnv = "1"
-indexmap = "1.9"
+indexmap = "2.2"
 thiserror = "1.0"
 
 [dev-dependencies]

partiql-ast/Cargo.toml

@@ -20,7 +20,7 @@ path = "src/lib.rs"
 bench = false
 
 [dependencies]
-indexmap = { version = "1.9", default-features = false }
+indexmap = "2.2"
 rust_decimal = { version = "1.25.0", default-features = false, features = ["std"] }
 serde = { version = "1.*", features = ["derive"], optional = true }
 pretty = "0.12"

partiql-logical-planner/Cargo.toml

@@ -34,7 +34,7 @@ ion-rs = "0.18"
 ordered-float = "3.*"
 itertools = "0.10.*"
 unicase = "2.6"
-indexmap = "1.9"
+indexmap = "2.2"
 petgraph = "0.6.*"
 num = "0.4"
 fnv = "1"

partiql-logical-planner/src/typer.rs

@@ -1,5 +1,5 @@
 use crate::typer::LookupOrder::{GlobalLocal, LocalGlobal};
-use indexmap::IndexMap;
+use indexmap::{IndexMap, IndexSet};
 use partiql_ast::ast::{CaseSensitivity, SymbolPrimitive};
 use partiql_catalog::Catalog;
 use partiql_logical::{BindingsOp, LogicalPlan, OpId, PathComponent, ValueExpr, VarRefType};
@@ -11,7 +11,7 @@ use partiql_value::{BindingsName, Value};
 use petgraph::algo::toposort;
 use petgraph::graph::NodeIndex;
 use petgraph::prelude::StableGraph;
-use std::collections::{BTreeSet, HashMap};
+use std::collections::HashMap;
 use thiserror::Error;
 
 #[macro_export]
@@ -218,7 +218,7 @@ impl<'c> PlanTyper<'c> {
                     StructField::new(k.as_str(), self.get_singleton_type_from_env())
                 });
 
-                let ty = PartiqlShape::new_struct(StructType::new(BTreeSet::from([
+                let ty = PartiqlShape::new_struct(StructType::new(IndexSet::from([
                     StructConstraint::Fields(fields.collect()),
                 ])));
 
@@ -686,7 +686,7 @@ mod tests {
 
         // Open Schema with `Strict` typing mode and `age` in nested attribute.
         let details_fields = struct_fields![("age", int!())];
-        let details = r#struct![BTreeSet::from([details_fields])];
+        let details = r#struct![IndexSet::from([details_fields])];
 
         assert_query_typing(
             TypingMode::Strict,
@@ -731,7 +731,7 @@ mod tests {
     fn simple_sfw_with_alias() {
         // Open Schema with `Strict` typing mode and `age` in nested attribute.
         let details_fields = struct_fields![("age", int!())];
-        let details = r#struct![BTreeSet::from([details_fields])];
+        let details = r#struct![IndexSet::from([details_fields])];
 
         // TODO Revise this behavior once the following discussion is conclusive and spec. is
         // in place: https://github.com/partiql/partiql-spec/discussions/65
@@ -775,7 +775,7 @@ mod tests {
     #[test]
     fn simple_sfw_err() {
         // Closed Schema with `Strict` typing mode and `age` non-existent projection.
-        let err1 = r#"No Typing Information for SymbolPrimitive { value: "age", case: CaseInsensitive } in closed Schema Static(StaticType { ty: Struct(StructType { constraints: {Open(false), Fields({StructField { optional: false, name: "id", ty: Static(StaticType { ty: Int, nullable: true }) }, StructField { optional: false, name: "name", ty: Static(StaticType { ty: String, nullable: true }) }})} }), nullable: true })"#;
+        let err1 = r#"No Typing Information for SymbolPrimitive { value: "age", case: CaseInsensitive } in closed Schema Static(StaticType { ty: Struct(StructType { constraints: {Fields({StructField { optional: false, name: "id", ty: Static(StaticType { ty: Int, nullable: true }) }, StructField { optional: false, name: "name", ty: Static(StaticType { ty: String, nullable: true }) }}), Open(false)} }), nullable: true })"#;
 
         assert_err(
             assert_query_typing(
@@ -792,7 +792,7 @@ mod tests {
                 vec![],
             ),
             vec![TypingError::TypeCheck(err1.to_string())],
-            Some(bag![r#struct![BTreeSet::from([StructConstraint::Fields(
+            Some(bag![r#struct![IndexSet::from([StructConstraint::Fields(
                 [
                     StructField::new("id", int!()),
                     StructField::new("name", str!()),
@@ -804,12 +804,12 @@ mod tests {
 
         // Closed Schema with `Strict` typing mode and `bar` non-existent projection from closed nested `details`.
         let details_fields = struct_fields![("age", int!())];
-        let details = r#struct![BTreeSet::from([
+        let details = r#struct![IndexSet::from([
             details_fields,
             StructConstraint::Open(false)
         ])];
 
-        let err1 = r#"No Typing Information for SymbolPrimitive { value: "details", case: CaseInsensitive } in closed Schema Static(StaticType { ty: Struct(StructType { constraints: {Open(false), Fields({StructField { optional: false, name: "age", ty: Static(StaticType { ty: Int, nullable: true }) }})} }), nullable: true })"#;
+        let err1 = r#"No Typing Information for SymbolPrimitive { value: "details", case: CaseInsensitive } in closed Schema Static(StaticType { ty: Struct(StructType { constraints: {Fields({StructField { optional: false, name: "age", ty: Static(StaticType { ty: Int, nullable: true }) }}), Open(false)} }), nullable: true })"#;
         let err2 = r"Illegal Derive Type Undefined";
 
         assert_err(
@@ -831,7 +831,7 @@ mod tests {
                 TypingError::TypeCheck(err1.to_string()),
                 TypingError::IllegalState(err2.to_string()),
             ],
-            Some(bag![r#struct![BTreeSet::from([StructConstraint::Fields(
+            Some(bag![r#struct![IndexSet::from([StructConstraint::Fields(
                 [
                     StructField::new("id", int!()),
                     StructField::new("name", str!()),
@@ -863,8 +863,8 @@ mod tests {
         };
     }
 
-    fn create_customer_schema(is_open: bool, fields: BTreeSet<StructField>) -> PartiqlShape {
-        bag![r#struct![BTreeSet::from([
+    fn create_customer_schema(is_open: bool, fields: IndexSet<StructField>) -> PartiqlShape {
+        bag![r#struct![IndexSet::from([
             StructConstraint::Fields(fields),
             StructConstraint::Open(is_open)
         ])]]
@@ -876,7 +876,7 @@ mod tests {
         schema: PartiqlShape,
         expected_fields: Vec<StructField>,
     ) -> Result<(), TypeErr> {
-        let expected_fields: BTreeSet<_> = expected_fields.into_iter().collect();
+        let expected_fields: IndexSet<_> = expected_fields.into_iter().collect();
         let actual = type_query(mode, query, TypeEnvEntry::new("customers", &[], schema))?
             .expect_static()?;
 
@@ -887,7 +887,7 @@ mod tests {
 
                     let f: Vec<_> = expected_fields
                         .iter()
-                        .filter(|f| !fields.contains(f))
+                        .filter(|f| !fields.contains(*f))
                         .collect();
                     assert!(f.is_empty());
                     assert_eq!(expected_fields.len(), fields.len());

partiql-types/Cargo.toml

@@ -29,5 +29,9 @@ unicase = "2.6"
 miette = { version ="7.2.*", features = ["fancy"] }
 thiserror = "1.*"
 
+indexmap = "2.2"
+
+derivative = "2.2"
+
 [dev-dependencies]
 criterion = "0.4"

partiql-types/src/lib.rs

@@ -1,11 +1,12 @@
 #![deny(rust_2018_idioms)]
 #![deny(clippy::all)]
 
+use derivative::Derivative;
+use indexmap::IndexSet;
 use itertools::Itertools;
 use miette::Diagnostic;
-use std::collections::BTreeSet;
 use std::fmt::{Debug, Display, Formatter};
-use std::hash::Hash;
+use std::hash::{Hash, Hasher};
 use thiserror::Error;
 
 #[derive(Debug, Clone, Eq, PartialEq, Hash, Error, Diagnostic)]
@@ -23,6 +24,16 @@ pub trait Type {}
 
 impl Type for StaticType {}
 
+fn indexset_hash<H, T>(set: &IndexSet<T>, state: &mut H)
+where
+    H: Hasher,
+    T: Hash,
+{
+    for val in set {
+        val.hash(state)
+    }
+}
+
 #[macro_export]
 macro_rules! dynamic {
     () => {
@@ -140,7 +151,7 @@ macro_rules! undefined {
 }
 
 /// Represents a PartiQL Shape
-#[derive(Debug, Clone, Eq, PartialEq, Hash, Ord, PartialOrd)]
+#[derive(Debug, Clone, Eq, PartialEq, Hash)]
 // With this implementation `Dynamic` and `AnyOf` cannot have `nullability`; this does not mean their
 // `null` value at runtime cannot belong to their domain.
 // TODO adopt the correct model Pending PartiQL Types semantics finalization: https://github.com/partiql/partiql-lang/issues/18
@@ -151,13 +162,13 @@ pub enum PartiqlShape {
     Undefined,
 }
 
-#[derive(Debug, Clone, Eq, PartialEq, Hash, Ord, PartialOrd)]
+#[derive(Debug, Clone, Eq, PartialEq, Hash)]
 pub struct StaticType {
     ty: Static,
     nullable: bool,
 }
 
-#[derive(Debug, Clone, Eq, PartialEq, Hash, Ord, PartialOrd)]
+#[derive(Debug, Clone, Eq, PartialEq, Hash)]
 pub enum Static {
     // Scalar Types
     Int,
@@ -506,15 +517,17 @@ impl Display for PartiqlShape {
     }
 }
 
-#[derive(Hash, Eq, PartialEq, Debug, Clone, Ord, PartialOrd)]
+#[derive(Derivative, Eq, Debug, Clone)]
+#[derivative(PartialEq, Hash)]
 #[allow(dead_code)]
 pub struct AnyOf {
-    types: BTreeSet<PartiqlShape>,
+    #[derivative(Hash(hash_with = "indexset_hash"))]
+    types: IndexSet<PartiqlShape>,
 }
 
 impl AnyOf {
     #[must_use]
-    pub const fn new(types: BTreeSet<PartiqlShape>) -> Self {
+    pub const fn new(types: IndexSet<PartiqlShape>) -> Self {
         AnyOf { types }
     }
 
@@ -531,15 +544,17 @@ impl FromIterator<PartiqlShape> for AnyOf {
     }
 }
 
-#[derive(Debug, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)]
+#[derive(Derivative, Eq, Debug, Clone)]
+#[derivative(PartialEq, Hash)]
 #[allow(dead_code)]
 pub struct StructType {
-    constraints: BTreeSet<StructConstraint>,
+    #[derivative(Hash(hash_with = "indexset_hash"))]
+    constraints: IndexSet<StructConstraint>,
 }
 
 impl StructType {
     #[must_use]
-    pub fn new(constraints: BTreeSet<StructConstraint>) -> Self {
+    pub fn new(constraints: IndexSet<StructConstraint>) -> Self {
         StructType { constraints }
     }
 
@@ -551,7 +566,7 @@ impl StructType {
     }
 
     #[must_use]
-    pub fn fields(&self) -> BTreeSet<StructField> {
+    pub fn fields(&self) -> IndexSet<StructField> {
         self.constraints
             .iter()
             .flat_map(|c| {
@@ -575,17 +590,18 @@ impl StructType {
     }
 }
 
-#[derive(Debug, Clone, Eq, PartialEq, Hash, Ord, PartialOrd)]
+#[derive(Derivative, Eq, Debug, Clone)]
+#[derivative(PartialEq, Hash)]
 #[allow(dead_code)]
 #[non_exhaustive]
 pub enum StructConstraint {
     Open(bool),
     Ordered(bool),
     DuplicateAttrs(bool),
-    Fields(BTreeSet<StructField>),
+    Fields(#[derivative(Hash(hash_with = "indexset_hash"))] IndexSet<StructField>),
 }
 
-#[derive(Debug, Clone, Eq, PartialEq, Hash, Ord, PartialOrd)]
+#[derive(Debug, Clone, Hash, Eq, PartialEq)]
 #[allow(dead_code)]
 pub struct StructField {
     optional: bool,
@@ -648,7 +664,8 @@ impl From<(&str, PartiqlShape, bool)> for StructField {
     }
 }
 
-#[derive(Debug, Clone, Eq, PartialEq, Hash, Ord, PartialOrd)]
+#[derive(Derivative, Eq, Debug, Clone)]
+#[derivative(PartialEq, Hash)]
 #[allow(dead_code)]
 pub struct BagType {
     element_type: Box<PartiqlShape>,
@@ -671,7 +688,8 @@ impl BagType {
     }
 }
 
-#[derive(Debug, Clone, Eq, PartialEq, Hash, Ord, PartialOrd)]
+#[derive(Derivative, Eq, Debug, Clone)]
+#[derivative(PartialEq, Hash)]
 #[allow(dead_code)]
 pub struct ArrayType {
     element_type: Box<PartiqlShape>,