From ea3ab2a9c7093420a5c8851a392932ba96062b0e Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Thu, 21 Aug 2025 20:05:29 +0530 Subject: [PATCH 01/13] add ifNeg op_code --- core/vdbe/execute.rs | 27 +++++++++++++++++++++++++++ core/vdbe/explain.rs | 9 +++++++++ core/vdbe/insn.rs | 5 +++++ 3 files changed, 41 insertions(+) diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 849b37839..b58a892ba 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -7336,6 +7336,33 @@ pub fn op_rename_column( Ok(InsnFunctionStepResult::Step) } +pub fn op_if_neg( + program: &Program, + state: &mut ProgramState, + insn: &Insn, + pager: &Rc, + mv_store: Option<&Arc>, +) -> Result { + load_insn!(IfNeg { reg, target_pc }, insn); + + match &state.registers[*reg] { + Register::Value(Value::Integer(i)) if *i < 0 => { + state.pc = target_pc.as_offset_int(); + } + Register::Value(Value::Float(f)) if *f < 0.0 => { + state.pc = target_pc.as_offset_int(); + } + Register::Value(Value::Null) => { + state.pc += 1; + } + _ => { + state.pc += 1; + } + } + + Ok(InsnFunctionStepResult::Step) +} + impl Value { pub fn exec_lower(&self) -> Option { match self { diff --git a/core/vdbe/explain.rs b/core/vdbe/explain.rs index 36e1ebca4..7fa74fe69 100644 --- a/core/vdbe/explain.rs +++ b/core/vdbe/explain.rs @@ -1709,6 +1709,15 @@ pub fn insn_to_str( 0, format!("collation={collation}"), ), + Insn::IfNeg { reg, target_pc } => ( + "IfNeg", + *reg as i32, + target_pc.as_debug_int(), + 0, + Value::build_text(""), + 0, + format!("if (r[{}] < 0) goto {}", reg, target_pc.as_debug_int()), + ), }; format!( "{:<4} {:<17} {:<4} {:<4} {:<4} {:<13} {:<2} {}", diff --git a/core/vdbe/insn.rs b/core/vdbe/insn.rs index 67bc2899e..982ab71c0 100644 --- a/core/vdbe/insn.rs +++ b/core/vdbe/insn.rs @@ -1075,6 +1075,10 @@ pub enum Insn { dest: usize, // P2: output register for result new_mode: Option, // P3: new journal mode (if setting) }, + IfNeg { + reg: usize, + target_pc: BranchOffset, + }, } impl Insn { @@ -1212,6 +1216,7 @@ impl Insn { Insn::RenameColumn { .. } => execute::op_rename_column, Insn::MaxPgcnt { .. } => execute::op_max_pgcnt, Insn::JournalMode { .. } => execute::op_journal_mode, + Insn::IfNeg { .. } => execute::op_if_neg, } } } From 28439efd097b692ab6fd23e1bcb72260a3acdbd0 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Thu, 21 Aug 2025 20:07:40 +0530 Subject: [PATCH 02/13] make offset and limit Expr --- core/translate/plan.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/core/translate/plan.rs b/core/translate/plan.rs index eba50ce89..99519ec28 100644 --- a/core/translate/plan.rs +++ b/core/translate/plan.rs @@ -154,8 +154,8 @@ pub enum Plan { CompoundSelect { left: Vec<(SelectPlan, ast::CompoundOperator)>, right_most: SelectPlan, - limit: Option, - offset: Option, + limit: Option, + offset: Option, order_by: Option>, }, Delete(DeletePlan), @@ -292,9 +292,9 @@ pub struct SelectPlan { /// all the aggregates collected from the result columns, order by, and (TODO) having clauses pub aggregates: Vec, /// limit clause - pub limit: Option, + pub limit: Option, /// offset clause - pub offset: Option, + pub offset: Option, /// query contains a constant condition that is always false pub contains_constant_false_condition: bool, /// the destination of the resulting rows from this plan. @@ -378,9 +378,9 @@ pub struct DeletePlan { /// order by clause pub order_by: Vec<(Box, SortOrder)>, /// limit clause - pub limit: Option, + pub limit: Option, /// offset clause - pub offset: Option, + pub offset: Option, /// query contains a constant condition that is always false pub contains_constant_false_condition: bool, /// Indexes that must be updated by the delete operation. @@ -394,8 +394,8 @@ pub struct UpdatePlan { pub set_clauses: Vec<(usize, Box)>, pub where_clause: Vec, pub order_by: Vec<(Box, SortOrder)>, - pub limit: Option, - pub offset: Option, + pub limit: Option, + pub offset: Option, // TODO: optional RETURNING clause pub returning: Option>, // whether the WHERE clause is always false From ffcadd00aefcfa8b506b5e2e48d338f95377ea37 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Thu, 21 Aug 2025 20:10:39 +0530 Subject: [PATCH 03/13] evaluate limit or offset expr --- core/translate/compound_select.rs | 84 ++++++++++++--- core/translate/display.rs | 2 +- core/translate/emitter.rs | 126 +++++++++++++++------- core/translate/planner.rs | 43 ++------ core/translate/result_row.rs | 170 +++++++++++++++++++++++++++++- 5 files changed, 331 insertions(+), 94 deletions(-) diff --git a/core/translate/compound_select.rs b/core/translate/compound_select.rs index 15fb4d841..5316a744f 100644 --- a/core/translate/compound_select.rs +++ b/core/translate/compound_select.rs @@ -1,6 +1,7 @@ use crate::schema::{Index, IndexColumn, Schema}; use crate::translate::emitter::{emit_query, LimitCtx, TranslateCtx}; use crate::translate::plan::{Plan, QueryDestination, SelectPlan}; +use crate::translate::result_row::{build_limit_offset_expr, try_fold_expr_to_i64}; use crate::vdbe::builder::{CursorType, ProgramBuilder}; use crate::vdbe::insn::Insn; use crate::vdbe::BranchOffset; @@ -31,8 +32,8 @@ pub fn emit_program_for_compound_select( let right_plan = right_most.clone(); // Trivial exit on LIMIT 0 - if let Some(limit) = limit { - if *limit == 0 { + if let Some(expr) = limit { + if let Some(0) = try_fold_expr_to_i64(expr) { program.result_columns = right_plan.result_columns; program.table_references.extend(right_plan.table_references); return Ok(()); @@ -41,26 +42,79 @@ pub fn emit_program_for_compound_select( // Each subselect shares the same limit_ctx and offset, because the LIMIT, OFFSET applies to // the entire compound select, not just a single subselect. - let limit_ctx = limit.map(|limit| { + let limit_ctx = limit.clone().map(|limit| { let reg = program.alloc_register(); - program.emit_insn(Insn::Integer { - value: limit as i64, - dest: reg, - }); + if let Some(val) = try_fold_expr_to_i64(&limit) { + // Compile-time constant limit + program.emit_insn(Insn::Integer { + value: val, + dest: reg, + }); + } else { + program.add_comment(program.offset(), "OFFSET expr"); + + let label_zero = program.allocate_label(); + + build_limit_offset_expr(program, reg, &limit); + + program.emit_insn(Insn::MustBeInt { reg }); + + program.emit_insn(Insn::IfNeg { + reg, + target_pc: label_zero, + }); + program.emit_insn(Insn::IsNull { + reg, + target_pc: label_zero, + }); + + program.preassign_label_to_next_insn(label_zero); + program.emit_insn(Insn::Integer { + value: 0, + dest: reg, + }); + } LimitCtx::new_shared(reg) }); - let offset_reg = offset.map(|offset| { + let offset_reg = offset.as_ref().map(|offset_expr| { let reg = program.alloc_register(); - program.emit_insn(Insn::Integer { - value: offset as i64, - dest: reg, - }); + + if let Some(val) = try_fold_expr_to_i64(offset_expr) { + // Compile-time constant offset + program.emit_insn(Insn::Integer { + value: val, + dest: reg, + }); + } else { + program.add_comment(program.offset(), "OFFSET expr"); + + let label_zero = program.allocate_label(); + + build_limit_offset_expr(program, reg, &offset_expr); + + program.emit_insn(Insn::MustBeInt { reg }); + + program.emit_insn(Insn::IfNeg { + reg, + target_pc: label_zero, + }); + program.emit_insn(Insn::IsNull { + reg, + target_pc: label_zero, + }); + + program.preassign_label_to_next_insn(label_zero); + program.emit_insn(Insn::Integer { + value: 0, + dest: reg, + }); + } let combined_reg = program.alloc_register(); program.emit_insn(Insn::OffsetLimit { offset_reg: reg, combined_reg, - limit_reg: limit_ctx.unwrap().reg_limit, + limit_reg: limit_ctx.as_ref().unwrap().reg_limit, }); reg @@ -137,8 +191,8 @@ fn emit_compound_select( let compound_select = Plan::CompoundSelect { left, right_most: plan, - limit, - offset, + limit: limit.clone(), + offset: offset.clone(), order_by, }; emit_compound_select( diff --git a/core/translate/display.rs b/core/translate/display.rs index 631e5a295..3e79ef223 100644 --- a/core/translate/display.rs +++ b/core/translate/display.rs @@ -217,7 +217,7 @@ impl fmt::Display for UpdatePlan { )?; } } - if let Some(limit) = self.limit { + if let Some(limit) = self.limit.clone() { writeln!(f, "LIMIT: {limit}")?; } if let Some(ret) = &self.returning { diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 29cbd210a..70b2f8310 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -26,6 +26,7 @@ use crate::schema::{BTreeTable, Column, Schema, Table}; use crate::translate::compound_select::emit_program_for_compound_select; use crate::translate::expr::{emit_returning_results, ReturningValueRegisters}; use crate::translate::plan::{DeletePlan, Plan, QueryDestination, Search}; +use crate::translate::result_row::{build_limit_offset_expr, try_fold_expr_to_i64}; use crate::translate::values::emit_values; use crate::util::exprs_are_equivalent; use crate::vdbe::builder::{CursorKey, CursorType, ProgramBuilder}; @@ -227,8 +228,8 @@ fn emit_program_for_select( ); // Trivial exit on LIMIT 0 - if let Some(limit) = plan.limit { - if limit == 0 { + if let Some(limit) = plan.limit.as_ref().and_then(try_fold_expr_to_i64) { + if limit <= 0 { program.result_columns = plan.result_columns; program.table_references.extend(plan.table_references); return Ok(()); @@ -256,7 +257,7 @@ pub fn emit_query<'a>( // Emit subqueries first so the results can be read in the main query loop. emit_subqueries(program, t_ctx, &mut plan.table_references)?; - init_limit(program, t_ctx, plan.limit, plan.offset); + init_limit(program, t_ctx, plan.limit.clone(), plan.offset.clone()); // No rows will be read from source table loops if there is a constant false condition eg. WHERE 0 // however an aggregation might still happen, @@ -404,10 +405,12 @@ fn emit_program_for_delete( ); // exit early if LIMIT 0 - if let Some(0) = plan.limit { - program.result_columns = plan.result_columns; - program.table_references.extend(plan.table_references); - return Ok(()); + if let Some(limit) = plan.limit.as_ref().and_then(try_fold_expr_to_i64) { + if limit <= 0 { + program.result_columns = plan.result_columns; + program.table_references.extend(plan.table_references); + return Ok(()); + } } init_limit(program, &mut t_ctx, plan.limit, None); @@ -660,13 +663,15 @@ fn emit_program_for_update( ); // Exit on LIMIT 0 - if let Some(0) = plan.limit { - program.result_columns = plan.returning.unwrap_or_default(); - program.table_references.extend(plan.table_references); - return Ok(()); + if let Some(limit) = plan.limit.as_ref().and_then(try_fold_expr_to_i64) { + if limit <= 0 { + program.result_columns = plan.returning.unwrap_or_default(); + program.table_references.extend(plan.table_references); + return Ok(()); + } } - init_limit(program, &mut t_ctx, plan.limit, plan.offset); + init_limit(program, &mut t_ctx, plan.limit.clone(), plan.offset.clone()); let after_main_loop_label = program.allocate_label(); t_ctx.label_main_loop_end = Some(after_main_loop_label); if plan.contains_constant_false_condition { @@ -1541,41 +1546,90 @@ pub fn emit_cdc_insns( }); Ok(()) } - /// Initialize the limit/offset counters and registers. /// In case of compound SELECTs, the limit counter is initialized only once, /// hence [LimitCtx::initialize_counter] being false in those cases. fn init_limit( program: &mut ProgramBuilder, t_ctx: &mut TranslateCtx, - limit: Option, - offset: Option, + limit: Option, + offset: Option, ) { - if t_ctx.limit_ctx.is_none() { - t_ctx.limit_ctx = limit.map(|_| LimitCtx::new(program)); + if t_ctx.limit_ctx.is_none() && limit.is_some() { + t_ctx.limit_ctx = Some(LimitCtx::new(program)); } - let Some(limit_ctx) = t_ctx.limit_ctx else { + let Some(limit_ctx) = &t_ctx.limit_ctx else { return; }; if limit_ctx.initialize_counter { - program.emit_insn(Insn::Integer { - value: limit.expect("limit must be Some if limit_ctx is Some") as i64, - dest: limit_ctx.reg_limit, - }); + if let Some(expr) = limit { + if let Some(value) = try_fold_expr_to_i64(&expr) { + program.emit_insn(Insn::Integer { + value, + dest: limit_ctx.reg_limit, + }); + } else { + let r = limit_ctx.reg_limit; + program.add_comment(program.offset(), "OFFSET expr"); + let label_zero = program.allocate_label(); + build_limit_offset_expr(program, r, &expr); + program.emit_insn(Insn::MustBeInt { reg: r }); + program.emit_insn(Insn::IfNeg { + reg: r, + target_pc: label_zero, + }); + program.emit_insn(Insn::IsNull { + reg: r, + target_pc: label_zero, + }); + program.preassign_label_to_next_insn(label_zero); + program.emit_insn(Insn::Integer { value: 0, dest: r }); + } + } } - if t_ctx.reg_offset.is_none() && offset.is_some_and(|n| n.ne(&0)) { - let reg = program.alloc_register(); - t_ctx.reg_offset = Some(reg); - program.emit_insn(Insn::Integer { - value: offset.unwrap() as i64, - dest: reg, - }); - let combined_reg = program.alloc_register(); - t_ctx.reg_limit_offset_sum = Some(combined_reg); - program.emit_insn(Insn::OffsetLimit { - limit_reg: t_ctx.limit_ctx.unwrap().reg_limit, - offset_reg: reg, - combined_reg, - }); + + if t_ctx.reg_offset.is_none() { + if let Some(expr) = offset { + if let Some(value) = try_fold_expr_to_i64(&expr) { + if value != 0 { + let reg = program.alloc_register(); + t_ctx.reg_offset = Some(reg); + program.emit_insn(Insn::Integer { value, dest: reg }); + let combined_reg = program.alloc_register(); + t_ctx.reg_limit_offset_sum = Some(combined_reg); + program.emit_insn(Insn::OffsetLimit { + limit_reg: limit_ctx.reg_limit, + offset_reg: reg, + combined_reg, + }); + } + } else { + + let reg = program.alloc_register(); + t_ctx.reg_offset = Some(reg); + let r = reg; + program.add_comment(program.offset(), "OFFSET expr"); + let label_zero = program.allocate_label(); + build_limit_offset_expr(program, r, &expr); + program.emit_insn(Insn::MustBeInt { reg: r }); + program.emit_insn(Insn::IfNeg { + reg: r, + target_pc: label_zero, + }); + program.emit_insn(Insn::IsNull { + reg: r, + target_pc: label_zero, + }); + program.preassign_label_to_next_insn(label_zero); + program.emit_insn(Insn::Integer { value: 0, dest: r }); + let combined_reg = program.alloc_register(); + t_ctx.reg_limit_offset_sum = Some(combined_reg); + program.emit_insn(Insn::OffsetLimit { + limit_reg: limit_ctx.reg_limit, + offset_reg: reg, + combined_reg, + }); + } + } } } diff --git a/core/translate/planner.rs b/core/translate/planner.rs index 43f012875..c8c34d4ca 100644 --- a/core/translate/planner.rs +++ b/core/translate/planner.rs @@ -22,7 +22,7 @@ use crate::{ use turso_parser::ast::Literal::Null; use turso_parser::ast::{ self, As, Expr, FromClause, JoinType, Limit, Literal, Materialized, QualifiedName, - TableInternalId, UnaryOperator, With, + TableInternalId, With, }; pub const ROWID: &str = "rowid"; @@ -1106,43 +1106,12 @@ fn parse_join( Ok(()) } -pub fn parse_limit(limit: &Limit) -> Result<(Option, Option)> { - let offset_val = match &limit.offset { - Some(offset_expr) => match offset_expr.as_ref() { - Expr::Literal(ast::Literal::Numeric(n)) => n.parse().ok(), - // If OFFSET is negative, the result is as if OFFSET is zero - Expr::Unary(UnaryOperator::Negative, expr) => { - if let Expr::Literal(ast::Literal::Numeric(ref n)) = &**expr { - n.parse::().ok().map(|num| -num) - } else { - crate::bail_parse_error!("Invalid OFFSET clause"); - } - } - _ => crate::bail_parse_error!("Invalid OFFSET clause"), - }, - None => Some(0), - }; +pub fn parse_limit(limit: &Limit) -> Result<(Option, Option)> { + let limit_expr = Some(limit.expr.clone()); - if let Expr::Literal(ast::Literal::Numeric(n)) = limit.expr.as_ref() { - Ok((n.parse().ok(), offset_val)) - } else if let Expr::Unary(UnaryOperator::Negative, expr) = limit.expr.as_ref() { - if let Expr::Literal(ast::Literal::Numeric(n)) = expr.as_ref() { - let limit_val = n.parse::().ok().map(|num| -num); - Ok((limit_val, offset_val)) - } else { - crate::bail_parse_error!("Invalid LIMIT clause"); - } - } else if let Expr::Id(id) = limit.expr.as_ref() { - if id.as_str().eq_ignore_ascii_case("true") { - Ok((Some(1), offset_val)) - } else if id.as_str().eq_ignore_ascii_case("false") { - Ok((Some(0), offset_val)) - } else { - crate::bail_parse_error!("Invalid LIMIT clause"); - } - } else { - crate::bail_parse_error!("Invalid LIMIT clause"); - } + let offset_expr = limit.offset.clone(); + + Ok((limit_expr, offset_expr)) } pub fn break_predicate_at_and_boundaries(predicate: &Expr, out_predicates: &mut Vec) { diff --git a/core/translate/result_row.rs b/core/translate/result_row.rs index f2b722988..e2950a435 100644 --- a/core/translate/result_row.rs +++ b/core/translate/result_row.rs @@ -1,7 +1,10 @@ +use turso_sqlite3_parser::ast::{Expr, Literal, Operator, UnaryOperator}; + use crate::{ + error::SQLITE_CONSTRAINT, vdbe::{ builder::ProgramBuilder, - insn::{IdxInsertFlags, InsertFlags, Insn}, + insn::{CmpInsFlags, IdxInsertFlags, InsertFlags, Insn}, BranchOffset, }, Result, @@ -164,15 +167,172 @@ pub fn emit_offset( jump_to: BranchOffset, reg_offset: Option, ) { - match plan.offset { - Some(offset) if offset > 0 => { - program.add_comment(program.offset(), "OFFSET"); + let Some(offset_expr) = &plan.offset else { + return; + }; + + if let Some(val) = try_fold_expr_to_i64(offset_expr) { + if val > 0 { + program.add_comment(program.offset(), "OFFSET const"); program.emit_insn(Insn::IfPos { reg: reg_offset.expect("reg_offset must be Some"), target_pc: jump_to, decrement_by: 1, }); } - _ => {} + return; + } + + let r = reg_offset.expect("reg_offset must be Some"); + + program.add_comment(program.offset(), "OFFSET expr"); + + let label_zero = program.allocate_label(); + + build_limit_offset_expr(program, r, offset_expr); + + program.emit_insn(Insn::MustBeInt { reg: r }); + + program.emit_insn(Insn::IfNeg { + reg: r, + target_pc: label_zero, + }); + program.emit_insn(Insn::IsNull { + reg: r, + target_pc: label_zero, + }); + + program.emit_insn(Insn::IfPos { + reg: r, + target_pc: jump_to, + decrement_by: 1, + }); + + program.preassign_label_to_next_insn(label_zero); + program.emit_insn(Insn::Integer { value: 0, dest: r }); +} + +pub fn build_limit_offset_expr(program: &mut ProgramBuilder, r: usize, expr: &Expr) { + match expr { + Expr::Literal(Literal::Numeric(n)) => { + let value = n.parse::().unwrap_or_else(|_| { + program.emit_insn(Insn::Halt { + err_code: SQLITE_CONSTRAINT, + description: "invalid numeric literal".into(), + }); + 0 + }); + program.emit_int(value, r); + } + Expr::Unary(UnaryOperator::Negative, inner) => { + let inner_reg = program.alloc_register(); + build_limit_offset_expr(program, inner_reg, inner); + + let neg_one_reg = program.alloc_register(); + program.emit_int(-1, neg_one_reg); + + program.emit_insn(Insn::Multiply { + lhs: inner_reg, + rhs: neg_one_reg, + dest: r, + }); + } + Expr::Unary(UnaryOperator::Positive, inner) => { + let inner_reg = program.alloc_register(); + build_limit_offset_expr(program, inner_reg, inner); + program.emit_insn(Insn::Copy { + src_reg: inner_reg, + dst_reg: r, + extra_amount: 0, + }); + } + Expr::Binary(left, op, right) => { + let left_reg = program.alloc_register(); + let right_reg = program.alloc_register(); + build_limit_offset_expr(program, left_reg, left); + build_limit_offset_expr(program, right_reg, right); + + match op { + Operator::Add => { + program.emit_insn(Insn::Add { + lhs: left_reg, + rhs: right_reg, + dest: r, + }); + } + Operator::Subtract => { + program.emit_insn(Insn::Subtract { + lhs: left_reg, + rhs: right_reg, + dest: r, + }); + } + Operator::Multiply => { + program.emit_insn(Insn::Multiply { + lhs: left_reg, + rhs: right_reg, + dest: r, + }); + } + Operator::Divide => { + let zero_reg = program.alloc_register(); + program.emit_int(0, zero_reg); + + let ok_pc = program.allocate_label(); + program.emit_insn(Insn::Ne { + lhs: right_reg, + rhs: zero_reg, + target_pc: ok_pc, + flags: CmpInsFlags::default().jump_if_null(), + collation: None, + }); + + program.emit_insn(Insn::Halt { + err_code: SQLITE_CONSTRAINT, + description: "divide by zero".into(), + }); + + program.resolve_label(ok_pc, program.offset()); + program.emit_insn(Insn::Divide { + lhs: left_reg, + rhs: right_reg, + dest: r, + }); + } + _ => { + program.emit_insn(Insn::Halt { + err_code: SQLITE_CONSTRAINT, + description: "unsupported operator in offset expr".into(), + }); + } + } + } + _ => { + program.emit_insn(Insn::Halt { + err_code: SQLITE_CONSTRAINT, + description: "non-integer expression in offset".into(), + }); + } + } +} + +pub fn try_fold_expr_to_i64(expr: &Expr) -> Option { + match expr { + Expr::Literal(Literal::Numeric(n)) => n.parse::().ok(), + Expr::Unary(UnaryOperator::Negative, inner) => try_fold_expr_to_i64(inner).map(|v| -v), + Expr::Unary(UnaryOperator::Positive, inner) => try_fold_expr_to_i64(inner), + Expr::Binary(left, op, right) => { + let l = try_fold_expr_to_i64(left)?; + let r = try_fold_expr_to_i64(right)?; + match op { + Operator::Add => Some(l.saturating_add(r)), + Operator::Subtract => Some(l.saturating_sub(r)), + Operator::Multiply => Some(l.saturating_mul(r)), + Operator::Divide if r != 0 => Some(l.saturating_div(r)), + _ => None, + } + } + + _ => None, } } From 1e5275682d46e32a556c3c11b33037e8b3b1e635 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Thu, 21 Aug 2025 20:35:37 +0530 Subject: [PATCH 04/13] make early exit for value less than equal zero --- core/translate/compound_select.rs | 13 +++++-------- core/translate/emitter.rs | 3 +-- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/core/translate/compound_select.rs b/core/translate/compound_select.rs index 5316a744f..e98dd921c 100644 --- a/core/translate/compound_select.rs +++ b/core/translate/compound_select.rs @@ -32,12 +32,10 @@ pub fn emit_program_for_compound_select( let right_plan = right_most.clone(); // Trivial exit on LIMIT 0 - if let Some(expr) = limit { - if let Some(0) = try_fold_expr_to_i64(expr) { - program.result_columns = right_plan.result_columns; - program.table_references.extend(right_plan.table_references); - return Ok(()); - } + if matches!(limit.as_ref().and_then(try_fold_expr_to_i64), Some(v) if v <= 0) { + program.result_columns = right_plan.result_columns; + program.table_references.extend(right_plan.table_references); + return Ok(()); } // Each subselect shares the same limit_ctx and offset, because the LIMIT, OFFSET applies to @@ -45,7 +43,6 @@ pub fn emit_program_for_compound_select( let limit_ctx = limit.clone().map(|limit| { let reg = program.alloc_register(); if let Some(val) = try_fold_expr_to_i64(&limit) { - // Compile-time constant limit program.emit_insn(Insn::Integer { value: val, dest: reg, @@ -90,7 +87,7 @@ pub fn emit_program_for_compound_select( let label_zero = program.allocate_label(); - build_limit_offset_expr(program, reg, &offset_expr); + build_limit_offset_expr(program, reg, offset_expr); program.emit_insn(Insn::MustBeInt { reg }); diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 70b2f8310..305acef97 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -1576,7 +1576,7 @@ fn init_limit( program.emit_insn(Insn::MustBeInt { reg: r }); program.emit_insn(Insn::IfNeg { reg: r, - target_pc: label_zero, + target_pc: label_zero, }); program.emit_insn(Insn::IsNull { reg: r, @@ -1604,7 +1604,6 @@ fn init_limit( }); } } else { - let reg = program.alloc_register(); t_ctx.reg_offset = Some(reg); let r = reg; From 7e5043edfcf0c6373861629c65d8af7b2199cb6b Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Fri, 22 Aug 2025 20:36:47 +0530 Subject: [PATCH 05/13] add null and boolean limit handling --- core/translate/result_row.rs | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/core/translate/result_row.rs b/core/translate/result_row.rs index e2950a435..90884595f 100644 --- a/core/translate/result_row.rs +++ b/core/translate/result_row.rs @@ -1,4 +1,4 @@ -use turso_sqlite3_parser::ast::{Expr, Literal, Operator, UnaryOperator}; +use turso_sqlite3_parser::ast::{Expr, Literal, Name, Operator, UnaryOperator}; use crate::{ error::SQLITE_CONSTRAINT, @@ -224,6 +224,22 @@ pub fn build_limit_offset_expr(program: &mut ProgramBuilder, r: usize, expr: &Ex }); program.emit_int(value, r); } + Expr::Literal(Literal::Null) => { + program.emit_int(0, r); + } + Expr::Id(Name::Ident(s)) => { + let lowered = s.to_ascii_lowercase(); + if lowered == "true" { + program.emit_int(1, r); + } else if lowered == "false" { + program.emit_int(0, r); + } else { + program.emit_insn(Insn::Halt { + err_code: SQLITE_CONSTRAINT, + description: format!("invalid boolean string literal: {}", s), + }); + } + } Expr::Unary(UnaryOperator::Negative, inner) => { let inner_reg = program.alloc_register(); build_limit_offset_expr(program, inner_reg, inner); @@ -319,6 +335,19 @@ pub fn build_limit_offset_expr(program: &mut ProgramBuilder, r: usize, expr: &Ex pub fn try_fold_expr_to_i64(expr: &Expr) -> Option { match expr { Expr::Literal(Literal::Numeric(n)) => n.parse::().ok(), + Expr::Literal(Literal::Null) => { + Some(0) + } + Expr::Id(Name::Ident(s)) => { + let lowered = s.to_ascii_lowercase(); + if lowered == "true" { + Some(1) + } else if lowered == "false" { + Some(0) + } else { + None + } + } Expr::Unary(UnaryOperator::Negative, inner) => try_fold_expr_to_i64(inner).map(|v| -v), Expr::Unary(UnaryOperator::Positive, inner) => try_fold_expr_to_i64(inner), Expr::Binary(left, op, right) => { From 26d71603ac14e125a47771ab02d29341bc57c9f1 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Fri, 22 Aug 2025 20:47:23 +0530 Subject: [PATCH 06/13] add a test to test limit expressiveness --- testing/select.test | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/testing/select.test b/testing/select.test index 9ed482e4e..d33beb5c4 100755 --- a/testing/select.test +++ b/testing/select.test @@ -59,6 +59,12 @@ do_execsql_test_error select-doubly-qualified-wrong-column { SELECT main.users.wrong FROM users LIMIT 0; } {.*} +do_execsql_test select-limit-expression { + select price from products limit 2 + 1 - 1; +} {79.0 +82.0} + + # ORDER BY id here because sqlite uses age_idx here and we (yet) don't so force it to evaluate in ID order do_execsql_test select-limit-true { SELECT id FROM users ORDER BY id LIMIT true; From a16bee457439689e4b3be21528d057c8bdfa8c98 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Fri, 22 Aug 2025 21:01:24 +0530 Subject: [PATCH 07/13] move to new parser --- core/translate/emitter.rs | 4 ++-- core/translate/plan.rs | 16 ++++++++-------- core/translate/planner.rs | 2 +- core/translate/result_row.rs | 12 +++++------- 4 files changed, 16 insertions(+), 18 deletions(-) diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 305acef97..24b6637b5 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -1552,8 +1552,8 @@ pub fn emit_cdc_insns( fn init_limit( program: &mut ProgramBuilder, t_ctx: &mut TranslateCtx, - limit: Option, - offset: Option, + limit: Option>, + offset: Option>, ) { if t_ctx.limit_ctx.is_none() && limit.is_some() { t_ctx.limit_ctx = Some(LimitCtx::new(program)); diff --git a/core/translate/plan.rs b/core/translate/plan.rs index 99519ec28..ae35a8774 100644 --- a/core/translate/plan.rs +++ b/core/translate/plan.rs @@ -154,8 +154,8 @@ pub enum Plan { CompoundSelect { left: Vec<(SelectPlan, ast::CompoundOperator)>, right_most: SelectPlan, - limit: Option, - offset: Option, + limit: Option>, + offset: Option>, order_by: Option>, }, Delete(DeletePlan), @@ -292,9 +292,9 @@ pub struct SelectPlan { /// all the aggregates collected from the result columns, order by, and (TODO) having clauses pub aggregates: Vec, /// limit clause - pub limit: Option, + pub limit: Option>, /// offset clause - pub offset: Option, + pub offset: Option>, /// query contains a constant condition that is always false pub contains_constant_false_condition: bool, /// the destination of the resulting rows from this plan. @@ -378,9 +378,9 @@ pub struct DeletePlan { /// order by clause pub order_by: Vec<(Box, SortOrder)>, /// limit clause - pub limit: Option, + pub limit: Option>, /// offset clause - pub offset: Option, + pub offset: Option>, /// query contains a constant condition that is always false pub contains_constant_false_condition: bool, /// Indexes that must be updated by the delete operation. @@ -394,8 +394,8 @@ pub struct UpdatePlan { pub set_clauses: Vec<(usize, Box)>, pub where_clause: Vec, pub order_by: Vec<(Box, SortOrder)>, - pub limit: Option, - pub offset: Option, + pub limit: Option>, + pub offset: Option>, // TODO: optional RETURNING clause pub returning: Option>, // whether the WHERE clause is always false diff --git a/core/translate/planner.rs b/core/translate/planner.rs index c8c34d4ca..1d0e0ce72 100644 --- a/core/translate/planner.rs +++ b/core/translate/planner.rs @@ -1106,7 +1106,7 @@ fn parse_join( Ok(()) } -pub fn parse_limit(limit: &Limit) -> Result<(Option, Option)> { +pub fn parse_limit(limit: &Limit) -> Result<(Option>, Option>)> { let limit_expr = Some(limit.expr.clone()); let offset_expr = limit.offset.clone(); diff --git a/core/translate/result_row.rs b/core/translate/result_row.rs index 90884595f..105d1eaca 100644 --- a/core/translate/result_row.rs +++ b/core/translate/result_row.rs @@ -1,4 +1,4 @@ -use turso_sqlite3_parser::ast::{Expr, Literal, Name, Operator, UnaryOperator}; +use turso_parser::ast::{Expr, Literal, Name, Operator, UnaryOperator}; use crate::{ error::SQLITE_CONSTRAINT, @@ -171,7 +171,7 @@ pub fn emit_offset( return; }; - if let Some(val) = try_fold_expr_to_i64(offset_expr) { + if let Some(val) = try_fold_expr_to_i64(&offset_expr.clone()) { if val > 0 { program.add_comment(program.offset(), "OFFSET const"); program.emit_insn(Insn::IfPos { @@ -332,12 +332,10 @@ pub fn build_limit_offset_expr(program: &mut ProgramBuilder, r: usize, expr: &Ex } } -pub fn try_fold_expr_to_i64(expr: &Expr) -> Option { - match expr { +pub fn try_fold_expr_to_i64(expr: &Box) -> Option { + match expr.as_ref() { Expr::Literal(Literal::Numeric(n)) => n.parse::().ok(), - Expr::Literal(Literal::Null) => { - Some(0) - } + Expr::Literal(Literal::Null) => Some(0), Expr::Id(Name::Ident(s)) => { let lowered = s.to_ascii_lowercase(); if lowered == "true" { From 50d9b0f7e37026a5189040ce386efd837805a7a4 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Fri, 22 Aug 2025 21:25:55 +0530 Subject: [PATCH 08/13] ignore limit for value less than 9 --- core/translate/compound_select.rs | 2 +- core/translate/emitter.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/translate/compound_select.rs b/core/translate/compound_select.rs index e98dd921c..43f96d647 100644 --- a/core/translate/compound_select.rs +++ b/core/translate/compound_select.rs @@ -32,7 +32,7 @@ pub fn emit_program_for_compound_select( let right_plan = right_most.clone(); // Trivial exit on LIMIT 0 - if matches!(limit.as_ref().and_then(try_fold_expr_to_i64), Some(v) if v <= 0) { + if matches!(limit.as_ref().and_then(try_fold_expr_to_i64), Some(v) if v == 0) { program.result_columns = right_plan.result_columns; program.table_references.extend(right_plan.table_references); return Ok(()); diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 24b6637b5..1d74eb226 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -229,7 +229,7 @@ fn emit_program_for_select( // Trivial exit on LIMIT 0 if let Some(limit) = plan.limit.as_ref().and_then(try_fold_expr_to_i64) { - if limit <= 0 { + if limit == 0 { program.result_columns = plan.result_columns; program.table_references.extend(plan.table_references); return Ok(()); @@ -406,7 +406,7 @@ fn emit_program_for_delete( // exit early if LIMIT 0 if let Some(limit) = plan.limit.as_ref().and_then(try_fold_expr_to_i64) { - if limit <= 0 { + if limit == 0 { program.result_columns = plan.result_columns; program.table_references.extend(plan.table_references); return Ok(()); @@ -664,7 +664,7 @@ fn emit_program_for_update( // Exit on LIMIT 0 if let Some(limit) = plan.limit.as_ref().and_then(try_fold_expr_to_i64) { - if limit <= 0 { + if limit == 0 { program.result_columns = plan.returning.unwrap_or_default(); program.table_references.extend(plan.table_references); return Ok(()); From 9bebc9b5c70ad8939e3bb0e746736dc08f07aa78 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Fri, 22 Aug 2025 21:35:01 +0530 Subject: [PATCH 09/13] clippy'ed --- core/translate/planner.rs | 1 + core/translate/result_row.rs | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/core/translate/planner.rs b/core/translate/planner.rs index 1d0e0ce72..f71145d93 100644 --- a/core/translate/planner.rs +++ b/core/translate/planner.rs @@ -1106,6 +1106,7 @@ fn parse_join( Ok(()) } +#[allow(clippy::type_complexity)] pub fn parse_limit(limit: &Limit) -> Result<(Option>, Option>)> { let limit_expr = Some(limit.expr.clone()); diff --git a/core/translate/result_row.rs b/core/translate/result_row.rs index 105d1eaca..40e9af6f7 100644 --- a/core/translate/result_row.rs +++ b/core/translate/result_row.rs @@ -236,7 +236,7 @@ pub fn build_limit_offset_expr(program: &mut ProgramBuilder, r: usize, expr: &Ex } else { program.emit_insn(Insn::Halt { err_code: SQLITE_CONSTRAINT, - description: format!("invalid boolean string literal: {}", s), + description: format!("invalid boolean string literal: {s}"), }); } } @@ -332,6 +332,7 @@ pub fn build_limit_offset_expr(program: &mut ProgramBuilder, r: usize, expr: &Ex } } +#[allow(clippy::borrowed_box)] pub fn try_fold_expr_to_i64(expr: &Box) -> Option { match expr.as_ref() { Expr::Literal(Literal::Numeric(n)) => n.parse::().ok(), From a3b87cd97f0f2f08f0c5eaf17a00058611233267 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sun, 24 Aug 2025 15:14:39 +0530 Subject: [PATCH 10/13] add review comments --- core/translate/compound_select.rs | 18 +++-- core/translate/delete.rs | 5 +- core/translate/display.rs | 2 +- core/translate/emitter.rs | 22 ++--- core/translate/order_by.rs | 8 +- core/translate/planner.rs | 13 +-- core/translate/result_row.rs | 130 ++---------------------------- core/translate/select.rs | 9 +-- core/translate/update.rs | 8 +- core/translate/values.rs | 4 +- 10 files changed, 54 insertions(+), 165 deletions(-) diff --git a/core/translate/compound_select.rs b/core/translate/compound_select.rs index 43f96d647..3f0247262 100644 --- a/core/translate/compound_select.rs +++ b/core/translate/compound_select.rs @@ -1,7 +1,8 @@ use crate::schema::{Index, IndexColumn, Schema}; use crate::translate::emitter::{emit_query, LimitCtx, TranslateCtx}; +use crate::translate::expr::translate_expr; use crate::translate::plan::{Plan, QueryDestination, SelectPlan}; -use crate::translate::result_row::{build_limit_offset_expr, try_fold_expr_to_i64}; +use crate::translate::result_row::try_fold_expr_to_i64; use crate::vdbe::builder::{CursorType, ProgramBuilder}; use crate::vdbe::insn::Insn; use crate::vdbe::BranchOffset; @@ -38,11 +39,18 @@ pub fn emit_program_for_compound_select( return Ok(()); } + let right_most_ctx = TranslateCtx::new( + program, + schema, + syms, + right_most.table_references.joined_tables().len(), + ); + // Each subselect shares the same limit_ctx and offset, because the LIMIT, OFFSET applies to // the entire compound select, not just a single subselect. - let limit_ctx = limit.clone().map(|limit| { + let limit_ctx = limit.as_ref().map(|limit| { let reg = program.alloc_register(); - if let Some(val) = try_fold_expr_to_i64(&limit) { + if let Some(val) = try_fold_expr_to_i64(limit) { program.emit_insn(Insn::Integer { value: val, dest: reg, @@ -52,7 +60,7 @@ pub fn emit_program_for_compound_select( let label_zero = program.allocate_label(); - build_limit_offset_expr(program, reg, &limit); + _ = translate_expr(program, None, limit, reg, &right_most_ctx.resolver); program.emit_insn(Insn::MustBeInt { reg }); @@ -87,7 +95,7 @@ pub fn emit_program_for_compound_select( let label_zero = program.allocate_label(); - build_limit_offset_expr(program, reg, offset_expr); + _ = translate_expr(program, None, offset_expr, reg, &right_most_ctx.resolver); program.emit_insn(Insn::MustBeInt { reg }); diff --git a/core/translate/delete.rs b/core/translate/delete.rs index 5dd26ee8c..b2bcda24d 100644 --- a/core/translate/delete.rs +++ b/core/translate/delete.rs @@ -2,7 +2,7 @@ use crate::schema::Table; use crate::translate::emitter::emit_program; use crate::translate::optimizer::optimize_plan; use crate::translate::plan::{DeletePlan, Operation, Plan}; -use crate::translate::planner::{parse_limit, parse_where}; +use crate::translate::planner::parse_where; use crate::util::normalize_ident; use crate::vdbe::builder::{ProgramBuilder, ProgramBuilderOpts, TableRefIdCounter}; use crate::{schema::Schema, Result, SymbolTable}; @@ -107,7 +107,8 @@ pub fn prepare_delete_plan( )?; // Parse the LIMIT/OFFSET clause - let (resolved_limit, resolved_offset) = limit.map_or(Ok((None, None)), |l| parse_limit(&l))?; + let (resolved_limit, resolved_offset) = + limit.map_or((None, None), |l| (Some(l.expr), l.offset)); let plan = DeletePlan { table_references, diff --git a/core/translate/display.rs b/core/translate/display.rs index 3e79ef223..df4c0a076 100644 --- a/core/translate/display.rs +++ b/core/translate/display.rs @@ -217,7 +217,7 @@ impl fmt::Display for UpdatePlan { )?; } } - if let Some(limit) = self.limit.clone() { + if let Some(limit) = self.limit.as_ref() { writeln!(f, "LIMIT: {limit}")?; } if let Some(ret) = &self.returning { diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 1d74eb226..90f3567c3 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -26,7 +26,7 @@ use crate::schema::{BTreeTable, Column, Schema, Table}; use crate::translate::compound_select::emit_program_for_compound_select; use crate::translate::expr::{emit_returning_results, ReturningValueRegisters}; use crate::translate::plan::{DeletePlan, Plan, QueryDestination, Search}; -use crate::translate::result_row::{build_limit_offset_expr, try_fold_expr_to_i64}; +use crate::translate::result_row::try_fold_expr_to_i64; use crate::translate::values::emit_values; use crate::util::exprs_are_equivalent; use crate::vdbe::builder::{CursorKey, CursorType, ProgramBuilder}; @@ -257,7 +257,7 @@ pub fn emit_query<'a>( // Emit subqueries first so the results can be read in the main query loop. emit_subqueries(program, t_ctx, &mut plan.table_references)?; - init_limit(program, t_ctx, plan.limit.clone(), plan.offset.clone()); + init_limit(program, t_ctx, &plan.limit, &plan.offset); // No rows will be read from source table loops if there is a constant false condition eg. WHERE 0 // however an aggregation might still happen, @@ -413,7 +413,7 @@ fn emit_program_for_delete( } } - init_limit(program, &mut t_ctx, plan.limit, None); + init_limit(program, &mut t_ctx, &plan.limit, &None); // No rows will be read from source table loops if there is a constant false condition eg. WHERE 0 let after_main_loop_label = program.allocate_label(); @@ -671,7 +671,7 @@ fn emit_program_for_update( } } - init_limit(program, &mut t_ctx, plan.limit.clone(), plan.offset.clone()); + init_limit(program, &mut t_ctx, &plan.limit, &plan.offset); let after_main_loop_label = program.allocate_label(); t_ctx.label_main_loop_end = Some(after_main_loop_label); if plan.contains_constant_false_condition { @@ -1552,8 +1552,8 @@ pub fn emit_cdc_insns( fn init_limit( program: &mut ProgramBuilder, t_ctx: &mut TranslateCtx, - limit: Option>, - offset: Option>, + limit: &Option>, + offset: &Option>, ) { if t_ctx.limit_ctx.is_none() && limit.is_some() { t_ctx.limit_ctx = Some(LimitCtx::new(program)); @@ -1563,7 +1563,7 @@ fn init_limit( }; if limit_ctx.initialize_counter { if let Some(expr) = limit { - if let Some(value) = try_fold_expr_to_i64(&expr) { + if let Some(value) = try_fold_expr_to_i64(expr) { program.emit_insn(Insn::Integer { value, dest: limit_ctx.reg_limit, @@ -1572,7 +1572,8 @@ fn init_limit( let r = limit_ctx.reg_limit; program.add_comment(program.offset(), "OFFSET expr"); let label_zero = program.allocate_label(); - build_limit_offset_expr(program, r, &expr); + + _ = translate_expr(program, None, expr, r, &t_ctx.resolver); program.emit_insn(Insn::MustBeInt { reg: r }); program.emit_insn(Insn::IfNeg { reg: r, @@ -1590,7 +1591,7 @@ fn init_limit( if t_ctx.reg_offset.is_none() { if let Some(expr) = offset { - if let Some(value) = try_fold_expr_to_i64(&expr) { + if let Some(value) = try_fold_expr_to_i64(expr) { if value != 0 { let reg = program.alloc_register(); t_ctx.reg_offset = Some(reg); @@ -1609,7 +1610,8 @@ fn init_limit( let r = reg; program.add_comment(program.offset(), "OFFSET expr"); let label_zero = program.allocate_label(); - build_limit_offset_expr(program, r, &expr); + + _ = translate_expr(program, None, expr, r, &t_ctx.resolver); program.emit_insn(Insn::MustBeInt { reg: r }); program.emit_insn(Insn::IfNeg { reg: r, diff --git a/core/translate/order_by.rs b/core/translate/order_by.rs index 4993e8010..4c17e10e5 100644 --- a/core/translate/order_by.rs +++ b/core/translate/order_by.rs @@ -117,7 +117,13 @@ pub fn emit_order_by( }); program.preassign_label_to_next_insn(sort_loop_start_label); - emit_offset(program, plan, sort_loop_next_label, t_ctx.reg_offset); + emit_offset( + program, + plan, + sort_loop_next_label, + t_ctx.reg_offset, + &t_ctx.resolver, + ); program.emit_insn(Insn::SorterData { cursor_id: sort_cursor, diff --git a/core/translate/planner.rs b/core/translate/planner.rs index f71145d93..fcc6f572c 100644 --- a/core/translate/planner.rs +++ b/core/translate/planner.rs @@ -21,8 +21,8 @@ use crate::{ }; use turso_parser::ast::Literal::Null; use turso_parser::ast::{ - self, As, Expr, FromClause, JoinType, Limit, Literal, Materialized, QualifiedName, - TableInternalId, With, + self, As, Expr, FromClause, JoinType, Literal, Materialized, QualifiedName, TableInternalId, + With, }; pub const ROWID: &str = "rowid"; @@ -1106,15 +1106,6 @@ fn parse_join( Ok(()) } -#[allow(clippy::type_complexity)] -pub fn parse_limit(limit: &Limit) -> Result<(Option>, Option>)> { - let limit_expr = Some(limit.expr.clone()); - - let offset_expr = limit.offset.clone(); - - Ok((limit_expr, offset_expr)) -} - pub fn break_predicate_at_and_boundaries(predicate: &Expr, out_predicates: &mut Vec) { match predicate { Expr::Binary(left, ast::Operator::And, right) => { diff --git a/core/translate/result_row.rs b/core/translate/result_row.rs index 40e9af6f7..9fdbdb38c 100644 --- a/core/translate/result_row.rs +++ b/core/translate/result_row.rs @@ -1,10 +1,9 @@ use turso_parser::ast::{Expr, Literal, Name, Operator, UnaryOperator}; use crate::{ - error::SQLITE_CONSTRAINT, vdbe::{ builder::ProgramBuilder, - insn::{CmpInsFlags, IdxInsertFlags, InsertFlags, Insn}, + insn::{IdxInsertFlags, InsertFlags, Insn}, BranchOffset, }, Result, @@ -33,7 +32,7 @@ pub fn emit_select_result( limit_ctx: Option, ) -> Result<()> { if let (Some(jump_to), Some(_)) = (offset_jump_to, label_on_limit_reached) { - emit_offset(program, plan, jump_to, reg_offset); + emit_offset(program, plan, jump_to, reg_offset, resolver); } let start_reg = reg_result_cols_start; @@ -166,12 +165,13 @@ pub fn emit_offset( plan: &SelectPlan, jump_to: BranchOffset, reg_offset: Option, + resolver: &Resolver, ) { let Some(offset_expr) = &plan.offset else { return; }; - if let Some(val) = try_fold_expr_to_i64(&offset_expr.clone()) { + if let Some(val) = try_fold_expr_to_i64(offset_expr) { if val > 0 { program.add_comment(program.offset(), "OFFSET const"); program.emit_insn(Insn::IfPos { @@ -189,7 +189,7 @@ pub fn emit_offset( let label_zero = program.allocate_label(); - build_limit_offset_expr(program, r, offset_expr); + _ = translate_expr(program, None, offset_expr, r, resolver); program.emit_insn(Insn::MustBeInt { reg: r }); @@ -212,126 +212,6 @@ pub fn emit_offset( program.emit_insn(Insn::Integer { value: 0, dest: r }); } -pub fn build_limit_offset_expr(program: &mut ProgramBuilder, r: usize, expr: &Expr) { - match expr { - Expr::Literal(Literal::Numeric(n)) => { - let value = n.parse::().unwrap_or_else(|_| { - program.emit_insn(Insn::Halt { - err_code: SQLITE_CONSTRAINT, - description: "invalid numeric literal".into(), - }); - 0 - }); - program.emit_int(value, r); - } - Expr::Literal(Literal::Null) => { - program.emit_int(0, r); - } - Expr::Id(Name::Ident(s)) => { - let lowered = s.to_ascii_lowercase(); - if lowered == "true" { - program.emit_int(1, r); - } else if lowered == "false" { - program.emit_int(0, r); - } else { - program.emit_insn(Insn::Halt { - err_code: SQLITE_CONSTRAINT, - description: format!("invalid boolean string literal: {s}"), - }); - } - } - Expr::Unary(UnaryOperator::Negative, inner) => { - let inner_reg = program.alloc_register(); - build_limit_offset_expr(program, inner_reg, inner); - - let neg_one_reg = program.alloc_register(); - program.emit_int(-1, neg_one_reg); - - program.emit_insn(Insn::Multiply { - lhs: inner_reg, - rhs: neg_one_reg, - dest: r, - }); - } - Expr::Unary(UnaryOperator::Positive, inner) => { - let inner_reg = program.alloc_register(); - build_limit_offset_expr(program, inner_reg, inner); - program.emit_insn(Insn::Copy { - src_reg: inner_reg, - dst_reg: r, - extra_amount: 0, - }); - } - Expr::Binary(left, op, right) => { - let left_reg = program.alloc_register(); - let right_reg = program.alloc_register(); - build_limit_offset_expr(program, left_reg, left); - build_limit_offset_expr(program, right_reg, right); - - match op { - Operator::Add => { - program.emit_insn(Insn::Add { - lhs: left_reg, - rhs: right_reg, - dest: r, - }); - } - Operator::Subtract => { - program.emit_insn(Insn::Subtract { - lhs: left_reg, - rhs: right_reg, - dest: r, - }); - } - Operator::Multiply => { - program.emit_insn(Insn::Multiply { - lhs: left_reg, - rhs: right_reg, - dest: r, - }); - } - Operator::Divide => { - let zero_reg = program.alloc_register(); - program.emit_int(0, zero_reg); - - let ok_pc = program.allocate_label(); - program.emit_insn(Insn::Ne { - lhs: right_reg, - rhs: zero_reg, - target_pc: ok_pc, - flags: CmpInsFlags::default().jump_if_null(), - collation: None, - }); - - program.emit_insn(Insn::Halt { - err_code: SQLITE_CONSTRAINT, - description: "divide by zero".into(), - }); - - program.resolve_label(ok_pc, program.offset()); - program.emit_insn(Insn::Divide { - lhs: left_reg, - rhs: right_reg, - dest: r, - }); - } - _ => { - program.emit_insn(Insn::Halt { - err_code: SQLITE_CONSTRAINT, - description: "unsupported operator in offset expr".into(), - }); - } - } - } - _ => { - program.emit_insn(Insn::Halt { - err_code: SQLITE_CONSTRAINT, - description: "non-integer expression in offset".into(), - }); - } - } -} - #[allow(clippy::borrowed_box)] pub fn try_fold_expr_to_i64(expr: &Box) -> Option { match expr.as_ref() { diff --git a/core/translate/select.rs b/core/translate/select.rs index d276471aa..65344fc41 100644 --- a/core/translate/select.rs +++ b/core/translate/select.rs @@ -8,8 +8,8 @@ use crate::schema::Table; use crate::translate::optimizer::optimize_plan; use crate::translate::plan::{Aggregate, GroupBy, Plan, ResultSetColumn, SelectPlan}; use crate::translate::planner::{ - bind_column_references, break_predicate_at_and_boundaries, parse_from, parse_limit, - parse_where, resolve_aggregates, + bind_column_references, break_predicate_at_and_boundaries, parse_from, parse_where, + resolve_aggregates, }; use crate::util::normalize_ident; use crate::vdbe::builder::{ProgramBuilderOpts, TableRefIdCounter}; @@ -151,8 +151,7 @@ pub fn prepare_select_plan( } let (limit, offset) = select .limit - .as_ref() - .map_or(Ok((None, None)), parse_limit)?; + .map_or((None, None), |l| (Some(l.expr), l.offset)); // FIXME: handle ORDER BY for compound selects if !select.order_by.is_empty() { @@ -622,7 +621,7 @@ fn prepare_one_select_plan( plan.order_by = key; // Parse the LIMIT/OFFSET clause - (plan.limit, plan.offset) = limit.as_ref().map_or(Ok((None, None)), parse_limit)?; + (plan.limit, plan.offset) = limit.map_or((None, None), |l| (Some(l.expr), l.offset)); // Return the unoptimized query plan Ok(plan) diff --git a/core/translate/update.rs b/core/translate/update.rs index f94ebe118..96f4769a5 100644 --- a/core/translate/update.rs +++ b/core/translate/update.rs @@ -21,8 +21,7 @@ use super::plan::{ ColumnUsedMask, IterationDirection, JoinedTable, Plan, ResultSetColumn, TableReferences, UpdatePlan, }; -use super::planner::bind_column_references; -use super::planner::{parse_limit, parse_where}; +use super::planner::{bind_column_references, parse_where}; /* * Update is simple. By default we scan the table, and for each row, we check the WHERE * clause. If it evaluates to true, we build the new record with the updated value and insert. @@ -331,7 +330,10 @@ pub fn prepare_update_plan( }; // Parse the LIMIT/OFFSET clause - let (limit, offset) = body.limit.as_ref().map_or(Ok((None, None)), parse_limit)?; + let (limit, offset) = body + .limit + .as_ref() + .map_or((None, None), |l| (Some(l.expr.clone()), l.offset.clone())); // Check what indexes will need to be updated by checking set_clauses and see // if a column is contained in an index. diff --git a/core/translate/values.rs b/core/translate/values.rs index 8315290e6..63f90055e 100644 --- a/core/translate/values.rs +++ b/core/translate/values.rs @@ -34,7 +34,7 @@ fn emit_values_when_single_row( t_ctx: &TranslateCtx, ) -> Result { let end_label = program.allocate_label(); - emit_offset(program, plan, end_label, t_ctx.reg_offset); + emit_offset(program, plan, end_label, t_ctx.reg_offset, &t_ctx.resolver); let first_row = &plan.values[0]; let row_len = first_row.len(); let start_reg = program.alloc_registers(row_len); @@ -87,7 +87,7 @@ fn emit_toplevel_values( }); let goto_label = program.allocate_label(); - emit_offset(program, plan, goto_label, t_ctx.reg_offset); + emit_offset(program, plan, goto_label, t_ctx.reg_offset, &t_ctx.resolver); let row_len = plan.values[0].len(); let copy_start_reg = program.alloc_registers(row_len); for i in 0..row_len { From 51d40092db459c30a5340aa0a68dae385d1da7dd Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Sun, 24 Aug 2025 17:55:30 +0530 Subject: [PATCH 11/13] add empty table references, and error out in case if the table references are present in limit/offset --- core/translate/delete.rs | 4 ++-- core/translate/planner.rs | 14 ++++++++++++++ core/translate/select.rs | 10 +++++----- core/translate/update.rs | 5 +++-- 4 files changed, 24 insertions(+), 9 deletions(-) diff --git a/core/translate/delete.rs b/core/translate/delete.rs index b2bcda24d..8f9cf48ca 100644 --- a/core/translate/delete.rs +++ b/core/translate/delete.rs @@ -2,7 +2,7 @@ use crate::schema::Table; use crate::translate::emitter::emit_program; use crate::translate::optimizer::optimize_plan; use crate::translate::plan::{DeletePlan, Operation, Plan}; -use crate::translate::planner::parse_where; +use crate::translate::planner::{parse_limit, parse_where}; use crate::util::normalize_ident; use crate::vdbe::builder::{ProgramBuilder, ProgramBuilderOpts, TableRefIdCounter}; use crate::{schema::Schema, Result, SymbolTable}; @@ -108,7 +108,7 @@ pub fn prepare_delete_plan( // Parse the LIMIT/OFFSET clause let (resolved_limit, resolved_offset) = - limit.map_or((None, None), |l| (Some(l.expr), l.offset)); + limit.map_or(Ok((None, None)), |mut l| parse_limit(&mut l, connection))?; let plan = DeletePlan { table_references, diff --git a/core/translate/planner.rs b/core/translate/planner.rs index fcc6f572c..9757c05f1 100644 --- a/core/translate/planner.rs +++ b/core/translate/planner.rs @@ -12,6 +12,7 @@ use super::{ }; use crate::translate::expr::WalkControl; use crate::{ + ast::Limit, function::Func, schema::{Schema, Table}, translate::expr::walk_expr_mut, @@ -1138,3 +1139,16 @@ where } Ok(None) } + +#[allow(clippy::type_complexity)] +pub fn parse_limit( + limit: &mut Limit, + connection: &std::sync::Arc, +) -> Result<(Option>, Option>)> { + let mut empty_refs = TableReferences::new(Vec::new(), Vec::new()); + bind_column_references(&mut limit.expr, &mut empty_refs, None, connection)?; + if let Some(ref mut off_expr) = limit.offset { + bind_column_references(off_expr, &mut empty_refs, None, connection)?; + } + Ok((Some(limit.expr.clone()), limit.offset.clone())) +} diff --git a/core/translate/select.rs b/core/translate/select.rs index 65344fc41..21330c53d 100644 --- a/core/translate/select.rs +++ b/core/translate/select.rs @@ -8,8 +8,8 @@ use crate::schema::Table; use crate::translate::optimizer::optimize_plan; use crate::translate::plan::{Aggregate, GroupBy, Plan, ResultSetColumn, SelectPlan}; use crate::translate::planner::{ - bind_column_references, break_predicate_at_and_boundaries, parse_from, parse_where, - resolve_aggregates, + bind_column_references, break_predicate_at_and_boundaries, parse_from, parse_limit, + parse_where, resolve_aggregates, }; use crate::util::normalize_ident; use crate::vdbe::builder::{ProgramBuilderOpts, TableRefIdCounter}; @@ -151,7 +151,7 @@ pub fn prepare_select_plan( } let (limit, offset) = select .limit - .map_or((None, None), |l| (Some(l.expr), l.offset)); + .map_or(Ok((None, None)), |mut l| parse_limit(&mut l, connection))?; // FIXME: handle ORDER BY for compound selects if !select.order_by.is_empty() { @@ -621,8 +621,8 @@ fn prepare_one_select_plan( plan.order_by = key; // Parse the LIMIT/OFFSET clause - (plan.limit, plan.offset) = limit.map_or((None, None), |l| (Some(l.expr), l.offset)); - + (plan.limit, plan.offset) = + limit.map_or(Ok((None, None)), |mut l| parse_limit(&mut l, connection))?; // Return the unoptimized query plan Ok(plan) } diff --git a/core/translate/update.rs b/core/translate/update.rs index 96f4769a5..676fd37ca 100644 --- a/core/translate/update.rs +++ b/core/translate/update.rs @@ -4,6 +4,7 @@ use std::sync::Arc; use crate::schema::{BTreeTable, Column, Type}; use crate::translate::optimizer::optimize_select_plan; use crate::translate::plan::{Operation, QueryDestination, Scan, Search, SelectPlan}; +use crate::translate::planner::parse_limit; use crate::vdbe::builder::CursorType; use crate::{ bail_parse_error, @@ -332,8 +333,8 @@ pub fn prepare_update_plan( // Parse the LIMIT/OFFSET clause let (limit, offset) = body .limit - .as_ref() - .map_or((None, None), |l| (Some(l.expr.clone()), l.offset.clone())); + .as_mut() + .map_or(Ok((None, None)), |l| parse_limit(l, connection))?; // Check what indexes will need to be updated by checking set_clauses and see // if a column is contained in an index. From 05267454dc0a978717c25dbb37755f818f498268 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Mon, 25 Aug 2025 18:02:34 +0530 Subject: [PATCH 12/13] remove redundant step during limit/offset evaluation and add test coverage most of the datatypes and some expression --- core/translate/compound_select.rs | 38 --------------------- core/translate/emitter.rs | 26 ++------------ core/translate/result_row.rs | 14 -------- testing/select.test | 56 +++++++++++++++++++++++++++++++ 4 files changed, 58 insertions(+), 76 deletions(-) diff --git a/core/translate/compound_select.rs b/core/translate/compound_select.rs index 3f0247262..8f816cd1f 100644 --- a/core/translate/compound_select.rs +++ b/core/translate/compound_select.rs @@ -57,27 +57,8 @@ pub fn emit_program_for_compound_select( }); } else { program.add_comment(program.offset(), "OFFSET expr"); - - let label_zero = program.allocate_label(); - _ = translate_expr(program, None, limit, reg, &right_most_ctx.resolver); - program.emit_insn(Insn::MustBeInt { reg }); - - program.emit_insn(Insn::IfNeg { - reg, - target_pc: label_zero, - }); - program.emit_insn(Insn::IsNull { - reg, - target_pc: label_zero, - }); - - program.preassign_label_to_next_insn(label_zero); - program.emit_insn(Insn::Integer { - value: 0, - dest: reg, - }); } LimitCtx::new_shared(reg) }); @@ -92,27 +73,8 @@ pub fn emit_program_for_compound_select( }); } else { program.add_comment(program.offset(), "OFFSET expr"); - - let label_zero = program.allocate_label(); - _ = translate_expr(program, None, offset_expr, reg, &right_most_ctx.resolver); - program.emit_insn(Insn::MustBeInt { reg }); - - program.emit_insn(Insn::IfNeg { - reg, - target_pc: label_zero, - }); - program.emit_insn(Insn::IsNull { - reg, - target_pc: label_zero, - }); - - program.preassign_label_to_next_insn(label_zero); - program.emit_insn(Insn::Integer { - value: 0, - dest: reg, - }); } let combined_reg = program.alloc_register(); diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 90f3567c3..02d613beb 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -1571,20 +1571,8 @@ fn init_limit( } else { let r = limit_ctx.reg_limit; program.add_comment(program.offset(), "OFFSET expr"); - let label_zero = program.allocate_label(); - _ = translate_expr(program, None, expr, r, &t_ctx.resolver); program.emit_insn(Insn::MustBeInt { reg: r }); - program.emit_insn(Insn::IfNeg { - reg: r, - target_pc: label_zero, - }); - program.emit_insn(Insn::IsNull { - reg: r, - target_pc: label_zero, - }); - program.preassign_label_to_next_insn(label_zero); - program.emit_insn(Insn::Integer { value: 0, dest: r }); } } } @@ -1608,21 +1596,11 @@ fn init_limit( let reg = program.alloc_register(); t_ctx.reg_offset = Some(reg); let r = reg; - program.add_comment(program.offset(), "OFFSET expr"); - let label_zero = program.allocate_label(); + program.add_comment(program.offset(), "OFFSET expr"); _ = translate_expr(program, None, expr, r, &t_ctx.resolver); program.emit_insn(Insn::MustBeInt { reg: r }); - program.emit_insn(Insn::IfNeg { - reg: r, - target_pc: label_zero, - }); - program.emit_insn(Insn::IsNull { - reg: r, - target_pc: label_zero, - }); - program.preassign_label_to_next_insn(label_zero); - program.emit_insn(Insn::Integer { value: 0, dest: r }); + let combined_reg = program.alloc_register(); t_ctx.reg_limit_offset_sum = Some(combined_reg); program.emit_insn(Insn::OffsetLimit { diff --git a/core/translate/result_row.rs b/core/translate/result_row.rs index 9fdbdb38c..2e636caca 100644 --- a/core/translate/result_row.rs +++ b/core/translate/result_row.rs @@ -187,29 +187,15 @@ pub fn emit_offset( program.add_comment(program.offset(), "OFFSET expr"); - let label_zero = program.allocate_label(); - _ = translate_expr(program, None, offset_expr, r, resolver); program.emit_insn(Insn::MustBeInt { reg: r }); - program.emit_insn(Insn::IfNeg { - reg: r, - target_pc: label_zero, - }); - program.emit_insn(Insn::IsNull { - reg: r, - target_pc: label_zero, - }); - program.emit_insn(Insn::IfPos { reg: r, target_pc: jump_to, decrement_by: 1, }); - - program.preassign_label_to_next_insn(label_zero); - program.emit_insn(Insn::Integer { value: 0, dest: r }); } #[allow(clippy::borrowed_box)] diff --git a/testing/select.test b/testing/select.test index d33beb5c4..5c3c7930b 100755 --- a/testing/select.test +++ b/testing/select.test @@ -749,3 +749,59 @@ do_execsql_test_on_specific_db {:memory:} select-in-complex { SELECT * FROM test_table WHERE category IN ('A', 'B') AND value IN (10, 30, 40); } {1|A|10 3|A|30} + +do_execsql_test_on_specific_db {:memory:} limit-complex-exprs { + CREATE TABLE t (x INTEGER); + INSERT INTO t VALUES (10),(20),(30),(40),(50); + SELECT x FROM t ORDER BY x LIMIT 1; + SELECT x FROM t ORDER BY x LIMIT '2'; + SELECT x FROM t ORDER BY x LIMIT true; + SELECT x FROM t ORDER BY x LIMIT 1+2+3; + SELECT x FROM t ORDER BY x LIMIT 1-2-3; + SELECT x FROM t ORDER BY x LIMIT (2+3)*2; + SELECT x FROM t ORDER BY x LIMIT true + 2; + SELECT x FROM t ORDER BY x LIMIT '2' * 2 + 1; + SELECT x FROM t ORDER BY x LIMIT false + 4; + SELECT x FROM t ORDER BY x LIMIT (1 + '2') * (1 + 3) - (5/5); + SELECT x FROM t ORDER BY x LIMIT ('false' + 2); + SELECT * FROM t LIMIT COALESCE(NULL, 0+1); + +} {10 +10 +20 +10 +10 +20 +30 +40 +50 +10 +20 +30 +40 +50 +10 +20 +30 +40 +50 +10 +20 +30 +10 +20 +30 +40 +50 +10 +20 +30 +40 +10 +20 +30 +40 +50 +10 +20 +10} From 881b98630214501925854f3a84547f389886bc83 Mon Sep 17 00:00:00 2001 From: bit-aloo Date: Mon, 25 Aug 2025 21:20:34 +0530 Subject: [PATCH 13/13] improve the limi exprs test with foreach block --- testing/select.test | 72 +++++++++++---------------------------------- 1 file changed, 17 insertions(+), 55 deletions(-) diff --git a/testing/select.test b/testing/select.test index 5c3c7930b..6efc3e061 100755 --- a/testing/select.test +++ b/testing/select.test @@ -750,58 +750,20 @@ do_execsql_test_on_specific_db {:memory:} select-in-complex { } {1|A|10 3|A|30} -do_execsql_test_on_specific_db {:memory:} limit-complex-exprs { - CREATE TABLE t (x INTEGER); - INSERT INTO t VALUES (10),(20),(30),(40),(50); - SELECT x FROM t ORDER BY x LIMIT 1; - SELECT x FROM t ORDER BY x LIMIT '2'; - SELECT x FROM t ORDER BY x LIMIT true; - SELECT x FROM t ORDER BY x LIMIT 1+2+3; - SELECT x FROM t ORDER BY x LIMIT 1-2-3; - SELECT x FROM t ORDER BY x LIMIT (2+3)*2; - SELECT x FROM t ORDER BY x LIMIT true + 2; - SELECT x FROM t ORDER BY x LIMIT '2' * 2 + 1; - SELECT x FROM t ORDER BY x LIMIT false + 4; - SELECT x FROM t ORDER BY x LIMIT (1 + '2') * (1 + 3) - (5/5); - SELECT x FROM t ORDER BY x LIMIT ('false' + 2); - SELECT * FROM t LIMIT COALESCE(NULL, 0+1); - -} {10 -10 -20 -10 -10 -20 -30 -40 -50 -10 -20 -30 -40 -50 -10 -20 -30 -40 -50 -10 -20 -30 -10 -20 -30 -40 -50 -10 -20 -30 -40 -10 -20 -30 -40 -50 -10 -20 -10} +foreach {testname limit ans} { + limit-const-1 1 {1} + limit-text-2 '2' {1 2} + limit-bool-true true {1} + limit-expr-add 1+2+3 {1 2 3 4 5 6} + limit-expr-sub 5-2-3 {} + limit-expr-paren (1+1)*2 {1 2 3 4} + limit-bool-add true+2 {1 2 3} + limit-text-math '2'*2+1 {1 2 3 4 5} + limit-bool-false-add false+4 {1 2 3 4} + limit-mixed-math (1+'1')*(1+1)-(5/5) {1 2 3} + limit-text-bool ('false'+2) {1 2} + limit-coalesce COALESCE(NULL,0+1) {1} +} { + do_execsql_test limit-complex-exprs-$testname \ + "SELECT id FROM users ORDER BY id LIMIT $limit" $ans +} \ No newline at end of file