Skip to content

Commit

Permalink
Merge pull request #208 from jussisaurio/fix-agg-functions-on-text
Browse files Browse the repository at this point in the history
  • Loading branch information
@penberg
penberg authored Jul 23, 2024
2 parents ee0398b + cc79ff5 commit e3fea5c
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 30 deletions.
32 changes: 23 additions & 9 deletions core/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ impl Display for OwnedValue {
AggContext::Avg(acc, _count) => write!(f, "{}", acc),
AggContext::Sum(acc) => write!(f, "{}", acc),
AggContext::Count(count) => write!(f, "{}", count),
AggContext::Max(max) => write!(f, "{}", max),
AggContext::Min(min) => write!(f, "{}", min),
AggContext::Max(max) => write!(f, "{}", max.as_ref().unwrap_or(&OwnedValue::Null)),
AggContext::Min(min) => write!(f, "{}", min.as_ref().unwrap_or(&OwnedValue::Null)),
AggContext::GroupConcat(s) => write!(f, "{}", s),
},
OwnedValue::Record(r) => write!(f, "{:?}", r),
Expand All @@ -61,8 +61,8 @@ pub enum AggContext {
Avg(OwnedValue, OwnedValue), // acc and count
Sum(OwnedValue),
Count(OwnedValue),
Max(OwnedValue),
Min(OwnedValue),
Max(Option<OwnedValue>),
Min(Option<OwnedValue>),
GroupConcat(OwnedValue),
}

Expand Down Expand Up @@ -201,7 +201,7 @@ impl std::ops::Div<OwnedValue> for OwnedValue {
(OwnedValue::Float(float_left), OwnedValue::Float(float_right)) => {
OwnedValue::Float(float_left / float_right)
}
_ => unreachable!(),
_ => OwnedValue::Float(0.0),
}
}
}
Expand All @@ -220,11 +220,25 @@ pub fn to_value(value: &OwnedValue) -> Value<'_> {
OwnedValue::Text(s) => Value::Text(s),
OwnedValue::Blob(b) => Value::Blob(b),
OwnedValue::Agg(a) => match a.as_ref() {
AggContext::Avg(acc, _count) => to_value(acc), // we assume aggfinal was called
AggContext::Sum(acc) => to_value(acc),
AggContext::Avg(acc, _count) => match acc {
OwnedValue::Integer(i) => Value::Integer(*i),
OwnedValue::Float(f) => Value::Float(*f),
_ => Value::Float(0.0),
},
AggContext::Sum(acc) => match acc {
OwnedValue::Integer(i) => Value::Integer(*i),
OwnedValue::Float(f) => Value::Float(*f),
_ => Value::Float(0.0),
},
AggContext::Count(count) => to_value(count),
AggContext::Max(max) => to_value(max),
AggContext::Min(min) => to_value(min),
AggContext::Max(max) => match max {
Some(max) => to_value(max),
None => Value::Null,
},
AggContext::Min(min) => match min {
Some(min) => to_value(min),
None => Value::Null,
},
AggContext::GroupConcat(s) => to_value(s),
},
OwnedValue::Record(_) => todo!(),
Expand Down
64 changes: 46 additions & 18 deletions core/vdbe/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -733,12 +733,15 @@ impl Program {
AggFunc::Max => {
let col = state.registers[*col].clone();
match col {
OwnedValue::Integer(_) => OwnedValue::Agg(Box::new(
AggContext::Max(OwnedValue::Integer(i64::MIN)),
)),
OwnedValue::Float(_) => OwnedValue::Agg(Box::new(
AggContext::Max(OwnedValue::Float(f64::NEG_INFINITY)),
)),
OwnedValue::Integer(_) => {
OwnedValue::Agg(Box::new(AggContext::Max(None)))
}
OwnedValue::Float(_) => {
OwnedValue::Agg(Box::new(AggContext::Max(None)))
}
OwnedValue::Text(_) => {
OwnedValue::Agg(Box::new(AggContext::Max(None)))
}
_ => {
unreachable!();
}
Expand All @@ -747,12 +750,15 @@ impl Program {
AggFunc::Min => {
let col = state.registers[*col].clone();
match col {
OwnedValue::Integer(_) => OwnedValue::Agg(Box::new(
AggContext::Min(OwnedValue::Integer(i64::MAX)),
)),
OwnedValue::Float(_) => OwnedValue::Agg(Box::new(
AggContext::Min(OwnedValue::Float(f64::INFINITY)),
)),
OwnedValue::Integer(_) => {
OwnedValue::Agg(Box::new(AggContext::Min(None)))
}
OwnedValue::Float(_) => {
OwnedValue::Agg(Box::new(AggContext::Min(None)))
}
OwnedValue::Text(_) => {
OwnedValue::Agg(Box::new(AggContext::Min(None)))
}
_ => {
unreachable!();
}
Expand Down Expand Up @@ -807,23 +813,34 @@ impl Program {
unreachable!();
};

match (acc, col) {
match (acc.as_mut(), col) {
(None, value) => {
*acc = Some(value);
}
(
OwnedValue::Integer(ref mut current_max),
Some(OwnedValue::Integer(ref mut current_max)),
OwnedValue::Integer(value),
) => {
if value > *current_max {
*current_max = value;
}
}
(
OwnedValue::Float(ref mut current_max),
Some(OwnedValue::Float(ref mut current_max)),
OwnedValue::Float(value),
) => {
if value > *current_max {
*current_max = value;
}
}
(
Some(OwnedValue::Text(ref mut current_max)),
OwnedValue::Text(value),
) => {
if value > *current_max {
*current_max = value;
}
}
_ => {
eprintln!("Unexpected types in max aggregation");
}
Expand All @@ -839,23 +856,34 @@ impl Program {
unreachable!();
};

match (acc, col) {
match (acc.as_mut(), col) {
(None, value) => {
*acc.borrow_mut() = Some(value);
}
(
OwnedValue::Integer(ref mut current_min),
Some(OwnedValue::Integer(ref mut current_min)),
OwnedValue::Integer(value),
) => {
if value < *current_min {
*current_min = value;
}
}
(
OwnedValue::Float(ref mut current_min),
Some(OwnedValue::Float(ref mut current_min)),
OwnedValue::Float(value),
) => {
if value < *current_min {
*current_min = value;
}
}
(
Some(OwnedValue::Text(ref mut current_min)),
OwnedValue::Text(value),
) => {
if value < *current_min {
*current_min = value;
}
}
_ => {
eprintln!("Unexpected types in min aggregation");
}
Expand Down
25 changes: 22 additions & 3 deletions testing/agg-functions.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,25 @@ do_execsql_test select-avg {
SELECT avg(age) FROM users;
} {50.396}

do_execsql_test select-avg-text {
SELECT avg(first_name) FROM users;
} {0.0}

do_execsql_test select-sum {
SELECT sum(age) FROM users;
} {503960}

do_execsql_test select-sum-text {
SELECT sum(first_name) FROM users;
} {0.0}

do_execsql_test select-total {
SELECT sum(age) FROM users;
} {503960}
SELECT total(age) FROM users;
} {503960.0}

do_execsql_test select-total-text {
SELECT total(first_name) FROM users WHERE id < 3;
} {0.0}

do_execsql_test select-limit {
SELECT id FROM users LIMIT 1;
Expand All @@ -31,6 +43,14 @@ do_execsql_test select-min {
SELECT min(age) FROM users;
} {1}

do_execsql_test select-max-text {
SELECT max(first_name) FROM users;
} {Zoe}

do_execsql_test select-min-text {
SELECT min(first_name) FROM users;
} {Aaron}

do_execsql_test select-group-concat {
SELECT group_concat(name) FROM products;
} {hat,cap,shirt,sweater,sweatshirt,shorts,jeans,sneakers,boots,coat,accessories}
Expand All @@ -50,4 +70,3 @@ do_execsql_test select-string-agg-with-delimiter {
do_execsql_test select-string-agg-with-column-delimiter {
SELECT string_agg(name, id) FROM products;
} {hat2cap3shirt4sweater5sweatshirt6shorts7jeans8sneakers9boots10coat11accessories}

0 comments on commit e3fea5c

Please sign in to comment.