Skip to content

Commit

Permalink
sql: remove Expression::Field label
Browse files Browse the repository at this point in the history
  • Loading branch information
@erikgrinaker
erikgrinaker committed Jul 16, 2024
1 parent 40291e7 commit f34b327
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 34 deletions.
21 changes: 10 additions & 11 deletions src/sql/planner/optimizer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ pub(super) fn fold_constants(node: Node) -> Result<Node> {

// Transforms expressions.
let transform = |expr: Expression| {
// If the expression is constant, evaulate it.
if !expr.contains(&|e| matches!(e, Expression::Field(_, _))) {
// If the expression is constant, evaluate it.
if !expr.contains(&|e| matches!(e, Expression::Field(_))) {
return expr.evaluate(None).map(Expression::Constant);
}
// If the expression is a logical operator, and one of the sides is
Expand Down Expand Up @@ -123,7 +123,7 @@ pub(super) fn push_filters(node: Node) -> Result<Node> {
for expr in cnf {
let (mut ref_left, mut ref_right) = (false, false);
expr.walk(&mut |e| {
if let Expression::Field(index, _) = e {
if let Expression::Field(index) = e {
ref_left = ref_left || *index < left.size();
ref_right = ref_right || *index >= left.size();
}
Expand Down Expand Up @@ -160,19 +160,18 @@ pub(super) fn push_filters(node: Node) -> Result<Node> {
for expr in &predicate {
// Find equijoins.
let Expression::Equal(lhs, rhs) = expr else { continue };
let Expression::Field(l, lname) = lhs.as_ref() else { continue };
let Expression::Field(r, rname) = rhs.as_ref() else { continue };
let Expression::Field(l) = lhs.as_ref() else { continue };
let Expression::Field(r) = rhs.as_ref() else { continue };

// The lhs may be a reference to the right source; swap them.
let (l, lname, r, rname) =
if l > r { (r, rname, l, lname) } else { (l, lname, r, rname) };
let (l, r) = if l > r { (r, l) } else { (l, r) };

// Check if either side is a field lookup, and copy it over.
if let Some(expr) = left_lookups.get(l).map(|i| push_left[*i].clone()) {
push_right.push(expr.replace_field(*l, *r, rname));
push_right.push(expr.replace_field(*l, *r));
}
if let Some(expr) = right_lookups.get(r).map(|i| push_right[*i].clone()) {
push_left.push(expr.replace_field(*r, *l, lname));
push_left.push(expr.replace_field(*r, *l));
}
}

Expand Down Expand Up @@ -260,7 +259,7 @@ pub(super) fn join_type(node: Node) -> Result<Node> {
predicate: Some(Expression::Equal(lhs, rhs)),
outer,
} => match (*lhs, *rhs) {
(Expression::Field(mut left_field, _), Expression::Field(mut right_field, _)) => {
(Expression::Field(mut left_field), Expression::Field(mut right_field)) => {
// The LHS field may be a field in the right table; swap them.
if right_field < left_field {
(left_field, right_field) = (right_field, left_field);
Expand Down Expand Up @@ -343,7 +342,7 @@ pub(super) fn short_circuit(node: Node) -> Result<Node> {
&& expressions
.iter()
.enumerate()
.all(|(i, e)| matches!(e, Expression::Field(f, _) if i == *f)) =>
.all(|(i, e)| matches!(e, Expression::Field(f) if i == *f)) =>
{
*source
}
Expand Down
4 changes: 2 additions & 2 deletions src/sql/planner/plan.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,13 +282,13 @@ impl Node {
// Some nodes rearrange columns. Route them to the correct
// upstream column where appropriate.
Self::Aggregate { source, group_by, .. } => match group_by.get(index) {
Some(Expression::Field(index, _)) => source.column_label(*index),
Some(Expression::Field(index)) => source.column_label(*index),
Some(_) | None => Label::None,
},
Self::Projection { source, expressions, aliases } => match aliases.get(index) {
Some(Label::None) | None => match expressions.get(index) {
// Unaliased field references route to the source.
Some(Expression::Field(index, _)) => source.column_label(*index),
Some(Expression::Field(index)) => source.column_label(*index),
// Unaliased expressions don't have a name.
Some(_) | None => Label::None,
},
Expand Down
9 changes: 4 additions & 5 deletions src/sql/planner/planner.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ impl<'a, C: Catalog> Planner<'a, C> {
if let Some(index) = scope.lookup_aggregate(expr) {
if child_scope.lookup_aggregate(expr).is_none() {
child_scope.add_passthrough(scope, index, true);
hidden.push(Expression::Field(index, scope.get_label(index)));
hidden.push(Expression::Field(index));
return true;
}
}
Expand All @@ -459,7 +459,7 @@ impl<'a, C: Catalog> Planner<'a, C> {
};
// Add the hidden column to the projection.
child_scope.add_passthrough(scope, index, true);
hidden.push(Expression::Field(index, scope.get_label(index)));
hidden.push(Expression::Field(index));
true
});
}
Expand All @@ -473,7 +473,7 @@ impl<'a, C: Catalog> Planner<'a, C> {
// Look up aggregate functions or GROUP BY expressions. These were added
// to the parent scope when building the Aggregate node, if any.
if let Some(index) = scope.lookup_aggregate(&expr) {
return Ok(Field(index, scope.get_label(index)));
return Ok(Field(index));
}

// Helper for building a boxed expression.
Expand All @@ -490,8 +490,7 @@ impl<'a, C: Catalog> Planner<'a, C> {
ast::Literal::String(s) => Value::String(s),
}),
ast::Expression::Field(table, name) => {
let index = scope.lookup_column(table.as_deref(), &name)?;
Field(index, scope.get_label(index))
Field(scope.lookup_column(table.as_deref(), &name)?)
}
// Currently, all functions are aggregates, and processed above.
// TODO: consider adding some basic functions for fun.
Expand Down
31 changes: 15 additions & 16 deletions src/sql/types/expression.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@ use serde::{Deserialize, Serialize};
pub enum Expression {
/// A constant value.
Constant(Value),
/// A field reference (row index) with optional label. The label is only
/// used for display purposes.
Field(usize, Label),
/// A field reference (row index).
Field(usize),

/// Logical AND of two booleans: a AND b.
And(Box<Expression>, Box<Expression>),
Expand Down Expand Up @@ -65,7 +64,7 @@ impl Expression {
pub fn format(&self, node: &Node) -> String {
let format = |expr: &Expression| expr.format(node);
match self {
Self::Field(index, _) => match node.column_label(*index) {
Self::Field(index) => match node.column_label(*index) {
Label::None => format!("#{index}"),
label => format!("{label}"),
},
Expand Down Expand Up @@ -106,7 +105,7 @@ impl Expression {

// Field references look up a row value. The planner must make sure
// the field reference is valid.
Self::Field(i, _) => row.map(|row| row[*i].clone()).unwrap_or(Null),
Self::Field(i) => row.map(|row| row[*i].clone()).unwrap_or(Null),

// Logical AND. Inputs must be boolean or NULL. NULLs generally
// yield NULL, except the special case NULL AND false == false.
Expand Down Expand Up @@ -243,7 +242,7 @@ impl Expression {
| Self::Negate(expr)
| Self::Not(expr) => expr.walk(visitor),

Self::Constant(_) | Self::Field(_, _) => true,
Self::Constant(_) | Self::Field(_) => true,
}
}

Expand Down Expand Up @@ -289,7 +288,7 @@ impl Expression {
Self::Negate(expr) => Self::Negate(transform(expr)?),
Self::Not(expr) => Self::Not(transform(expr)?),

expr @ (Self::Constant(_) | Self::Field(_, _)) => expr,
expr @ (Self::Constant(_) | Self::Field(_)) => expr,
};
self = after(self)?;
Ok(self)
Expand Down Expand Up @@ -371,12 +370,12 @@ impl Expression {
// use index lookups. NULL and NaN won't return any matches, but we
// handle this in into_field_values().
Equal(lhs, rhs) => match (lhs.as_ref(), rhs.as_ref()) {
(Field(f, _), Constant(_)) | (Constant(_), Field(f, _)) => Some(*f),
(Field(f), Constant(_)) | (Constant(_), Field(f)) => Some(*f),
_ => None,
},
// IS NULL and IS NAN can use index lookups, since we index these.
IsNull(expr) | IsNaN(expr) => match expr.as_ref() {
Field(f, _) => Some(*f),
Field(f) => Some(*f),
_ => None,
},
// For OR branches, check if all branches are lookups on the same
Expand All @@ -397,20 +396,20 @@ impl Expression {
// NULL and NAN index lookups are for IS NULL and IS NAN.
// Equality comparisons with = shouldn't yield any results, so
// just return an empty value set for these.
(Field(f, _), Constant(v)) | (Constant(v), Field(f, _)) if v.is_undefined() => {
(Field(f), Constant(v)) | (Constant(v), Field(f)) if v.is_undefined() => {
Some((f, Vec::new()))
}
(Field(f, _), Constant(v)) | (Constant(v), Field(f, _)) => Some((f, vec![v])),
(Field(f), Constant(v)) | (Constant(v), Field(f)) => Some((f, vec![v])),
_ => None,
},
// IS NULL index lookups should look up NULL.
IsNull(expr) => match *expr {
Field(f, _) => Some((f, vec![Value::Null])),
Field(f) => Some((f, vec![Value::Null])),
_ => None,
},
// IS NAN index lookups should look up NAN.
IsNaN(expr) => match *expr {
Field(f, _) => Some((f, vec![Value::Float(f64::NAN)])),
Field(f) => Some((f, vec![Value::Float(f64::NAN)])),
_ => None,
},
Or(lhs, rhs) => match (lhs.into_field_values(), rhs.into_field_values()) {
Expand All @@ -424,9 +423,9 @@ impl Expression {
}

/// Replaces field references with the given field.
pub fn replace_field(self, from: usize, to: usize, label: &Label) -> Self {
pub fn replace_field(self, from: usize, to: usize) -> Self {
let transform = |expr| match expr {
Expression::Field(i, _) if i == from => Expression::Field(to, label.clone()),
Expression::Field(i) if i == from => Expression::Field(to),
expr => expr,
};
self.transform(&|e| Ok(transform(e)), &Ok).unwrap() // infallible
Expand All @@ -435,7 +434,7 @@ impl Expression {
/// Shifts any field indexes by the given amount.
pub fn shift_field(self, diff: isize) -> Self {
let transform = |expr| match expr {
Expression::Field(i, label) => Expression::Field((i as isize + diff) as usize, label),
Expression::Field(i) => Expression::Field((i as isize + diff) as usize),
expr => expr,
};
self.transform(&|e| Ok(transform(e)), &Ok).unwrap() // infallible
Expand Down

0 comments on commit f34b327

Please sign in to comment.