From 005e8ba91a731974fa1948f51218e34fff9d09ea Mon Sep 17 00:00:00 2001 From: krishvishal Date: Sat, 22 Feb 2025 03:10:52 +0530 Subject: [PATCH 1/7] Add support for `VALUES` statement. Fixes: /~https://github.com/tursodatabase/limbo/issues/866 --- core/translate/mod.rs | 1 + core/translate/select.rs | 55 +++++++++++++++++++++++--- core/translate/values.rs | 85 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 core/translate/values.rs diff --git a/core/translate/mod.rs b/core/translate/mod.rs index 3ee8f4ce0..f0b205fd5 100644 --- a/core/translate/mod.rs +++ b/core/translate/mod.rs @@ -23,6 +23,7 @@ pub(crate) mod result_row; pub(crate) mod select; pub(crate) mod subquery; pub(crate) mod transaction; +pub(crate) mod values; use crate::schema::Schema; use crate::storage::pager::Pager; diff --git a/core/translate/select.rs b/core/translate/select.rs index fe7656f16..4fa9ab567 100644 --- a/core/translate/select.rs +++ b/core/translate/select.rs @@ -1,6 +1,7 @@ use super::emitter::emit_program; use super::plan::{select_star, Operation, Search, SelectQueryType}; use super::planner::Scope; +use super::values::emit_values; use crate::function::{AggFunc, ExtFunc, Func}; use crate::translate::optimizer::optimize_plan; use crate::translate::plan::{Aggregate, Direction, GroupBy, Plan, ResultSetColumn, SelectPlan}; @@ -21,18 +22,24 @@ pub fn translate_select( select: ast::Select, syms: &SymbolTable, ) -> Result { - let mut select_plan = prepare_select_plan(schema, select, syms, None)?; + let mut select_plan = prepare_select_plan(schema, select.clone(), syms, None)?; optimize_plan(&mut select_plan, schema)?; - let Plan::Select(ref select) = select_plan else { + let Plan::Select(ref select_ref) = select_plan else { panic!("select_plan is not a SelectPlan"); }; let mut program = ProgramBuilder::new(ProgramBuilderOpts { query_mode, - num_cursors: count_plan_required_cursors(select), - approx_num_insns: estimate_num_instructions(select), - approx_num_labels: estimate_num_labels(select), + num_cursors: count_plan_required_cursors(select_ref), + approx_num_insns: estimate_num_instructions(select_ref), + approx_num_labels: estimate_num_labels(select_ref), }); + + if let ast::OneSelect::Values(values) = &select.body.select.as_ref() { + emit_values(&mut program, &values)?; + return Ok(program); + } + emit_program(&mut program, select_plan, syms)?; Ok(program) } @@ -375,7 +382,43 @@ pub fn prepare_select_plan<'a>( // Return the unoptimized query plan Ok(Plan::Select(plan)) } - _ => todo!(), + ast::OneSelect::Values(values) => { + // New VALUES handling + if values.is_empty() { + crate::bail_parse_error!("VALUES clause cannot be empty"); + } + + let first_row_len = values[0].len(); + if first_row_len == 0 { + crate::bail_parse_error!("VALUES rows must have at least one column"); + } + + // Create result columns from first row + let mut result_columns = Vec::with_capacity(first_row_len); + for i in 0..first_row_len { + result_columns.push(ResultSetColumn { + expr: values[0][i].clone(), + alias: None, + contains_aggregates: false, + }); + } + + // Create minimal plan since VALUES just returns constant rows + let plan = SelectPlan { + table_references: vec![], // No tables needed + result_columns, + where_clause: vec![], + group_by: None, + order_by: None, + aggregates: vec![], + limit: None, + offset: None, + contains_constant_false_condition: false, + query_type: SelectQueryType::TopLevel, + }; + + Ok(Plan::Select(plan)) + } } } diff --git a/core/translate/values.rs b/core/translate/values.rs new file mode 100644 index 000000000..e97624538 --- /dev/null +++ b/core/translate/values.rs @@ -0,0 +1,85 @@ +use crate::{ + vdbe::{builder::ProgramBuilder, insn::Insn, BranchOffset}, + LimboError, Result, +}; +use limbo_sqlite3_parser::ast::{self, Literal}; + +pub fn emit_values(program: &mut ProgramBuilder, values: &[Vec]) -> Result<()> { + let goto_target = program.allocate_label(); + program.emit_insn(Insn::Init { + target_pc: goto_target, + }); + + let start_reg = 1; + let first_row_len = values[0].len(); + + for row in values { + if row.len() != first_row_len { + return Err(LimboError::ParseError( + "all VALUES rows must have the same number of values".into(), + )); + } + + for (i, expr) in row.iter().enumerate() { + let reg = start_reg + i; + + match expr { + ast::Expr::Literal(lit) => match lit { + Literal::String(s) => { + program.emit_insn(Insn::String8 { + value: s.clone(), + dest: reg, + }); + } + Literal::Numeric(num) => { + if let Ok(int_val) = num.parse::() { + program.emit_insn(Insn::Integer { + value: int_val, + dest: reg, + }); + } else { + let float_val = num.parse::()?; + program.emit_insn(Insn::Real { + value: float_val, + dest: reg, + }); + } + } + Literal::Null => { + program.emit_insn(Insn::Null { + dest: reg, + dest_end: None, + }); + } + _ => { + return Err(LimboError::ParseError( + "unsupported literal type in VALUES".into(), + )) + } + }, + _ => { + return Err(LimboError::ParseError( + "VALUES only supports literal values".into(), + )) + } + } + } + + program.emit_insn(Insn::ResultRow { + start_reg, + count: first_row_len, + }); + } + + program.emit_insn(Insn::Halt { + err_code: 0, + description: String::new(), + }); + + program.preassign_label_to_next_insn(goto_target); + program.emit_insn(Insn::Goto { + target_pc: BranchOffset::Offset(1), + }); + + Ok(()) +} From c347839017f92e1ea69f60d1aeb4cb99df5eb154 Mon Sep 17 00:00:00 2001 From: krishvishal Date: Sat, 22 Feb 2025 03:15:02 +0530 Subject: [PATCH 2/7] Allocate +1 extra register. --- core/translate/select.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/core/translate/select.rs b/core/translate/select.rs index 4fa9ab567..48042b794 100644 --- a/core/translate/select.rs +++ b/core/translate/select.rs @@ -36,6 +36,7 @@ pub fn translate_select( }); if let ast::OneSelect::Values(values) = &select.body.select.as_ref() { + program.alloc_registers(values[0].len() + 1); emit_values(&mut program, &values)?; return Ok(program); } From 79724e99bf35d016b30968c8c907c621362e1d38 Mon Sep 17 00:00:00 2001 From: krishvishal Date: Sun, 23 Feb 2025 14:44:11 +0530 Subject: [PATCH 3/7] Add tests for `VALUES` and support for edge cases, more coverage and SQLite compability. --- core/translate/values.rs | 43 +++++++++++++++++++++++++++++++---- tests/integration/fuzz/mod.rs | 37 ++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/core/translate/values.rs b/core/translate/values.rs index e97624538..b277294b5 100644 --- a/core/translate/values.rs +++ b/core/translate/values.rs @@ -22,12 +22,13 @@ pub fn emit_values(program: &mut ProgramBuilder, values: &[Vec]) -> R for (i, expr) in row.iter().enumerate() { let reg = start_reg + i; - match expr { ast::Expr::Literal(lit) => match lit { Literal::String(s) => { + let s = &s[1..s.len()-1]; + let s = s.replace("''", "'"); program.emit_insn(Insn::String8 { - value: s.clone(), + value: s, dest: reg, }); } @@ -57,9 +58,43 @@ pub fn emit_values(program: &mut ProgramBuilder, values: &[Vec]) -> R )) } }, + ast::Expr::Unary(op, expr) => { + match (&op, expr.as_ref()) { + (ast::UnaryOperator::Negative | ast::UnaryOperator::Positive, ast::Expr::Literal(Literal::Numeric(numeric_value))) => { + let multiplier = if let ast::UnaryOperator::Negative = op { -1 } else { 1 }; + + // Special case: check for negating i64::MAX+1 to get i64::MIN + if multiplier == -1 && numeric_value == "9223372036854775808" { + program.emit_insn(Insn::Integer { + value: i64::MIN, + dest: reg, + }); + } else { + let maybe_int = numeric_value.parse::(); + if let Ok(value) = maybe_int { + program.emit_insn(Insn::Integer { + value: value * multiplier, + dest: reg, + }); + } else { + let value = numeric_value.parse::()?; + program.emit_insn(Insn::Real { + value: value * multiplier as f64, + dest: reg, + }); + } + } + }, + _ => { + return Err(LimboError::ParseError( + "VALUES only supports literal values and unary on numbers".into(), + )) + } + } + }, _ => { return Err(LimboError::ParseError( - "VALUES only supports literal values".into(), + "VALUES only supports literal values and unary operations".into(), )) } } @@ -82,4 +117,4 @@ pub fn emit_values(program: &mut ProgramBuilder, values: &[Vec]) -> R }); Ok(()) -} +} \ No newline at end of file diff --git a/tests/integration/fuzz/mod.rs b/tests/integration/fuzz/mod.rs index 7360fb863..27adc6a13 100644 --- a/tests/integration/fuzz/mod.rs +++ b/tests/integration/fuzz/mod.rs @@ -913,4 +913,41 @@ mod tests { ); } } + + #[test] + pub fn values_statement_edge_cases() { + let db = TempDatabase::new_empty(); + let limbo_conn = db.connect_limbo(); + let sqlite_conn = rusqlite::Connection::open_in_memory().unwrap(); + + let test_cases = vec![ + "VALUES (NULL)", + "VALUES (1), (2), (3)", + "VALUES (1, 2, 3)", + "VALUES ('hello', 1, NULL)", + "VALUES (1.5, -2.5, 0.0)", + "VALUES (9223372036854775807)", + "VALUES (-9223372036854775808)", + "VALUES (1, 'text with spaces', NULL)", + "VALUES (''), ('a'), ('ab')", + "VALUES (NULL, NULL), (1, 2)", + "VALUES (1), (2), (NULL), (4)", + "VALUES (1.0, 2.0), (3.0, 4.0)", + "VALUES (-1, -2), (-3, -4)", + "VALUES (1), (NULL), ('text')", + ]; + + for query in test_cases { + println!("query: {:?}", query); + log::info!("Testing query: {}", query); + let limbo = limbo_exec_rows(&db, &limbo_conn, query); + let sqlite = sqlite_exec_rows(&sqlite_conn, query); + + assert_eq!( + limbo, sqlite, + "query: {}, limbo: {:?}, sqlite: {:?}", + query, limbo, sqlite + ); + } + } } From 66f8f5b39450bf476b46f0c12785aab2e5808208 Mon Sep 17 00:00:00 2001 From: krishvishal Date: Sun, 23 Feb 2025 15:30:45 +0530 Subject: [PATCH 4/7] Add support for Binary ops and extend Unary Ops. Add more tests --- core/translate/values.rs | 79 +++++++++++++++++++++++++++++++++-- tests/integration/fuzz/mod.rs | 18 +++++++- 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/core/translate/values.rs b/core/translate/values.rs index b277294b5..ac99cba4d 100644 --- a/core/translate/values.rs +++ b/core/translate/values.rs @@ -12,6 +12,19 @@ pub fn emit_values(program: &mut ProgramBuilder, values: &[Vec]) -> R let start_reg = 1; let first_row_len = values[0].len(); + + let needs_binary_regs = values.iter().any(|row| { + row.iter().any(|expr| matches!(expr, ast::Expr::Binary(..))) + }); + + let binary_regs = if needs_binary_regs { 2 } else { 0 }; + let total_regs = start_reg + first_row_len + binary_regs; + + for _ in 0..total_regs { + program.alloc_register(); + } + + let temp_reg = start_reg + first_row_len; for row in values { if row.len() != first_row_len { @@ -22,10 +35,11 @@ pub fn emit_values(program: &mut ProgramBuilder, values: &[Vec]) -> R for (i, expr) in row.iter().enumerate() { let reg = start_reg + i; + match expr { ast::Expr::Literal(lit) => match lit { Literal::String(s) => { - let s = &s[1..s.len()-1]; + let s = &s[1..s.len()-1]; let s = s.replace("''", "'"); program.emit_insn(Insn::String8 { value: s, @@ -60,10 +74,15 @@ pub fn emit_values(program: &mut ProgramBuilder, values: &[Vec]) -> R }, ast::Expr::Unary(op, expr) => { match (&op, expr.as_ref()) { + (_, ast::Expr::Literal(Literal::Null)) => { + program.emit_insn(Insn::Null { + dest: reg, + dest_end: None, + }); + }, (ast::UnaryOperator::Negative | ast::UnaryOperator::Positive, ast::Expr::Literal(Literal::Numeric(numeric_value))) => { let multiplier = if let ast::UnaryOperator::Negative = op { -1 } else { 1 }; - // Special case: check for negating i64::MAX+1 to get i64::MIN if multiplier == -1 && numeric_value == "9223372036854775808" { program.emit_insn(Insn::Integer { value: i64::MIN, @@ -87,14 +106,66 @@ pub fn emit_values(program: &mut ProgramBuilder, values: &[Vec]) -> R }, _ => { return Err(LimboError::ParseError( - "VALUES only supports literal values and unary on numbers".into(), + "VALUES only supports unary operations on NULL and numbers".into(), + )) + } + } + }, + ast::Expr::Binary(lhs, op, rhs) => { + if !needs_binary_regs { + return Err(LimboError::InternalError("binary operation found but no registers allocated".into())); + } + + match (lhs.as_ref(), op, rhs.as_ref()) { + ( + ast::Expr::Literal(Literal::Numeric(lhs)), + ast::Operator::Divide, + ast::Expr::Literal(Literal::Numeric(rhs)) + ) => { + let lhs_reg = temp_reg; + if let Ok(int_val) = lhs.parse::() { + program.emit_insn(Insn::Integer { + value: int_val, + dest: lhs_reg, + }); + } else { + let float_val = lhs.parse::()?; + program.emit_insn(Insn::Real { + value: float_val, + dest: lhs_reg, + }); + } + + let rhs_reg = temp_reg + 1; + if let Ok(int_val) = rhs.parse::() { + program.emit_insn(Insn::Integer { + value: int_val, + dest: rhs_reg, + }); + } else { + let float_val = rhs.parse::()?; + program.emit_insn(Insn::Real { + value: float_val, + dest: rhs_reg, + }); + } + + program.emit_insn(Insn::Divide { + lhs: lhs_reg, + rhs: rhs_reg, + dest: reg, + }); + } + _ => { + return Err(LimboError::ParseError( + "VALUES only supports division operations on numbers".into(), )) } } }, _ => { return Err(LimboError::ParseError( - "VALUES only supports literal values and unary operations".into(), + "VALUES only supports literals, unary and basic arithmetic operations".into(), )) } } diff --git a/tests/integration/fuzz/mod.rs b/tests/integration/fuzz/mod.rs index 27adc6a13..53bebb465 100644 --- a/tests/integration/fuzz/mod.rs +++ b/tests/integration/fuzz/mod.rs @@ -920,6 +920,7 @@ mod tests { let limbo_conn = db.connect_limbo(); let sqlite_conn = rusqlite::Connection::open_in_memory().unwrap(); + // Test edge cases and specific scenarios let test_cases = vec![ "VALUES (NULL)", "VALUES (1), (2), (3)", @@ -927,7 +928,7 @@ mod tests { "VALUES ('hello', 1, NULL)", "VALUES (1.5, -2.5, 0.0)", "VALUES (9223372036854775807)", - "VALUES (-9223372036854775808)", + "VALUES (-9223372036854775808)", "VALUES (1, 'text with spaces', NULL)", "VALUES (''), ('a'), ('ab')", "VALUES (NULL, NULL), (1, 2)", @@ -935,10 +936,23 @@ mod tests { "VALUES (1.0, 2.0), (3.0, 4.0)", "VALUES (-1, -2), (-3, -4)", "VALUES (1), (NULL), ('text')", + "VALUES (0.0), (-0.0), (1.0/0.0)", + "VALUES ('text''with''quotes')", + "VALUES ('Monty Python and the Holy Grail', 1975, 8.2)", + "VALUES ('And Now for Something Completely Different', 1971, 7.5)", + // Unary ops tests + "VALUES (-1)", + "VALUES (+1)", + "VALUES (+9223372036854775807)", + "VALUES (-1.7976931348623157e308)", + "VALUES (+1.7976931348623157e308)", + "VALUES (-0)", + "VALUES (+0)", + "VALUES (-NULL)", + "VALUES (-(1))", ]; for query in test_cases { - println!("query: {:?}", query); log::info!("Testing query: {}", query); let limbo = limbo_exec_rows(&db, &limbo_conn, query); let sqlite = sqlite_exec_rows(&sqlite_conn, query); From b0cfee734d24de7dfabf3866ce72f36206167fa2 Mon Sep 17 00:00:00 2001 From: krishvishal Date: Mon, 24 Feb 2025 13:51:02 +0530 Subject: [PATCH 5/7] Add basic value statements fuzzer --- tests/integration/fuzz/mod.rs | 86 +++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/tests/integration/fuzz/mod.rs b/tests/integration/fuzz/mod.rs index 53bebb465..44bd40a47 100644 --- a/tests/integration/fuzz/mod.rs +++ b/tests/integration/fuzz/mod.rs @@ -914,6 +914,92 @@ mod tests { } } + #[test] + pub fn values_expression_fuzz_run() { + let _ = env_logger::try_init(); + let g = GrammarGenerator::new(); + + let (expr, expr_builder) = g.create_handle(); + let (values_row, values_row_builder) = g.create_handle(); + let (values_list, values_list_builder) = g.create_handle(); + let (literal, literal_builder) = g.create_handle(); + let (number, number_builder) = g.create_handle(); + let (string, string_builder) = g.create_handle(); + + number_builder + .choice() + .option_symbol(rand_int(-5..10)) + .option_symbol(rand_int(-1000..1000)) + .option_symbol(rand_int(-100000..100000)) + .option_str("NULL") + .build(); + + string_builder + .concat("") + .push_str("'") // Open quote + .push_symbol(rand_str("abc123", 10)) + .push_str("'") // Close quote + .build(); + + literal_builder + .choice() + .option(number) + .option(string) + .option_str("NULL") + .option_str("1.5") + .option_str("-2.5") + .option_str("0.0") + .build(); + + let fixed_columns = g.create() + .concat("") + .push(literal) + .push_str(", ") + .push(literal) + .push_str(", ") + .push(literal) + .build(); + + values_row_builder + .concat("") + .push_str("(") + .push(fixed_columns) + .push_str(")") + .build(); + + values_list_builder + .concat("") + .push(g.create().concat("").push(values_row).repeat(1..10, ", ").build()) + .build(); + + expr_builder + .concat("") + .push_str("VALUES ") + .push(values_list) + .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 i in 0..1024 { + let query = g.generate(&mut rng, expr, 50); + log::info!("Query {} of 0..1024: {}", i, query); + + let limbo = limbo_exec_rows(&db, &limbo_conn, &query); + let sqlite = sqlite_exec_rows(&sqlite_conn, &query); + + assert_eq!( + limbo, sqlite, + "Query: {}\nLimbo result: {:?}\nSQLite result: {:?}", + query, limbo, sqlite + ); + } + } + #[test] pub fn values_statement_edge_cases() { let db = TempDatabase::new_empty(); From 257b5dfdc6a4074988f2300fd2e48bc068e877ac Mon Sep 17 00:00:00 2001 From: krishvishal Date: Mon, 24 Feb 2025 14:56:46 +0530 Subject: [PATCH 6/7] Use translate_expr to support values. --- core/translate/values.rs | 168 +++++---------------------------------- 1 file changed, 19 insertions(+), 149 deletions(-) diff --git a/core/translate/values.rs b/core/translate/values.rs index ac99cba4d..be181955b 100644 --- a/core/translate/values.rs +++ b/core/translate/values.rs @@ -1,8 +1,10 @@ use crate::{ + translate::expr::translate_expr, + translate::emitter::Resolver, vdbe::{builder::ProgramBuilder, insn::Insn, BranchOffset}, - LimboError, Result, + LimboError, Result, SymbolTable, }; -use limbo_sqlite3_parser::ast::{self, Literal}; +use limbo_sqlite3_parser::ast; pub fn emit_values(program: &mut ProgramBuilder, values: &[Vec]) -> Result<()> { let goto_target = program.allocate_label(); @@ -10,165 +12,29 @@ pub fn emit_values(program: &mut ProgramBuilder, values: &[Vec]) -> R target_pc: goto_target, }); + let start_offset = program.offset(); + let start_reg = 1; let first_row_len = values[0].len(); - - let needs_binary_regs = values.iter().any(|row| { - row.iter().any(|expr| matches!(expr, ast::Expr::Binary(..))) - }); - - let binary_regs = if needs_binary_regs { 2 } else { 0 }; - let total_regs = start_reg + first_row_len + binary_regs; + let num_regs = start_reg + first_row_len; - for _ in 0..total_regs { + for _ in 0..num_regs { program.alloc_register(); } - let temp_reg = start_reg + first_row_len; + let symbol_table = SymbolTable::new(); + let resolver = Resolver::new(&symbol_table); for row in values { if row.len() != first_row_len { return Err(LimboError::ParseError( - "all VALUES rows must have the same number of values".into(), + "all VALUES rows must have the same number of values".into(), )); } for (i, expr) in row.iter().enumerate() { let reg = start_reg + i; - - match expr { - ast::Expr::Literal(lit) => match lit { - Literal::String(s) => { - let s = &s[1..s.len()-1]; - let s = s.replace("''", "'"); - program.emit_insn(Insn::String8 { - value: s, - dest: reg, - }); - } - Literal::Numeric(num) => { - if let Ok(int_val) = num.parse::() { - program.emit_insn(Insn::Integer { - value: int_val, - dest: reg, - }); - } else { - let float_val = num.parse::()?; - program.emit_insn(Insn::Real { - value: float_val, - dest: reg, - }); - } - } - Literal::Null => { - program.emit_insn(Insn::Null { - dest: reg, - dest_end: None, - }); - } - _ => { - return Err(LimboError::ParseError( - "unsupported literal type in VALUES".into(), - )) - } - }, - ast::Expr::Unary(op, expr) => { - match (&op, expr.as_ref()) { - (_, ast::Expr::Literal(Literal::Null)) => { - program.emit_insn(Insn::Null { - dest: reg, - dest_end: None, - }); - }, - (ast::UnaryOperator::Negative | ast::UnaryOperator::Positive, ast::Expr::Literal(Literal::Numeric(numeric_value))) => { - let multiplier = if let ast::UnaryOperator::Negative = op { -1 } else { 1 }; - - if multiplier == -1 && numeric_value == "9223372036854775808" { - program.emit_insn(Insn::Integer { - value: i64::MIN, - dest: reg, - }); - } else { - let maybe_int = numeric_value.parse::(); - if let Ok(value) = maybe_int { - program.emit_insn(Insn::Integer { - value: value * multiplier, - dest: reg, - }); - } else { - let value = numeric_value.parse::()?; - program.emit_insn(Insn::Real { - value: value * multiplier as f64, - dest: reg, - }); - } - } - }, - _ => { - return Err(LimboError::ParseError( - "VALUES only supports unary operations on NULL and numbers".into(), - )) - } - } - }, - ast::Expr::Binary(lhs, op, rhs) => { - if !needs_binary_regs { - return Err(LimboError::InternalError("binary operation found but no registers allocated".into())); - } - - match (lhs.as_ref(), op, rhs.as_ref()) { - ( - ast::Expr::Literal(Literal::Numeric(lhs)), - ast::Operator::Divide, - ast::Expr::Literal(Literal::Numeric(rhs)) - ) => { - let lhs_reg = temp_reg; - if let Ok(int_val) = lhs.parse::() { - program.emit_insn(Insn::Integer { - value: int_val, - dest: lhs_reg, - }); - } else { - let float_val = lhs.parse::()?; - program.emit_insn(Insn::Real { - value: float_val, - dest: lhs_reg, - }); - } - - let rhs_reg = temp_reg + 1; - if let Ok(int_val) = rhs.parse::() { - program.emit_insn(Insn::Integer { - value: int_val, - dest: rhs_reg, - }); - } else { - let float_val = rhs.parse::()?; - program.emit_insn(Insn::Real { - value: float_val, - dest: rhs_reg, - }); - } - - program.emit_insn(Insn::Divide { - lhs: lhs_reg, - rhs: rhs_reg, - dest: reg, - }); - } - _ => { - return Err(LimboError::ParseError( - "VALUES only supports division operations on numbers".into(), - )) - } - } - }, - _ => { - return Err(LimboError::ParseError( - "VALUES only supports literals, unary and basic arithmetic operations".into(), - )) - } - } + translate_expr(program, None, expr, reg, &resolver)?; } program.emit_insn(Insn::ResultRow { @@ -179,12 +45,16 @@ pub fn emit_values(program: &mut ProgramBuilder, values: &[Vec]) -> R program.emit_insn(Insn::Halt { err_code: 0, - description: String::new(), + description: String::new(), }); - program.preassign_label_to_next_insn(goto_target); + program.resolve_label(goto_target, program.offset()); + program.emit_insn(Insn::Transaction { write: false }); + + program.emit_constant_insns(); + program.emit_insn(Insn::Goto { - target_pc: BranchOffset::Offset(1), + target_pc: start_offset, }); Ok(()) From 8052f519e6708804b4502ed88273bd0aae23d08c Mon Sep 17 00:00:00 2001 From: krishvishal Date: Mon, 24 Feb 2025 14:59:10 +0530 Subject: [PATCH 7/7] chore: cargo fmt --- core/translate/values.rs | 8 ++--- tests/integration/fuzz/mod.rs | 55 ++++++++++++++++++++--------------- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/core/translate/values.rs b/core/translate/values.rs index be181955b..107cee4ca 100644 --- a/core/translate/values.rs +++ b/core/translate/values.rs @@ -1,6 +1,6 @@ use crate::{ - translate::expr::translate_expr, translate::emitter::Resolver, + translate::expr::translate_expr, vdbe::{builder::ProgramBuilder, insn::Insn, BranchOffset}, LimboError, Result, SymbolTable, }; @@ -28,7 +28,7 @@ pub fn emit_values(program: &mut ProgramBuilder, values: &[Vec]) -> R for row in values { if row.len() != first_row_len { return Err(LimboError::ParseError( - "all VALUES rows must have the same number of values".into(), + "all VALUES rows must have the same number of values".into(), )); } @@ -45,7 +45,7 @@ pub fn emit_values(program: &mut ProgramBuilder, values: &[Vec]) -> R program.emit_insn(Insn::Halt { err_code: 0, - description: String::new(), + description: String::new(), }); program.resolve_label(goto_target, program.offset()); @@ -58,4 +58,4 @@ pub fn emit_values(program: &mut ProgramBuilder, values: &[Vec]) -> R }); Ok(()) -} \ No newline at end of file +} diff --git a/tests/integration/fuzz/mod.rs b/tests/integration/fuzz/mod.rs index 44bd40a47..8d2e62cb9 100644 --- a/tests/integration/fuzz/mod.rs +++ b/tests/integration/fuzz/mod.rs @@ -918,14 +918,14 @@ mod tests { pub fn values_expression_fuzz_run() { let _ = env_logger::try_init(); let g = GrammarGenerator::new(); - + let (expr, expr_builder) = g.create_handle(); let (values_row, values_row_builder) = g.create_handle(); let (values_list, values_list_builder) = g.create_handle(); let (literal, literal_builder) = g.create_handle(); let (number, number_builder) = g.create_handle(); let (string, string_builder) = g.create_handle(); - + number_builder .choice() .option_symbol(rand_int(-5..10)) @@ -933,25 +933,26 @@ mod tests { .option_symbol(rand_int(-100000..100000)) .option_str("NULL") .build(); - + string_builder .concat("") - .push_str("'") // Open quote + .push_str("'") // Open quote .push_symbol(rand_str("abc123", 10)) - .push_str("'") // Close quote + .push_str("'") // Close quote .build(); - + literal_builder .choice() .option(number) - .option(string) + .option(string) .option_str("NULL") .option_str("1.5") .option_str("-2.5") .option_str("0.0") .build(); - - let fixed_columns = g.create() + + let fixed_columns = g + .create() .concat("") .push(literal) .push_str(", ") @@ -959,39 +960,45 @@ mod tests { .push_str(", ") .push(literal) .build(); - + values_row_builder .concat("") .push_str("(") .push(fixed_columns) .push_str(")") .build(); - + values_list_builder .concat("") - .push(g.create().concat("").push(values_row).repeat(1..10, ", ").build()) + .push( + g.create() + .concat("") + .push(values_row) + .repeat(1..10, ", ") + .build(), + ) .build(); - + expr_builder .concat("") .push_str("VALUES ") .push(values_list) .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 i in 0..1024 { let query = g.generate(&mut rng, expr, 50); log::info!("Query {} of 0..1024: {}", i, query); - + let limbo = limbo_exec_rows(&db, &limbo_conn, &query); let sqlite = sqlite_exec_rows(&sqlite_conn, &query); - + assert_eq!( limbo, sqlite, "Query: {}\nLimbo result: {:?}\nSQLite result: {:?}", @@ -1005,8 +1012,8 @@ mod tests { let db = TempDatabase::new_empty(); let limbo_conn = db.connect_limbo(); let sqlite_conn = rusqlite::Connection::open_in_memory().unwrap(); - - // Test edge cases and specific scenarios + + // Test edge cases and specific scenarios let test_cases = vec![ "VALUES (NULL)", "VALUES (1), (2), (3)", @@ -1014,7 +1021,7 @@ mod tests { "VALUES ('hello', 1, NULL)", "VALUES (1.5, -2.5, 0.0)", "VALUES (9223372036854775807)", - "VALUES (-9223372036854775808)", + "VALUES (-9223372036854775808)", "VALUES (1, 'text with spaces', NULL)", "VALUES (''), ('a'), ('ab')", "VALUES (NULL, NULL), (1, 2)", @@ -1027,7 +1034,7 @@ mod tests { "VALUES ('Monty Python and the Holy Grail', 1975, 8.2)", "VALUES ('And Now for Something Completely Different', 1971, 7.5)", // Unary ops tests - "VALUES (-1)", + "VALUES (-1)", "VALUES (+1)", "VALUES (+9223372036854775807)", "VALUES (-1.7976931348623157e308)", @@ -1037,12 +1044,12 @@ mod tests { "VALUES (-NULL)", "VALUES (-(1))", ]; - + for query in test_cases { log::info!("Testing query: {}", query); let limbo = limbo_exec_rows(&db, &limbo_conn, query); let sqlite = sqlite_exec_rows(&sqlite_conn, query); - + assert_eq!( limbo, sqlite, "query: {}, limbo: {:?}, sqlite: {:?}",