Skip to content

Commit

Permalink
Merge 'Fix SELECT ABS(-9223372036854775808) causes limbo to panic. …
Browse files Browse the repository at this point in the history
…' from Krishna Vishal

Now we return `RuntimeError`.  Matches SQLite behavior.
SQLite:
```sql
sqlite> SELECT ABS(-9223372036854775808);
Runtime error: integer overflow
```
Limbo after this fix:
```sql
limbo> SELECT ABS(-9223372036854775808);
Runtime error: integer overflow
```
Closes #815

Closes #818
  • Loading branch information
penberg committed Jan 30, 2025
2 parents 4673ac9 + f8c1828 commit a2ac313
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 10 deletions.
2 changes: 2 additions & 0 deletions core/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ pub enum LimboError {
ExtensionError(String),
#[error("Unbound parameter at index {0}")]
Unbound(NonZero<usize>),
#[error("Runtime error: integer overflow")]
IntegerOverflow,
}

#[macro_export]
Expand Down
24 changes: 14 additions & 10 deletions core/vdbe/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1893,7 +1893,7 @@ impl Program {
let reg_value = state.registers[*start_reg].borrow_mut();
let result = match scalar_func {
ScalarFunc::Sign => exec_sign(reg_value),
ScalarFunc::Abs => exec_abs(reg_value),
ScalarFunc::Abs => Some(exec_abs(reg_value)?),
ScalarFunc::Lower => exec_lower(reg_value),
ScalarFunc::Upper => exec_upper(reg_value),
ScalarFunc::Length => Some(exec_length(reg_value)),
Expand Down Expand Up @@ -2718,24 +2718,25 @@ pub fn exec_soundex(reg: &OwnedValue) -> OwnedValue {
OwnedValue::build_text(Rc::new(result.to_uppercase()))
}

fn exec_abs(reg: &OwnedValue) -> Option<OwnedValue> {
fn exec_abs(reg: &OwnedValue) -> Result<OwnedValue> {
match reg {
OwnedValue::Integer(x) => {
if x < &0 {
Some(OwnedValue::Integer(-x))
} else {
Some(OwnedValue::Integer(*x))
match i64::checked_abs(*x) {
Some(y) => Ok(OwnedValue::Integer(y)),
// Special case: if we do the abs of "-9223372036854775808", it causes overflow.
// return IntegerOverflow error
None => Err(LimboError::IntegerOverflow),
}
}
OwnedValue::Float(x) => {
if x < &0.0 {
Some(OwnedValue::Float(-x))
Ok(OwnedValue::Float(-x))
} else {
Some(OwnedValue::Float(*x))
Ok(OwnedValue::Float(*x))
}
}
OwnedValue::Null => Some(OwnedValue::Null),
_ => Some(OwnedValue::Float(0.0)),
OwnedValue::Null => Ok(OwnedValue::Null),
_ => Ok(OwnedValue::Float(0.0)),
}
}

Expand Down Expand Up @@ -3768,6 +3769,9 @@ mod tests {
OwnedValue::Float(0.0)
);
assert_eq!(exec_abs(&OwnedValue::Null).unwrap(), OwnedValue::Null);

// ABS(i64::MIN) should return RuntimeError
assert!(exec_abs(&OwnedValue::Integer(i64::MIN)).is_err());
}

#[test]
Expand Down

0 comments on commit a2ac313

Please sign in to comment.