mirror of
https://github.com/aljazceru/turso.git
synced 2026-02-20 15:35:29 +01:00
Merge 'Fix DELETE not emitting constant WhereTerms' from Pedro Muniz
Fixes DELETE not emitting conditional jumps at all if the associated WhereTerm is a constant, e.g. ```sql limbo> create table t(x); limbo> explain DELETE FROM t WHERE 5-5; addr opcode p1 p2 p3 p4 p5 comment ---- ----------------- ---- ---- ---- ------------- -- ------- 0 Init 0 7 0 0 Start at 7 1 OpenWrite 0 2 0 0 root=2; t 2 Rewind 0 6 0 0 Rewind table t 3 RowId 0 1 0 0 r[1]=t.rowid 4 Delete 0 0 0 0 5 Next 0 3 0 0 6 Halt 0 0 0 0 7 Transaction 0 1 0 0 write=true 8 Goto 0 1 0 0 ``` I was adding more stuff to the simulator in a Branch of mine, and I caught this error with delete. Upstreaming the fix here. As we do with Update, I added the translation step for the `WhereTerms` of the query. Edit: Closes #1732. Closes #1733. Closes #1734. Closes #1735. Closes #1736. Closes #1738. Closes #1739. Closes #1740. Edit: Also pushes constant where term translation to `init_loop` for Update and Select as well. Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com> Closes #1746
This commit is contained in:
@@ -7,7 +7,7 @@ use limbo_sqlite3_parser::ast::{self, Expr};
|
||||
use tracing::{instrument, Level};
|
||||
|
||||
use super::aggregation::emit_ungrouped_aggregation;
|
||||
use super::expr::{translate_condition_expr, translate_expr, ConditionMetadata};
|
||||
use super::expr::translate_expr;
|
||||
use super::group_by::{
|
||||
group_by_agg_phase, group_by_emit_row_phase, init_group_by, GroupByMetadata, GroupByRowSource,
|
||||
};
|
||||
@@ -333,6 +333,7 @@ pub fn emit_query<'a>(
|
||||
&mut plan.aggregates,
|
||||
plan.group_by.as_ref(),
|
||||
OperationMode::SELECT,
|
||||
&plan.where_clause,
|
||||
)?;
|
||||
|
||||
if plan.is_simple_count() {
|
||||
@@ -340,27 +341,6 @@ pub fn emit_query<'a>(
|
||||
return Ok(t_ctx.reg_result_cols_start.unwrap());
|
||||
}
|
||||
|
||||
for where_term in plan
|
||||
.where_clause
|
||||
.iter()
|
||||
.filter(|wt| wt.should_eval_before_loop(&plan.join_order))
|
||||
{
|
||||
let jump_target_when_true = program.allocate_label();
|
||||
let condition_metadata = ConditionMetadata {
|
||||
jump_if_condition_is_true: false,
|
||||
jump_target_when_false: after_main_loop_label,
|
||||
jump_target_when_true,
|
||||
};
|
||||
translate_condition_expr(
|
||||
program,
|
||||
&plan.table_references,
|
||||
&where_term.expr,
|
||||
condition_metadata,
|
||||
&t_ctx.resolver,
|
||||
)?;
|
||||
program.preassign_label_to_next_insn(jump_target_when_true);
|
||||
}
|
||||
|
||||
// Set up main query execution loop
|
||||
open_loop(
|
||||
program,
|
||||
@@ -449,6 +429,7 @@ fn emit_program_for_delete(
|
||||
&mut [],
|
||||
None,
|
||||
OperationMode::DELETE,
|
||||
&plan.where_clause,
|
||||
)?;
|
||||
|
||||
// Set up main query execution loop
|
||||
@@ -629,6 +610,7 @@ fn emit_program_for_update(
|
||||
&mut [],
|
||||
None,
|
||||
OperationMode::UPDATE,
|
||||
&plan.where_clause,
|
||||
)?;
|
||||
// Open indexes for update.
|
||||
let mut index_cursors = Vec::with_capacity(plan.indexes_to_update.len());
|
||||
@@ -723,26 +705,6 @@ fn emit_update_insns(
|
||||
},
|
||||
};
|
||||
|
||||
for cond in plan
|
||||
.where_clause
|
||||
.iter()
|
||||
.filter(|c| c.should_eval_before_loop(&[JoinOrderMember::default()]))
|
||||
{
|
||||
let jump_target = program.allocate_label();
|
||||
let meta = ConditionMetadata {
|
||||
jump_if_condition_is_true: false,
|
||||
jump_target_when_true: jump_target,
|
||||
jump_target_when_false: t_ctx.label_main_loop_end.unwrap(),
|
||||
};
|
||||
translate_condition_expr(
|
||||
program,
|
||||
&plan.table_references,
|
||||
&cond.expr,
|
||||
meta,
|
||||
&t_ctx.resolver,
|
||||
)?;
|
||||
program.preassign_label_to_next_insn(jump_target);
|
||||
}
|
||||
let beg = program.alloc_registers(
|
||||
table_ref.table.columns().len()
|
||||
+ if is_virtual {
|
||||
@@ -809,27 +771,6 @@ fn emit_update_insns(
|
||||
});
|
||||
}
|
||||
|
||||
for cond in plan
|
||||
.where_clause
|
||||
.iter()
|
||||
.filter(|c| c.should_eval_before_loop(&[JoinOrderMember::default()]))
|
||||
{
|
||||
let jump_target = program.allocate_label();
|
||||
let meta = ConditionMetadata {
|
||||
jump_if_condition_is_true: false,
|
||||
jump_target_when_true: jump_target,
|
||||
jump_target_when_false: loop_labels.next,
|
||||
};
|
||||
translate_condition_expr(
|
||||
program,
|
||||
&plan.table_references,
|
||||
&cond.expr,
|
||||
meta,
|
||||
&t_ctx.resolver,
|
||||
)?;
|
||||
program.preassign_label_to_next_insn(jump_target);
|
||||
}
|
||||
|
||||
// we scan a column at a time, loading either the column's values, or the new value
|
||||
// from the Set expression, into registers so we can emit a MakeRecord and update the row.
|
||||
let start = if is_virtual { beg + 2 } else { beg + 1 };
|
||||
|
||||
@@ -111,6 +111,7 @@ pub fn init_loop(
|
||||
aggregates: &mut [Aggregate],
|
||||
group_by: Option<&GroupBy>,
|
||||
mode: OperationMode,
|
||||
where_clause: &[WhereTerm],
|
||||
) -> Result<()> {
|
||||
assert!(
|
||||
t_ctx.meta_left_joins.len() == tables.joined_tables().len(),
|
||||
@@ -330,6 +331,20 @@ pub fn init_loop(
|
||||
}
|
||||
}
|
||||
|
||||
for cond in where_clause
|
||||
.iter()
|
||||
.filter(|c| c.should_eval_before_loop(&[JoinOrderMember::default()]))
|
||||
{
|
||||
let jump_target = program.allocate_label();
|
||||
let meta = ConditionMetadata {
|
||||
jump_if_condition_is_true: false,
|
||||
jump_target_when_true: jump_target,
|
||||
jump_target_when_false: t_ctx.label_main_loop_end.unwrap(),
|
||||
};
|
||||
translate_condition_expr(program, &tables, &cond.expr, meta, &t_ctx.resolver)?;
|
||||
program.preassign_label_to_next_insn(jump_target);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
16
testing/delete.test
Normal file → Executable file
16
testing/delete.test
Normal file → Executable file
@@ -62,3 +62,19 @@ if {[info exists ::env(SQLITE_EXEC)] && $::env(SQLITE_EXEC) eq "scripts/limbo-sq
|
||||
SELECT * FROM t;
|
||||
} {}
|
||||
}
|
||||
|
||||
do_execsql_test_on_specific_db {:memory:} delete_where_falsy {
|
||||
CREATE TABLE resourceful_schurz (diplomatic_kaplan BLOB);
|
||||
INSERT INTO resourceful_schurz VALUES (X'696E646570656E64656E745F6A6165636B6C65'), (X'67656E65726F75735F62617262616E65677261'), (X'73757065725F74616E6E656E6261756D'), (X'6D6F76696E675F6E616F756D6F76'), (X'7374756E6E696E675F6B62');
|
||||
INSERT INTO resourceful_schurz VALUES (X'70617373696F6E6174655F726F62696E'), (X'666169746866756C5F74686F6D6173'), (X'76696272616E745F6D69726F736C6176'), (X'737061726B6C696E675F67726179');
|
||||
DELETE FROM resourceful_schurz WHERE - x'666169746866756c5f74686f6d6173';
|
||||
SELECT * FROM resourceful_schurz;
|
||||
} {independent_jaeckle
|
||||
generous_barbanegra
|
||||
super_tannenbaum
|
||||
moving_naoumov
|
||||
stunning_kb
|
||||
passionate_robin
|
||||
faithful_thomas
|
||||
vibrant_miroslav
|
||||
sparkling_gray}
|
||||
|
||||
Reference in New Issue
Block a user