Skip to content

Commit

Permalink
Merge 'Fix MustBeInt opcode semantics' from Vrishabh
Browse files Browse the repository at this point in the history
In sqlite3 , we can create primary key with only integer column and
during insert we can pass any value which can be parsed into integer as
shown below. When I tried the same with limbo, it failed. The cause of
this was due to MustBeInt opcode behaviour not aligned with sqlite. This
PR aims to fix it.
Sqlite output
```
SQLite version 3.45.3
sqlite> create table temp (t1 integer, primary key (t1));
sqlite> insert into temp values (1),(2.0),('3'),('4.0');
sqlite> select * from temp;
1
2
3
4
```
Limbo output main branch
```
limbo>     create table temp (t1 integer, primary key (t1));
limbo>     insert into temp values (1),(2.0),('3'),('4.0');
Parse error: MustBeInt: the value in the register is not an integer
limbo>     select * from temp;
1
```
Limbo output with this PR
```
limbo> create table temp (t1 integer, primary key (t1));
limbo> insert into temp values (1),(2.0),('3'),('4.0');
limbo> select * from temp;
1
2
3
4
```

Closes #706
  • Loading branch information
penberg committed Jan 15, 2025
2 parents f7af815 + 845de12 commit b589203
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 12 deletions.
51 changes: 40 additions & 11 deletions core/vdbe/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2038,11 +2038,25 @@ impl Program {
state.pc += 1;
}
Insn::MustBeInt { reg } => {
match state.registers[*reg] {
match &state.registers[*reg] {
OwnedValue::Integer(_) => {}
OwnedValue::Float(f) => match cast_real_to_integer(*f) {
Ok(i) => state.registers[*reg] = OwnedValue::Integer(i),
Err(_) => crate::bail_parse_error!(
"MustBeInt: the value in register cannot be cast to integer"
),
},
OwnedValue::Text(text) => match checked_cast_text_to_numeric(&text.value) {
Ok(OwnedValue::Integer(i)) => {
state.registers[*reg] = OwnedValue::Integer(i)
}
_ => crate::bail_parse_error!(
"MustBeInt: the value in register cannot be cast to integer"
),
},
_ => {
crate::bail_parse_error!(
"MustBeInt: the value in the register is not an integer"
"MustBeInt: the value in register cannot be cast to integer"
);
}
};
Expand Down Expand Up @@ -3079,23 +3093,38 @@ fn cast_text_to_real(text: &str) -> OwnedValue {
/// IEEE 754 64-bit float and thus provides a 1-bit of margin for the text-to-float conversion operation.)
/// Any text input that describes a value outside the range of a 64-bit signed integer yields a REAL result.
/// Casting a REAL or INTEGER value to NUMERIC is a no-op, even if a real value could be losslessly converted to an integer.
fn cast_text_to_numeric(text: &str) -> OwnedValue {
fn checked_cast_text_to_numeric(text: &str) -> std::result::Result<OwnedValue, ()> {
if !text.contains('.') && !text.contains('e') && !text.contains('E') {
// Looks like an integer
if let Ok(i) = text.parse::<i64>() {
return OwnedValue::Integer(i);
return Ok(OwnedValue::Integer(i));
}
}
// Try as float
if let Ok(f) = text.parse::<f64>() {
// Check if can be losslessly converted to 51-bit integer
let i = f as i64;
if f == i as f64 && i.abs() < (1i64 << 51) {
return OwnedValue::Integer(i);
}
return OwnedValue::Float(f);
return match cast_real_to_integer(f) {
Ok(i) => Ok(OwnedValue::Integer(i)),
Err(_) => Ok(OwnedValue::Float(f)),
};
}
OwnedValue::Integer(0)
Err(())
}

// try casting to numeric if not possible return integer 0
fn cast_text_to_numeric(text: &str) -> OwnedValue {
match checked_cast_text_to_numeric(text) {
Ok(value) => value,
Err(_) => OwnedValue::Integer(0),
}
}

// Check if float can be losslessly converted to 51-bit integer
fn cast_real_to_integer(float: f64) -> std::result::Result<i64, ()> {
let i = float as i64;
if float == i as f64 && i.abs() < (1i64 << 51) {
return Ok(i);
}
Err(())
}

fn execute_sqlite_version(version_integer: i64) -> String {
Expand Down
11 changes: 10 additions & 1 deletion testing/insert.test
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,13 @@ do_execsql_test_on_specific_db {:memory:} basic-insert {
create table temp (t1 integer, primary key (t1));
insert into temp values (1);
select * from temp;
} {1}
} {1}

do_execsql_test_on_specific_db {:memory:} must-be-int-insert {
create table temp (t1 integer, primary key (t1));
insert into temp values (1),(2.0),('3'),('4.0');
select * from temp;
} {1
2
3
4}

0 comments on commit b589203

Please sign in to comment.