diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index fc55eee0e..4a73b4b53 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -3679,13 +3679,17 @@ pub fn op_agg_step( match acc_i.checked_add(i) { Some(sum) => *acc = Value::Integer(sum), None => { - // Overflow -> switch to float with KBN summation - let acc_f = *acc_i as f64; - *acc = Value::Float(acc_f); - sum_state.approx = true; - sum_state.ovrfl = true; + if matches!(func, AggFunc::Total) { + // Total() never throw an integer overflow -> switch to float with KBN summation + let acc_f = *acc_i as f64; + *acc = Value::Float(acc_f); + sum_state.approx = true; + sum_state.ovrfl = true; - apply_kbn_step_int(acc, i, sum_state); + apply_kbn_step_int(acc, i, sum_state); + } else { + return Err(LimboError::IntegerOverflow); + } } } } diff --git a/tests/integration/functions/mod.rs b/tests/integration/functions/mod.rs index b31ed8f53..3ba68c833 100644 --- a/tests/integration/functions/mod.rs +++ b/tests/integration/functions/mod.rs @@ -1,3 +1,4 @@ mod test_cdc; mod test_function_rowid; +mod test_sum; mod test_wal_api; diff --git a/tests/integration/functions/test_sum.rs b/tests/integration/functions/test_sum.rs new file mode 100644 index 000000000..0b03a14c7 --- /dev/null +++ b/tests/integration/functions/test_sum.rs @@ -0,0 +1,63 @@ +use crate::common::{limbo_exec_rows, limbo_exec_rows_fallible, sqlite_exec_rows, TempDatabase}; +use turso_core::LimboError; + +#[test] +fn sum_errors_on_integer_overflow() { + let _ = env_logger::try_init(); + + let tmp_db = TempDatabase::new_empty(false); + let conn = tmp_db.connect_limbo(); + let sqlite_conn = rusqlite::Connection::open_in_memory().unwrap(); + + limbo_exec_rows(&tmp_db, &conn, "CREATE TABLE t(a)"); + sqlite_exec_rows(&sqlite_conn, "CREATE TABLE t(a)"); + + limbo_exec_rows(&tmp_db, &conn, "INSERT INTO t VALUES (9223372036854775807)"); + sqlite_exec_rows(&sqlite_conn, "INSERT INTO t VALUES (9223372036854775807)"); + + let limbo_before_overflow = limbo_exec_rows(&tmp_db, &conn, "SELECT sum(a) FROM t"); + let sqlite_before_overflow = sqlite_exec_rows(&sqlite_conn, "SELECT sum(a) FROM t"); + assert_eq!(limbo_before_overflow, sqlite_before_overflow); + + limbo_exec_rows(&tmp_db, &conn, "INSERT INTO t VALUES (1)"); + sqlite_exec_rows(&sqlite_conn, "INSERT INTO t VALUES (1)"); + + let err = limbo_exec_rows_fallible(&tmp_db, &conn, "SELECT sum(a) FROM t") + .expect_err("SUM should report integer overflow"); + assert!(matches!(err, LimboError::IntegerOverflow)); + + use rusqlite::ffi::Error as FfiError; + use rusqlite::Error::SqliteFailure; + use rusqlite::ErrorCode; + + let mut stmt = sqlite_conn + .prepare("SELECT sum(a) FROM t") + .expect("prepare should succeed"); + let mut rows = stmt.query([]).expect("query should start"); + let sqlite_err = loop { + match rows.next() { + Ok(Some(_)) => continue, + Ok(None) => panic!("expected overflow error"), + Err(err) => break err, + } + }; + match sqlite_err { + SqliteFailure( + FfiError { + code: ErrorCode::Unknown, + .. + }, + Some(message), + ) => { + assert_eq!(message, "integer overflow"); + } + SqliteFailure( + FfiError { + code: ErrorCode::Unknown, + .. + }, + None, + ) => {} + other => panic!("unexpected sqlite error: {other:?}"), + } +}