Merge 'Fix zero limit' from Nikita Sivukhin

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

Reviewed-by: Preston Thorpe <preston@turso.tech>

Closes #3421

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" (

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,
@@ -140,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,
@@ -366,6 +367,8 @@ fn emit_compound_select(
         }
     }
 
+    program.preassign_label_to_next_insn(compound_select_end);
+
     Ok(())
 }
 

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)?;
 
@@ -256,21 +248,23 @@ pub fn emit_query<'a>(
     plan: &'a mut SelectPlan,
     t_ctx: &mut TranslateCtx<'a>,
 ) -> Result<usize> {
+    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)?;
 
-    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 +418,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 +706,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 +1738,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 +1789,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,

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::<Vec<_>>());
+        let mut rows = Vec::new();
+        loop {
+            match stmt.step()? {
+                StepResult::Row => {
+                    let row = stmt.row().unwrap();
+                    rows.push(row.get_values().cloned().collect::<Vec<_>>());
+                }
+                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(())
 }