From 70e18ce3f756055ecad13ae99617215bf3df33e0 Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Mon, 29 Sep 2025 15:30:04 +0400 Subject: [PATCH 1/5] validate zero limit at the beginning in the VDBE program - before, we validated that condition during program emit - which works for fixed values of parameters but doesn't work with variables provided externally to the prepared statement --- .../packages/native/promise.test.ts | 19 +++++++ core/translate/compound_select.rs | 7 --- core/translate/emitter.rs | 50 ++++++++----------- 3 files changed, 39 insertions(+), 37 deletions(-) diff --git a/bindings/javascript/packages/native/promise.test.ts b/bindings/javascript/packages/native/promise.test.ts index 0d05a4b11..2dc5be8fb 100644 --- a/bindings/javascript/packages/native/promise.test.ts +++ b/bindings/javascript/packages/native/promise.test.ts @@ -72,6 +72,25 @@ test('explicit connect', async () => { expect(await db.prepare("SELECT 1 as x").all()).toEqual([{ x: 1 }]); }) +test('zero-limit-bug', async () => { + const db = await connect(':memory:'); + const create = db.prepare(`CREATE TABLE users (name TEXT NOT NULL);`); + await create.run(); + + const insert = db.prepare( + `insert into "users" values (?), (?), (?);`, + ); + await insert.run('John', 'Jane', 'Jack'); + + const stmt1 = db.prepare(`select * from "users" limit ?;`); + expect(await stmt1.all(0)).toEqual([]); + let rows = [{ name: 'John' }, { name: 'Jane' }, { name: 'Jack' }, { name: 'John' }, { name: 'Jane' }, { name: 'Jack' }]; + for (const limit of [0, 1, 2, 3, 4, 5, 6, 7]) { + const stmt2 = db.prepare(`select * from "users" union all select * from "users" limit ?;`); + expect(await stmt2.all(limit)).toEqual(rows.slice(0, Math.min(limit, 6))); + } +}) + test('avg-bug', async () => { const db = await connect(':memory:'); const create = db.prepare(`create table "aggregate_table" ( diff --git a/core/translate/compound_select.rs b/core/translate/compound_select.rs index 0cac57af9..eaa312b6a 100644 --- a/core/translate/compound_select.rs +++ b/core/translate/compound_select.rs @@ -31,13 +31,6 @@ 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) { - program.result_columns = right_plan.result_columns; - program.table_references.extend(right_plan.table_references); - return Ok(()); - } - let right_most_ctx = TranslateCtx::new( program, resolver.schema, diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 1d986d572..4758b9a1c 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -234,14 +234,6 @@ fn emit_program_for_select( plan.table_references.joined_tables().len(), ); - // Trivial exit on 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(()); - } - } // Emit main parts of query emit_query(program, &mut plan, &mut t_ctx)?; @@ -264,13 +256,14 @@ 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)?; + let after_main_loop_label = program.allocate_label(); + t_ctx.label_main_loop_end = Some(after_main_loop_label); + 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, // e.g. SELECT COUNT(*) WHERE 0 returns a row with 0, not an empty result set - 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 { program.emit_insn(Insn::Goto { target_pc: after_main_loop_label, @@ -424,20 +417,12 @@ fn emit_program_for_delete( plan.table_references.joined_tables().len(), ); - // exit early 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(()); - } - } + let after_main_loop_label = program.allocate_label(); + t_ctx.label_main_loop_end = Some(after_main_loop_label); 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(); - t_ctx.label_main_loop_end = Some(after_main_loop_label); if plan.contains_constant_false_condition { program.emit_insn(Insn::Goto { target_pc: after_main_loop_label, @@ -720,18 +705,12 @@ fn emit_program_for_update( plan.table_references.joined_tables().len(), ); - // Exit on LIMIT 0 - 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); let after_main_loop_label = program.allocate_label(); t_ctx.label_main_loop_end = Some(after_main_loop_label); + + init_limit(program, &mut 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 if plan.contains_constant_false_condition { program.emit_insn(Insn::Goto { target_pc: after_main_loop_label, @@ -1758,6 +1737,7 @@ fn init_limit( let Some(limit_ctx) = &t_ctx.limit_ctx else { return; }; + if limit_ctx.initialize_counter { if let Some(expr) = limit { if let Some(value) = try_fold_expr_to_i64(expr) { @@ -1808,6 +1788,16 @@ fn init_limit( } } } + + // exit early if LIMIT 0 + let main_loop_end = t_ctx + .label_main_loop_end + .expect("label_main_loop_end must be set before init_limit"); + program.emit_insn(Insn::IfNot { + reg: limit_ctx.reg_limit, + target_pc: main_loop_end, + jump_if_null: false, + }); } /// We have `Expr`s which have *not* had column references bound to them, From 4d8ef6c63f2f5262720580776122a2f896b3cba2 Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Mon, 29 Sep 2025 15:32:08 +0400 Subject: [PATCH 2/5] extend integration test --- .../query_processing/test_read_path.rs | 47 +++++++++++-------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/tests/integration/query_processing/test_read_path.rs b/tests/integration/query_processing/test_read_path.rs index 2319aada6..cbc53cfbe 100644 --- a/tests/integration/query_processing/test_read_path.rs +++ b/tests/integration/query_processing/test_read_path.rs @@ -792,29 +792,38 @@ fn test_offset_limit_bind() -> anyhow::Result<()> { conn.execute("INSERT INTO test VALUES (5), (4), (3), (2), (1)")?; - let mut stmt = conn.prepare("SELECT * FROM test LIMIT ? OFFSET ?")?; - stmt.bind_at(1.try_into()?, Value::Integer(2)); - stmt.bind_at(2.try_into()?, Value::Integer(1)); + for (limit, offset, expected) in [ + ( + 2, + 1, + vec![ + vec![turso_core::Value::Integer(4)], + vec![turso_core::Value::Integer(3)], + ], + ), + (0, 0, vec![]), + (1, 0, vec![vec![turso_core::Value::Integer(5)]]), + (0, 1, vec![]), + (1, 1, vec![vec![turso_core::Value::Integer(4)]]), + ] { + let mut stmt = conn.prepare("SELECT * FROM test LIMIT ? OFFSET ?")?; + stmt.bind_at(1.try_into()?, Value::Integer(limit)); + stmt.bind_at(2.try_into()?, Value::Integer(offset)); - let mut rows = Vec::new(); - loop { - match stmt.step()? { - StepResult::Row => { - let row = stmt.row().unwrap(); - rows.push(row.get_values().cloned().collect::>()); + let mut rows = Vec::new(); + loop { + match stmt.step()? { + StepResult::Row => { + let row = stmt.row().unwrap(); + rows.push(row.get_values().cloned().collect::>()); + } + StepResult::IO => stmt.run_once()?, + _ => break, } - StepResult::IO => stmt.run_once()?, - _ => break, } - } - assert_eq!( - rows, - vec![ - vec![turso_core::Value::Integer(4)], - vec![turso_core::Value::Integer(3)] - ] - ); + assert_eq!(rows, expected); + } Ok(()) } From 49a5617a951694e060a09630edca1d3f66a9ca7e Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Mon, 29 Sep 2025 15:51:39 +0400 Subject: [PATCH 3/5] fix limit for compount select --- core/translate/emitter.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 4758b9a1c..4b6163784 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -248,19 +248,20 @@ pub fn emit_query<'a>( plan: &'a mut SelectPlan, t_ctx: &mut TranslateCtx<'a>, ) -> Result { + let after_main_loop_label = program.allocate_label(); + t_ctx.label_main_loop_end = Some(after_main_loop_label); + + init_limit(program, t_ctx, &plan.limit, &plan.offset); + if !plan.values.is_empty() { let reg_result_cols_start = emit_values(program, plan, t_ctx)?; + program.preassign_label_to_next_insn(after_main_loop_label); return Ok(reg_result_cols_start); } // Emit subqueries first so the results can be read in the main query loop. emit_subqueries(program, t_ctx, &mut plan.table_references)?; - let after_main_loop_label = program.allocate_label(); - t_ctx.label_main_loop_end = Some(after_main_loop_label); - - 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, // e.g. SELECT COUNT(*) WHERE 0 returns a row with 0, not an empty result set From 12863b35c4d9f4cac9a1929fcdc3cce4a258db7b Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Mon, 29 Sep 2025 16:21:35 +0400 Subject: [PATCH 4/5] fix compound select --- core/translate/compound_select.rs | 12 +++++++++++- core/translate/emitter.rs | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/core/translate/compound_select.rs b/core/translate/compound_select.rs index eaa312b6a..82a9fca8f 100644 --- a/core/translate/compound_select.rs +++ b/core/translate/compound_select.rs @@ -1,5 +1,5 @@ use crate::schema::{Index, IndexColumn, Schema}; -use crate::translate::emitter::{emit_query, LimitCtx, Resolver, TranslateCtx}; +use crate::translate::emitter::{emit_query, init_limit, LimitCtx, Resolver, TranslateCtx}; use crate::translate::expr::translate_expr; use crate::translate::plan::{Plan, QueryDestination, SelectPlan}; use crate::translate::result_row::try_fold_expr_to_i64; @@ -133,6 +133,14 @@ fn emit_compound_select( unreachable!() }; + let compound_select_end = program.allocate_label(); + if let Some(limit_ctx) = &limit_ctx { + program.emit_insn(Insn::IfNot { + reg: limit_ctx.reg_limit, + target_pc: compound_select_end, + jump_if_null: false, + }); + } let mut right_most_ctx = TranslateCtx::new( program, schema, @@ -359,6 +367,8 @@ fn emit_compound_select( } } + program.preassign_label_to_next_insn(compound_select_end); + Ok(()) } diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 4b6163784..6222c9222 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -1726,7 +1726,7 @@ pub fn emit_cdc_insns( /// 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( +pub fn init_limit( program: &mut ProgramBuilder, t_ctx: &mut TranslateCtx, limit: &Option>, From 0910483522fb43c6a5d0bd193a4776df1015c6be Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Mon, 29 Sep 2025 16:30:07 +0400 Subject: [PATCH 5/5] fix clippy --- core/translate/compound_select.rs | 2 +- core/translate/emitter.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/translate/compound_select.rs b/core/translate/compound_select.rs index 82a9fca8f..4ffbc62a9 100644 --- a/core/translate/compound_select.rs +++ b/core/translate/compound_select.rs @@ -1,5 +1,5 @@ use crate::schema::{Index, IndexColumn, Schema}; -use crate::translate::emitter::{emit_query, init_limit, LimitCtx, Resolver, TranslateCtx}; +use crate::translate::emitter::{emit_query, LimitCtx, Resolver, TranslateCtx}; use crate::translate::expr::translate_expr; use crate::translate::plan::{Plan, QueryDestination, SelectPlan}; use crate::translate::result_row::try_fold_expr_to_i64; diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 6222c9222..4b6163784 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -1726,7 +1726,7 @@ pub fn emit_cdc_insns( /// 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. -pub fn init_limit( +fn init_limit( program: &mut ProgramBuilder, t_ctx: &mut TranslateCtx, limit: &Option>,