Skip to content

Commit dd48678

Browse files
committed
fix: Handle IVM old record capture when REQUIRE_SEEK flag is set
Panic: "to capture old record accurately, we must be located at the correct position in the table" Add SeekThenCapture state to OpInsertSubState to handle the case where both REQUIRE_SEEK is set AND there are dependent materialized views. Previously this combination caused a panic because the code assumed these conditions were mutually exclusive. The new state first seeks to the correct position, then captures the old record (if any) for incremental view maintenance before proceeding with the insert. This fixes INSERT OR REPLACE on tables with dependent materialized views.
1 parent 426426e commit dd48678

File tree

1 file changed

+65
-16
lines changed

1 file changed

+65
-16
lines changed

core/vdbe/execute.rs

Lines changed: 65 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6776,13 +6776,18 @@ pub struct OpInsertState {
67766776
#[derive(Debug, PartialEq)]
67776777
pub enum OpInsertSubState {
67786778
/// If this insert overwrites a record, capture the old record for incremental view maintenance.
6779+
/// If cursor is already positioned (no REQUIRE_SEEK), capture directly.
6780+
/// If REQUIRE_SEEK is set, transition to Seek first.
67796781
MaybeCaptureRecord,
67806782
/// Seek to the correct position if needed.
67816783
/// In a table insert, if the caller does not pass InsertFlags::REQUIRE_SEEK, they must ensure that a seek has already happened to the correct location.
67826784
/// This typically happens by invoking either Insn::NewRowid or Insn::NotExists, because:
67836785
/// 1. op_new_rowid() seeks to the end of the table, which is the correct insertion position.
67846786
/// 2. op_not_exists() seeks to the position in the table where the target rowid would be inserted.
67856787
Seek,
6788+
/// Capture the old record at the current cursor position for IVM.
6789+
/// The cursor must already be positioned (by a prior seek or by NotExists/NewRowid).
6790+
CaptureRecord,
67866791
/// Insert the row into the table.
67876792
Insert,
67886793
/// Updating last_insert_rowid may return IO, so we need a separate state for it so that we don't
@@ -6812,24 +6817,18 @@ pub fn op_insert(
68126817
loop {
68136818
match &state.op_insert_state.sub_state {
68146819
OpInsertSubState::MaybeCaptureRecord => {
6815-
let schema = program.connection.schema.read();
6816-
let dependent_views = schema.get_dependent_materialized_views(table_name);
6820+
let has_dependent_views = {
6821+
let schema = program.connection.schema.read();
6822+
!schema.get_dependent_materialized_views(table_name).is_empty()
6823+
};
68176824
// If there are no dependent views, we don't need to capture the old record.
6818-
// We also don't need to do it if the rowid of the UPDATEd row was changed, because that means
6819-
// we deleted it earlier and `op_delete` already captured the change.
6820-
if dependent_views.is_empty() || flag.has(InsertFlags::UPDATE_ROWID_CHANGE) {
6821-
if flag.has(InsertFlags::REQUIRE_SEEK) {
6822-
state.op_insert_state.sub_state = OpInsertSubState::Seek;
6823-
} else {
6824-
state.op_insert_state.sub_state = OpInsertSubState::Insert;
6825-
}
6826-
continue;
6827-
}
6825+
// We also don't need to do it if the rowid of the UPDATEd row was changed, because
6826+
// op_delete already captured the deletion for IVM, and this insert only needs to
6827+
// record the new row (which ApplyViewChange handles without old_record).
6828+
let needs_capture =
6829+
has_dependent_views && !flag.has(InsertFlags::UPDATE_ROWID_CHANGE);
68286830

6829-
turso_assert!(
6830-
!flag.has(InsertFlags::REQUIRE_SEEK),
6831-
"to capture old record accurately, we must be located at the correct position in the table"
6832-
);
6831+
turso_assert!(!flag.has(InsertFlags::REQUIRE_SEEK), "to capture old record accurately, we must be located at the correct position in the table");
68336832

68346833
// Get the key we're going to insert
68356834
let insert_key = match &state.registers[*key_reg].get_value() {
@@ -6860,6 +6859,7 @@ pub fn op_insert(
68606859
let mut values = record.get_values_owned()?;
68616860

68626861
// Fix rowid alias columns: replace Null with actual rowid value
6862+
let schema = program.connection.schema.read();
68636863
if let Some(table) = schema.get_table(table_name) {
68646864
for (i, col) in table.columns().iter().enumerate() {
68656865
if col.is_rowid_alias() && i < values.len() {
@@ -6884,6 +6884,8 @@ pub fn op_insert(
68846884
state.op_insert_state.old_record = old_record;
68856885
if flag.has(InsertFlags::REQUIRE_SEEK) {
68866886
state.op_insert_state.sub_state = OpInsertSubState::Seek;
6887+
} else if needs_capture {
6888+
state.op_insert_state.sub_state = OpInsertSubState::CaptureRecord;
68876889
} else {
68886890
state.op_insert_state.sub_state = OpInsertSubState::Insert;
68896891
}
@@ -6904,7 +6906,54 @@ pub fn op_insert(
69046906
)? {
69056907
return Ok(InsnFunctionStepResult::IO(io));
69066908
}
6909+
let has_dependent_views = {
6910+
let schema = program.connection.schema.read();
6911+
!schema.get_dependent_materialized_views(table_name).is_empty()
6912+
};
6913+
let needs_capture =
6914+
has_dependent_views && !flag.has(InsertFlags::UPDATE_ROWID_CHANGE);
6915+
if needs_capture {
6916+
state.op_insert_state.sub_state = OpInsertSubState::CaptureRecord;
6917+
} else {
6918+
state.op_insert_state.sub_state = OpInsertSubState::Insert;
6919+
}
6920+
continue;
6921+
}
6922+
OpInsertSubState::CaptureRecord => {
6923+
let insert_key = match &state.registers[*key_reg].get_value() {
6924+
Value::Numeric(Numeric::Integer(i)) => *i,
6925+
_ => unreachable!("expected integer key in insert"),
6926+
};
6927+
6928+
let cursor = state.get_cursor(*cursor_id);
6929+
let cursor = cursor.as_btree_mut();
6930+
let maybe_key = return_if_io!(cursor.rowid());
6931+
let old_record = if let Some(key) = maybe_key {
6932+
if key == insert_key {
6933+
let maybe_record = return_if_io!(cursor.record());
6934+
if let Some(record) = maybe_record {
6935+
let mut values = record.get_values_owned()?;
6936+
let schema = program.connection.schema.read();
6937+
if let Some(table) = schema.get_table(table_name) {
6938+
for (i, col) in table.columns().iter().enumerate() {
6939+
if col.is_rowid_alias() && i < values.len() {
6940+
values[i] = Value::from_i64(key);
6941+
}
6942+
}
6943+
}
6944+
Some((key, values))
6945+
} else {
6946+
None
6947+
}
6948+
} else {
6949+
None
6950+
}
6951+
} else {
6952+
None
6953+
};
6954+
state.op_insert_state.old_record = old_record;
69076955
state.op_insert_state.sub_state = OpInsertSubState::Insert;
6956+
continue;
69086957
}
69096958
OpInsertSubState::Insert => {
69106959
let key = match &state.registers[*key_reg].get_value() {

0 commit comments

Comments
 (0)