aljaz/turso
1
0
Fork 0
mirror of https://github.com/aljazceru/turso.git synced 2026-01-08 02:34:20 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge 'Fix logical codegen' from Nikita Sivukhin

Browse Source 
Fix few logical codegen issues and add fuzz tests for logical
expressions
-  Right now Limbo fails to recognize `false` constant in case when any
unary operator is used on the AST path. This PR add unary operator
option in the rewrite code and handle such cases properly.
```sql
limbo> SELECT NOT FALSE;

  × Parse error: no such column: FALSE - should this be a string literal in single-quotes?

```
- `ifnull` implementation produced incorrect codegen due to "careless"
management of registers
```
limbo> SELECT ifnull(0, NOT 0)
[NULL here]
```
- `like` implementation produced incorrect codegen due to "careless"
management of registers
```
limbo> SELECT like('a%', 'a') = 1;
thread 'main' panicked at core/vdbe/mod.rs:1902:41:
internal error: entered unreachable code: Like on non-text registers
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
Depends on https://github.com/tursodatabase/limbo/pull/867 (need
`GrammarGenerator` from this branch)

Closes #869
This commit is contained in:
Jussi Saurio
2025-02-03 12:39:41 +02:00
parent d41dfd0c5d f43a326649
commit d4cb0a1223
3 changed files with 146 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
core/translate/expr.rs
View File

@@ -1146,9 +1146,10 @@ pub fn translate_expr(
temp_reg,
resolver,
)?;
let before_copy_label = program.allocate_label();
program.emit_insn(Insn::NotNull {
reg: temp_reg,
target_pc: program.offset().add(2u32),
target_pc: before_copy_label,
});
translate_expr(
@@ -1158,6 +1159,7 @@ pub fn translate_expr(
temp_reg,
resolver,
)?;
program.resolve_label(before_copy_label, program.offset());
program.emit_insn(Insn::Copy {
src_reg: temp_reg,
dst_reg: target_register,
@@ -1225,15 +1227,21 @@ pub fn translate_expr(
srf.to_string()
);
};
for arg in args {
let _ =
translate_and_mark(program, referenced_tables, arg, resolver);
let func_registers = program.alloc_registers(args.len());
for (i, arg) in args.iter().enumerate() {
let _ = translate_expr(
program,
referenced_tables,
arg,
func_registers + i,
resolver,
)?;
}
program.emit_insn(Insn::Function {
// Only constant patterns for LIKE are supported currently, so this
// is always 1
constant_mask: 1,
start_reg: target_register + 1,
start_reg: func_registers,
dest: target_register,
func: func_ctx,
});

4
core/translate/optimizer.rs
View File

@@ -686,6 +686,10 @@ fn rewrite_expr(expr: &mut ast::Expr) -> Result<()> {
}
Ok(())
}
ast::Expr::Unary(_, arg) => {
rewrite_expr(arg)?;
Ok(())
}
_ => Ok(()),
}
}

130
tests/integration/fuzz/mod.rs
View File

@@ -10,7 +10,7 @@ mod tests {
use crate::{
common::TempDatabase,
fuzz::grammar_generator::{rand_int, GrammarGenerator},
fuzz::grammar_generator::{rand_int, rand_str, GrammarGenerator},
};
fn rng_from_time() -> (ChaCha8Rng, u64) {
@@ -146,4 +146,132 @@ mod tests {
);
}
}
#[test]
pub fn logical_expression_fuzz_ex1() {
let _ = env_logger::try_init();
let db = TempDatabase::new_empty();
let limbo_conn = db.connect_limbo();
let sqlite_conn = rusqlite::Connection::open_in_memory().unwrap();
for query in [
"SELECT FALSE",
"SELECT NOT FALSE",
"SELECT ((NULL) IS NOT TRUE <= ((NOT (FALSE))))",
"SELECT ifnull(0, NOT 0)",
"SELECT like('a%', 'a') = 1",
] {
let limbo = limbo_exec_row(&limbo_conn, query);
let sqlite = sqlite_exec_row(&sqlite_conn, query);
assert_eq!(
limbo, sqlite,
"query: {}, limbo: {:?}, sqlite: {:?}",
query, limbo, sqlite
);
}
}
#[test]
pub fn logical_expression_fuzz_run() {
let _ = env_logger::try_init();
let g = GrammarGenerator::new();
let (expr, expr_builder) = g.create_handle();
let (bin_op, bin_op_builder) = g.create_handle();
let (unary_infix_op, unary_infix_op_builder) = g.create_handle();
let (scalar, scalar_builder) = g.create_handle();
let (paren, paren_builder) = g.create_handle();
paren_builder
.concat("")
.push_str("(")
.push(expr)
.push_str(")")
.build();
unary_infix_op_builder
.concat(" ")
.push(g.create().choice().options_str(["NOT"]).build())
.push(expr)
.build();
bin_op_builder
.concat(" ")
.push(expr)
.push(
g.create()
.choice()
.options_str(["AND", "OR", "IS", "IS NOT", "=", "<>", ">", "<", ">=", "<="])
.build(),
)
.push(expr)
.build();
scalar_builder
.choice()
.option(
g.create()
.concat("")
.push_str("like('")
.push_symbol(rand_str("", 2))
.push_str("', '")
.push_symbol(rand_str("", 2))
.push_str("')")
.build(),
)
.option(
g.create()
.concat("")
.push_str("ifnull(")
.push(expr)
.push_str(",")
.push(expr)
.push_str(")")
.build(),
)
.option(
g.create()
.concat("")
.push_str("iif(")
.push(expr)
.push_str(",")
.push(expr)
.push_str(",")
.push(expr)
.push_str(")")
.build(),
)
.build();
expr_builder
.choice()
.option_w(unary_infix_op, 1.0)
.option_w(bin_op, 1.0)
.option_w(paren, 1.0)
.option_w(scalar, 1.0)
// unfortunatelly, sqlite behaves weirdly when IS operator is used with TRUE/FALSE constants
// e.g. 8 IS TRUE == 1 (although 8 = TRUE == 0)
// so, we do not use TRUE/FALSE constants as they will produce diff with sqlite results
.options_str(["1", "0", "NULL", "2.0", "1.5", "-0.5", "-2.0", "(1 / 0)"])
.build();
let sql = g.create().concat(" ").push_str("SELECT").push(expr).build();
let db = TempDatabase::new_empty();
let limbo_conn = db.connect_limbo();
let sqlite_conn = rusqlite::Connection::open_in_memory().unwrap();
let (mut rng, seed) = rng_from_time();
log::info!("seed: {}", seed);
for _ in 0..1024 {
let query = g.generate(&mut rng, sql, 50);
log::info!("query: {}", query);
let limbo = limbo_exec_row(&limbo_conn, &query);
let sqlite = sqlite_exec_row(&sqlite_conn, &query);
assert_eq!(
limbo, sqlite,
"query: {}, limbo: {:?}, sqlite: {:?}",
query, limbo, sqlite
);
}
}
}