From f408423d7725bb7f646bed5a70440c9cb56e81ec Mon Sep 17 00:00:00 2001
From: Scott Donnelly <scott@donnel.ly>
Date: Thu, 21 Mar 2024 07:22:47 +0000
Subject: [PATCH] feat: modify `Bind` calls so that they don't consume `self`
 and instead return a new struct, leaving the original unmoved"

---
 crates/iceberg/src/expr/mod.rs       |  2 +-
 crates/iceberg/src/expr/predicate.rs | 24 ++++++++++++++++--------
 crates/iceberg/src/expr/term.rs      |  4 ++--
 crates/iceberg/src/spec/values.rs    |  2 +-
 4 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/crates/iceberg/src/expr/mod.rs b/crates/iceberg/src/expr/mod.rs
index 567cf7e913..0d329682e5 100644
--- a/crates/iceberg/src/expr/mod.rs
+++ b/crates/iceberg/src/expr/mod.rs
@@ -154,7 +154,7 @@ pub trait Bind {
     /// The type of the bound result.
     type Bound;
     /// Bind an expression to a schema.
-    fn bind(self, schema: SchemaRef, case_sensitive: bool) -> crate::Result<Self::Bound>;
+    fn bind(&self, schema: SchemaRef, case_sensitive: bool) -> crate::Result<Self::Bound>;
 }
 
 #[cfg(test)]
diff --git a/crates/iceberg/src/expr/predicate.rs b/crates/iceberg/src/expr/predicate.rs
index 67a46e2b11..f8bcffe703 100644
--- a/crates/iceberg/src/expr/predicate.rs
+++ b/crates/iceberg/src/expr/predicate.rs
@@ -66,9 +66,9 @@ where
 {
     type Bound = LogicalExpression<T::Bound, N>;
 
-    fn bind(self, schema: SchemaRef, case_sensitive: bool) -> Result<Self::Bound> {
+    fn bind(&self, schema: SchemaRef, case_sensitive: bool) -> Result<Self::Bound> {
         let mut outputs: [Option<Box<T::Bound>>; N] = array_init(|_| None);
-        for (i, input) in self.inputs.into_iter().enumerate() {
+        for (i, input) in self.inputs.iter().enumerate() {
             outputs[i] = Some(Box::new(input.bind(schema.clone(), case_sensitive)?));
         }
 
@@ -105,7 +105,7 @@ impl<T: Display> Display for UnaryExpression<T> {
 impl<T: Bind> Bind for UnaryExpression<T> {
     type Bound = UnaryExpression<T::Bound>;
 
-    fn bind(self, schema: SchemaRef, case_sensitive: bool) -> Result<Self::Bound> {
+    fn bind(&self, schema: SchemaRef, case_sensitive: bool) -> Result<Self::Bound> {
         let bound_term = self.term.bind(schema, case_sensitive)?;
         Ok(UnaryExpression::new(self.op, bound_term))
     }
@@ -155,9 +155,13 @@ impl<T: Display> Display for BinaryExpression<T> {
 impl<T: Bind> Bind for BinaryExpression<T> {
     type Bound = BinaryExpression<T::Bound>;
 
-    fn bind(self, schema: SchemaRef, case_sensitive: bool) -> Result<Self::Bound> {
+    fn bind(&self, schema: SchemaRef, case_sensitive: bool) -> Result<Self::Bound> {
         let bound_term = self.term.bind(schema.clone(), case_sensitive)?;
-        Ok(BinaryExpression::new(self.op, bound_term, self.literal))
+        Ok(BinaryExpression::new(
+            self.op,
+            bound_term,
+            self.literal.clone(),
+        ))
     }
 }
 
@@ -192,9 +196,13 @@ impl<T> SetExpression<T> {
 impl<T: Bind> Bind for SetExpression<T> {
     type Bound = SetExpression<T::Bound>;
 
-    fn bind(self, schema: SchemaRef, case_sensitive: bool) -> Result<Self::Bound> {
+    fn bind(&self, schema: SchemaRef, case_sensitive: bool) -> Result<Self::Bound> {
         let bound_term = self.term.bind(schema.clone(), case_sensitive)?;
-        Ok(SetExpression::new(self.op, bound_term, self.literals))
+        Ok(SetExpression::new(
+            self.op,
+            bound_term,
+            self.literals.clone(),
+        ))
     }
 }
 
@@ -226,7 +234,7 @@ pub enum Predicate {
 impl Bind for Predicate {
     type Bound = BoundPredicate;
 
-    fn bind(self, schema: SchemaRef, case_sensitive: bool) -> Result<BoundPredicate> {
+    fn bind(&self, schema: SchemaRef, case_sensitive: bool) -> Result<BoundPredicate> {
         match self {
             Predicate::And(expr) => {
                 let bound_expr = expr.bind(schema, case_sensitive)?;
diff --git a/crates/iceberg/src/expr/term.rs b/crates/iceberg/src/expr/term.rs
index e39c97e0f6..15cb298172 100644
--- a/crates/iceberg/src/expr/term.rs
+++ b/crates/iceberg/src/expr/term.rs
@@ -175,7 +175,7 @@ impl Display for Reference {
 impl Bind for Reference {
     type Bound = BoundReference;
 
-    fn bind(self, schema: SchemaRef, case_sensitive: bool) -> crate::Result<Self::Bound> {
+    fn bind(&self, schema: SchemaRef, case_sensitive: bool) -> crate::Result<Self::Bound> {
         let field = if case_sensitive {
             schema.field_by_name(&self.name)
         } else {
@@ -188,7 +188,7 @@ impl Bind for Reference {
                 format!("Field {} not found in schema", self.name),
             )
         })?;
-        Ok(BoundReference::new(self.name, field.clone()))
+        Ok(BoundReference::new(self.name.clone(), field.clone()))
     }
 }
 
diff --git a/crates/iceberg/src/spec/values.rs b/crates/iceberg/src/spec/values.rs
index 00f2e57d2b..f31d64779d 100644
--- a/crates/iceberg/src/spec/values.rs
+++ b/crates/iceberg/src/spec/values.rs
@@ -84,7 +84,7 @@ pub enum PrimitiveLiteral {
 ///
 /// By default, we decouple the type and value of a literal, so we can use avoid the cost of storing extra type info
 /// for each literal. But associate type with literal can be useful in some cases, for example, in unbound expression.
-#[derive(Debug, PartialEq, Hash, Eq)]
+#[derive(Clone, Debug, PartialEq, Hash, Eq)]
 pub struct Datum {
     r#type: PrimitiveType,
     literal: PrimitiveLiteral,