From 625403cc2a307f855b80d3ff0b1c29d5012276be Mon Sep 17 00:00:00 2001 From: Diego Reis Date: Tue, 7 Oct 2025 16:54:50 -0300 Subject: [PATCH] Fix register reuse when called inside a coroutine - On each interaction we assume that the value is NULL, so we need to set it like so for every interaction in the list. So we force to not emit this NULL as constant; - Forces a copy so IN expressions works inside an aggregation expression. Not ideal but it works, we should work more on the query planner for sure. --- core/translate/expr.rs | 42 +++++++++++++++++++++++++++++++++--------- core/vdbe/insn.rs | 4 +++- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 2ab33a85a..99c0ad4bb 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -43,6 +43,13 @@ pub struct ReturningValueRegisters { pub num_columns: usize, } +/// Emit an instruction that is guaranteed not to be in any constant span. +/// This ensures the instruction won't be hoisted when emit_constant_insns is called. +fn emit_no_constant_insn(program: &mut ProgramBuilder, insn: Insn) { + program.constant_span_end_all(); + program.emit_insn(insn); +} + #[instrument(skip_all, level = Level::DEBUG)] fn emit_cond_jump(program: &mut ProgramBuilder, cond_meta: ConditionMetadata, reg: usize) { if cond_meta.jump_if_condition_is_true { @@ -2073,16 +2080,28 @@ pub fn translate_expr( // but wrap it with appropriate expression context handling let result_reg = target_register; - program.emit_insn(Insn::Null { - dest: result_reg, - dest_end: None, - }); - let dest_if_false = program.allocate_label(); let dest_if_null = program.allocate_label(); // won't use this label :/ let dest_if_true = program.allocate_label(); + let tmp = program.alloc_register(); + emit_no_constant_insn( + program, + Insn::Null { + dest: tmp, + dest_end: None, + }, + ); + translate_expr_no_constant_opt( + program, + referenced_tables, + &ast::Expr::Literal(ast::Literal::Null), + tmp, + resolver, + NoConstantOptReason::RegisterReuse, + )?; + // Call the core InList logic with expression-appropriate condition metadata translate_in_list( program, @@ -2101,22 +2120,27 @@ pub fn translate_expr( // condition true: set result to 1 program.emit_insn(Insn::Integer { value: 1, - dest: result_reg, + dest: tmp, }); // False path: set result to 0 program.resolve_label(dest_if_false, program.offset()); // Force integer conversion with AddImm 0 program.emit_insn(Insn::AddImm { - register: result_reg, + register: tmp, value: 0, }); if *not { program.emit_insn(Insn::Not { - reg: result_reg, - dest: result_reg, + reg: tmp, + dest: tmp, }); } program.resolve_label(dest_if_null, program.offset()); + program.emit_insn(Insn::Copy { + src_reg: tmp, + dst_reg: result_reg, + extra_amount: 0, + }); Ok(result_reg) } ast::Expr::InSelect { .. } => { diff --git a/core/vdbe/insn.rs b/core/vdbe/insn.rs index 9a5bab21c..64bae3947 100644 --- a/core/vdbe/insn.rs +++ b/core/vdbe/insn.rs @@ -853,10 +853,12 @@ pub enum Insn { db: usize, }, + /// Make a copy of register src..src+extra_amount into dst..dst+extra_amount. Copy { src_reg: usize, dst_reg: usize, - extra_amount: usize, // 0 extra_amount means we include src_reg, dst_reg..=dst_reg+amount = src_reg..=src_reg+amount + /// 0 extra_amount means we include src_reg, dst_reg..=dst_reg+amount = src_reg..=src_reg+amount + extra_amount: usize, }, /// Allocate a new b-tree.