Merge 'Fix infinite loop when inserting multiple rows' from Jussi Saurio

Due to constant instruction reshuffling introduced in #1359, it is
advisable not to do either of the following:
1. Use raw offsets as jump targets
2. Use `program.resolve_label(my_label, program.offset())` when it is
uncertain what the next instruction will be
Instead, if you want a label to point to "whatever instruction follows
the last one", you should use
`program.preassign_label_to_next_insn(label)`, which will work correctly
even with instruction rerdering

Reviewed-by: Preston Thorpe (@PThorpe92)

Closes #1474
This commit is contained in:
Jussi Saurio
2025-05-14 09:24:47 +03:00
2 changed files with 11 additions and 5 deletions

View File

@@ -10,7 +10,6 @@ use crate::schema::{IndexColumn, Table};
use crate::util::normalize_ident;
use crate::vdbe::builder::{ProgramBuilderOpts, QueryMode};
use crate::vdbe::insn::{IdxInsertFlags, RegisterOrLiteral};
use crate::vdbe::BranchOffset;
use crate::{
schema::{Column, Schema},
vdbe::{
@@ -137,7 +136,7 @@ pub fn translate_insert(
let record_register = program.alloc_register();
let halt_label = program.allocate_label();
let mut loop_start_offset = BranchOffset::Offset(0);
let loop_start_label = program.allocate_label();
let inserting_multiple_rows = values.len() > 1;
@@ -152,7 +151,7 @@ pub fn translate_insert(
start_offset: start_offset_label,
});
program.resolve_label(start_offset_label, program.offset());
program.preassign_label_to_next_insn(start_offset_label);
for (i, value) in values.iter().enumerate() {
populate_column_registers(
@@ -181,7 +180,7 @@ pub fn translate_insert(
// Main loop
// FIXME: rollback is not implemented. E.g. if you insert 2 rows and one fails to unique constraint violation,
// the other row will still be inserted.
loop_start_offset = program.offset();
program.resolve_label(loop_start_label, program.offset());
program.emit_insn(Insn::Yield {
yield_reg,
end_offset: halt_label,
@@ -397,7 +396,7 @@ pub fn translate_insert(
if inserting_multiple_rows {
// For multiple rows, loop back
program.emit_insn(Insn::Goto {
target_pc: loop_start_offset,
target_pc: loop_start_label,
});
}

View File

@@ -173,3 +173,10 @@ do_execsql_test_on_specific_db {:memory:} named-insert-2 {
INSERT INTO test (col_b, col_d, col_c) VALUES ('1', '2', '4');
SELECT * FROM test;
} {1|Empty|1|4|2}
do_execsql_test_on_specific_db {:memory:} multi-rows {
CREATE TABLE test (id INTEGER PRIMARY KEY AUTOINCREMENT, col);
INSERT INTO test (col) VALUES (1),(1);
SELECT * FROM test;
} {1|1
2|1}