From cc79ff5cfd7232c7d5e2858e33083cd0cb8fe7c2 Mon Sep 17 00:00:00 2001 From: jussisaurio Date: Tue, 23 Jul 2024 23:10:36 +0300 Subject: [PATCH] Fix #205: agg functions on text columns --- core/types.rs | 32 +++++++++++++------ core/vdbe/mod.rs | 64 +++++++++++++++++++++++++++----------- testing/agg-functions.test | 25 +++++++++++++-- 3 files changed, 91 insertions(+), 30 deletions(-) diff --git a/core/types.rs b/core/types.rs index 137f8e8fd..4eabbb10a 100644 --- a/core/types.rs +++ b/core/types.rs @@ -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), @@ -61,8 +61,8 @@ pub enum AggContext { Avg(OwnedValue, OwnedValue), // acc and count Sum(OwnedValue), Count(OwnedValue), - Max(OwnedValue), - Min(OwnedValue), + Max(Option), + Min(Option), GroupConcat(OwnedValue), } @@ -201,7 +201,7 @@ impl std::ops::Div for OwnedValue { (OwnedValue::Float(float_left), OwnedValue::Float(float_right)) => { OwnedValue::Float(float_left / float_right) } - _ => unreachable!(), + _ => OwnedValue::Float(0.0), } } } @@ -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!(), diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index c103b770b..345bd0b1c 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -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!(); } @@ -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!(); } @@ -807,9 +813,12 @@ 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 { @@ -817,13 +826,21 @@ impl Program { } } ( - 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"); } @@ -839,9 +856,12 @@ 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 { @@ -849,13 +869,21 @@ impl Program { } } ( - 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"); } diff --git a/testing/agg-functions.test b/testing/agg-functions.test index 9e83b545e..6410b1425 100755 --- a/testing/agg-functions.test +++ b/testing/agg-functions.test @@ -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; @@ -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} @@ -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} -