From 70965f4b28644ea4c02de7242931a854060d139c Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Mon, 26 May 2025 16:24:16 +0300 Subject: [PATCH 1/2] Insn::Return: add possibility to fallthrough on non-integer values as per sqlite spec --- core/translate/group_by.rs | 4 ++++ core/vdbe/execute.rs | 15 +++++++++++---- core/vdbe/explain.rs | 7 +++++-- core/vdbe/insn.rs | 3 +++ 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/core/translate/group_by.rs b/core/translate/group_by.rs index 971ebe403..7b54ab3ef 100644 --- a/core/translate/group_by.rs +++ b/core/translate/group_by.rs @@ -760,6 +760,7 @@ pub fn group_by_emit_row_phase<'a>( }); program.emit_insn(Insn::Return { return_reg: registers.reg_subrtn_acc_output_return_offset, + can_fallthrough: false, }); program.resolve_label(labels.label_subrtn_acc_output, program.offset()); @@ -784,6 +785,7 @@ pub fn group_by_emit_row_phase<'a>( } program.emit_insn(Insn::Return { return_reg: registers.reg_subrtn_acc_output_return_offset, + can_fallthrough: false, }); // Finalize aggregate values for output @@ -916,6 +918,7 @@ pub fn group_by_emit_row_phase<'a>( program.emit_insn(Insn::Return { return_reg: registers.reg_subrtn_acc_output_return_offset, + can_fallthrough: false, }); // Subroutine to clear accumulators for a new group @@ -955,6 +958,7 @@ pub fn group_by_emit_row_phase<'a>( }); program.emit_insn(Insn::Return { return_reg: registers.reg_subrtn_acc_clear_return_offset, + can_fallthrough: false, }); program.preassign_label_to_next_insn(labels.label_group_by_end); Ok(()) diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index dddfd2098..68173b3bd 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -1809,7 +1809,11 @@ pub fn op_return( pager: &Rc, mv_store: Option<&Rc>, ) -> Result { - let Insn::Return { return_reg } = insn else { + let Insn::Return { + return_reg, + can_fallthrough, + } = insn + else { unreachable!("unexpected Insn {:?}", insn) }; if let Value::Integer(pc) = state.registers[*return_reg].get_owned_value() { @@ -1818,9 +1822,12 @@ pub fn op_return( .unwrap_or_else(|_| panic!("Return register is negative: {}", pc)); state.pc = pc; } else { - return Err(LimboError::InternalError( - "Return register is not an integer".to_string(), - )); + if !*can_fallthrough { + return Err(LimboError::InternalError( + "Return register is not an integer".to_string(), + )); + } + state.pc += 1; } Ok(InsnFunctionStepResult::Step) } diff --git a/core/vdbe/explain.rs b/core/vdbe/explain.rs index 4055fb072..687cd42ff 100644 --- a/core/vdbe/explain.rs +++ b/core/vdbe/explain.rs @@ -625,11 +625,14 @@ pub fn insn_to_str( 0, "".to_string(), ), - Insn::Return { return_reg } => ( + Insn::Return { + return_reg, + can_fallthrough, + } => ( "Return", *return_reg as i32, 0, - 0, + *can_fallthrough as i32, Value::build_text(""), 0, "".to_string(), diff --git a/core/vdbe/insn.rs b/core/vdbe/insn.rs index f5ff8352d..2f8c2ff42 100644 --- a/core/vdbe/insn.rs +++ b/core/vdbe/insn.rs @@ -429,8 +429,11 @@ pub enum Insn { }, /// Returns to the program counter stored in register 'return_reg'. + /// If can_fallthrough is true, fall through to the next instruction + /// if return_reg does not contain an integer value. Otherwise raise an error. Return { return_reg: usize, + can_fallthrough: bool, }, /// Write an integer value into a register. From 6914d611808bfed8d7e088791beb37eb203a7562 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Tue, 27 May 2025 19:06:21 +0300 Subject: [PATCH 2/2] allow calling op_null with Insn::BeginSubrtn --- core/vdbe/execute.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 68173b3bd..7aaab3882 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -322,15 +322,17 @@ pub fn op_null( pager: &Rc, mv_store: Option<&Rc>, ) -> Result { - let Insn::Null { dest, dest_end } = insn else { - unreachable!("unexpected Insn {:?}", insn) - }; - if let Some(dest_end) = dest_end { - for i in *dest..=*dest_end { - state.registers[i] = Register::Value(Value::Null); + match insn { + Insn::Null { dest, dest_end } | Insn::BeginSubrtn { dest, dest_end } => { + if let Some(dest_end) = dest_end { + for i in *dest..=*dest_end { + state.registers[i] = Register::Value(Value::Null); + } + } else { + state.registers[*dest] = Register::Value(Value::Null); + } } - } else { - state.registers[*dest] = Register::Value(Value::Null); + _ => unreachable!("unexpected Insn {:?}", insn), } state.pc += 1; Ok(InsnFunctionStepResult::Step)