Refactors to make working with types a bit more ergonomic (#478)

Author: jpschorr

CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - *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
+- *BREAKING:* partiql-types: changed some interfaces to reduce clones and be more ergonomic
 
 ### Added
 - partiql-ast: Pretty-printing of AST via `ToPretty` trait

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

@@ -122,9 +122,9 @@ impl PartiqlBasicDdlEncoder {
             Static::Float32 => out.push_str("REAL"),
             Static::Float64 => out.push_str("DOUBLE"),
             Static::String => out.push_str("VARCHAR"),
-            Static::Struct(s) => out.push_str(&self.write_struct(&s)?),
-            Static::Bag(b) => out.push_str(&self.write_bag(&b)?),
-            Static::Array(a) => out.push_str(&self.write_array(&a)?),
+            Static::Struct(s) => out.push_str(&self.write_struct(s)?),
+            Static::Bag(b) => out.push_str(&self.write_bag(b)?),
+            Static::Array(a) => out.push_str(&self.write_array(a)?),
             // non-exhaustive catch-all
             _ => todo!("handle type for {}", ty),
         }
@@ -147,8 +147,7 @@ impl PartiqlBasicDdlEncoder {
     fn write_struct(&self, strct: &StructType) -> ShapeDdlEncodeResult<String> {
         let mut struct_out = String::from("STRUCT<");
 
-        let fields = strct.fields();
-        let mut fields = fields.iter().peekable();
+        let mut fields = strct.fields().peekable();
         while let Some(field) = fields.next() {
             struct_out.push_str(&format!("\"{}\": ", field.name()));
             struct_out.push_str(&self.write_shape(field.ty())?);
@@ -192,8 +191,7 @@ impl PartiqlDdlEncoder for PartiqlBasicDdlEncoder {
 
         if let Static::Bag(bag) = ty.ty() {
             let s = bag.element_type().expect_struct()?;
-            let fields = s.fields();
-            let mut fields = fields.iter().peekable();
+            let mut fields = s.fields().peekable();
             while let Some(field) = fields.next() {
                 output.push_str(&format!("\"{}\" ", field.name()));
 

partiql-logical-planner/src/typer.rs

@@ -580,7 +580,6 @@ fn string_to_sym(name: &str) -> SymbolPrimitive {
 
 fn to_bindings(s: &StructType) -> Vec<(SymbolPrimitive, PartiqlShape)> {
     s.fields()
-        .into_iter()
         .map(|field| {
             let sym = SymbolPrimitive {
                 value: field.name().to_string(),
@@ -883,7 +882,7 @@ mod tests {
         match &actual.ty() {
             Static::Bag(b) => {
                 if let Ok(s) = b.element_type().expect_struct() {
-                    let fields = s.fields();
+                    let fields: IndexSet<_> = s.fields().collect();
 
                     let f: Vec<_> = expected_fields
                         .iter()

partiql-types/src/lib.rs

@@ -196,23 +196,37 @@ pub enum Static {
     // TODO Add BitString, ByteString, Blob, Clob, and Graph types
 }
 
+impl Static {
+    pub fn is_scalar(&self) -> bool {
+        !matches!(self, Static::Struct(_) | Static::Bag(_) | Static::Array(_))
+    }
+
+    pub fn is_sequence(&self) -> bool {
+        matches!(self, Static::Bag(_) | Static::Array(_))
+    }
+
+    pub fn is_struct(&self) -> bool {
+        matches!(self, Static::Struct(_))
+    }
+}
+
 impl StaticType {
     #[must_use]
-    pub fn new(&self, ty: Static) -> StaticType {
+    pub fn new(ty: Static) -> StaticType {
         StaticType { ty, nullable: true }
     }
 
     #[must_use]
-    pub fn new_non_nullable(&self, ty: Static) -> StaticType {
+    pub fn new_non_nullable(ty: Static) -> StaticType {
         StaticType {
             ty,
             nullable: false,
         }
     }
 
     #[must_use]
-    pub fn ty(&self) -> Static {
-        self.ty.clone()
+    pub fn ty(&self) -> &Static {
+        &self.ty
     }
 
     #[must_use]
@@ -224,6 +238,18 @@ impl StaticType {
     pub fn is_not_nullable(&self) -> bool {
         !self.nullable
     }
+
+    pub fn is_scalar(&self) -> bool {
+        self.ty.is_scalar()
+    }
+
+    pub fn is_sequence(&self) -> bool {
+        self.ty.is_sequence()
+    }
+
+    pub fn is_struct(&self) -> bool {
+        self.ty.is_struct()
+    }
 }
 
 impl Display for StaticType {
@@ -565,8 +591,7 @@ impl StructType {
         }
     }
 
-    #[must_use]
-    pub fn fields(&self) -> IndexSet<StructField> {
+    pub fn fields_set(&self) -> IndexSet<StructField> {
         self.constraints
             .iter()
             .flat_map(|c| {
@@ -579,6 +604,19 @@ impl StructType {
             .collect()
     }
 
+    pub fn fields(&self) -> impl Iterator<Item = &StructField> {
+        self.constraints
+            .iter()
+            .filter_map(|c| {
+                if let StructConstraint::Fields(fields) = c {
+                    Some(fields)
+                } else {
+                    None
+                }
+            })
+            .flat_map(|f| f.iter())
+    }
+
     #[must_use]
     pub fn is_partial(&self) -> bool {
         !self.is_closed()