Skip to content

Commit

Permalink
feat: modify Bind calls so that they don't consume self and inste…
Browse files Browse the repository at this point in the history
…ad return a new struct, leaving the original unmoved" (#290)
  • Loading branch information
sdd authored Mar 22, 2024
1 parent ce09b1e commit 757ef4c
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 12 deletions.
2 changes: 1 addition & 1 deletion crates/iceberg/src/expr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
24 changes: 16 additions & 8 deletions crates/iceberg/src/expr/predicate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?));
}

Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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(),
))
}
}

Expand Down Expand Up @@ -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(),
))
}
}

Expand Down Expand Up @@ -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)?;
Expand Down
4 changes: 2 additions & 2 deletions crates/iceberg/src/expr/term.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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()))
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/iceberg/src/spec/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 757ef4c

Please sign in to comment.