Merge 'Fix rowid generation' from Nikita Sivukhin

Fix panic in case when table has row with rowid equals to `-1`
(`=u64::max`)
```sql
limbo> CREATE TABLE t(x INTEGER PRIMARY KEY)
limbo> INSERT INTO t VALUES (-1)
limbo> INSERT INTO t VALUES (NULL);
thread 'main' panicked at core/vdbe/mod.rs:2499:21:
attempt to add with overflow
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

Reviewed-by: Pere Diaz Bou <pere-altea@homail.com>

Closes #868
This commit is contained in:
Pekka Enberg
2025-02-03 09:09:12 +02:00
2 changed files with 44 additions and 1 deletions

View File

@@ -2496,7 +2496,11 @@ fn get_new_rowid<R: Rng>(cursor: &mut BTreeCursor, mut rng: R) -> Result<CursorR
CursorResult::Ok(()) => {}
CursorResult::IO => return Ok(CursorResult::IO),
}
let mut rowid = cursor.rowid()?.unwrap_or(0) + 1;
let mut rowid = cursor
.rowid()?
.unwrap_or(0) // if BTree is empty - use 0 as initial value for rowid
.checked_add(1) // add 1 but be careful with overflows
.unwrap_or(u64::MAX); // in case of overflow - use u64::MAX
if rowid > i64::MAX.try_into().unwrap() {
let distribution = Uniform::from(1..=i64::MAX);
let max_attempts = 100;

View File

@@ -81,3 +81,42 @@ fn test_last_insert_rowid_basic() -> anyhow::Result<()> {
do_flush(&conn, &tmp_db)?;
Ok(())
}
#[test]
fn test_integer_primary_key() -> anyhow::Result<()> {
let _ = env_logger::try_init();
let tmp_db = TempDatabase::new("CREATE TABLE test_rowid (id INTEGER PRIMARY KEY);");
let conn = tmp_db.connect_limbo();
for query in &[
"INSERT INTO test_rowid VALUES (-1)",
"INSERT INTO test_rowid VALUES (NULL)",
] {
let mut insert_query = conn.query(query)?.unwrap();
loop {
match insert_query.step()? {
StepResult::IO => tmp_db.io.run_once()?,
StepResult::Done => break,
_ => unreachable!(),
}
}
}
let mut rowids = Vec::new();
let mut select_query = conn.query("SELECT * FROM test_rowid")?.unwrap();
loop {
match select_query.step()? {
StepResult::Row(row) => {
if let Value::Integer(id) = row.values[0] {
rowids.push(id);
}
}
StepResult::IO => tmp_db.io.run_once()?,
StepResult::Interrupt | StepResult::Done => break,
StepResult::Busy => panic!("Database is busy"),
}
}
assert_eq!(rowids.len(), 2);
assert!(rowids[0] > 0);
assert!(rowids[1] == -1);
Ok(())
}