Skip to content

Commit d249968

Browse files
authored
Merge 'remove Arc<RwLock<Statement>> from Insn::Program' from Pedro Muniz
`Program` should be an immutable set of instructions, and the `Arc<RwLock<Statement>>` in `Insn::Program` violates this behaviour breaking cached prepared statements. I was exploring how we could properly isolate the translation code into its own separate thing (if not in a crate at least logically), and I saw we were creating statements and needing a pager for creating sub programs. Some of refactoring Claude helped, especially for `explain_step` Closes #5501
2 parents b3009e0 + aa3be97 commit d249968

File tree

7 files changed

+100
-145
lines changed

7 files changed

+100
-145
lines changed

core/translate/fkeys.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
use crate::sync::RwLock;
21
use rustc_hash::FxHashSet as HashSet;
32
use turso_parser::ast::{self, Expr, Literal, Name, QualifiedName, RefAct};
43

@@ -12,7 +11,7 @@ use crate::{
1211
insn::{CmpInsFlags, Insn},
1312
BranchOffset,
1413
},
15-
Connection, LimboError, Result, Statement, Value,
14+
Connection, LimboError, Result, Value,
1615
};
1716
use std::{num::NonZero, num::NonZeroUsize, sync::Arc};
1817

@@ -1302,14 +1301,9 @@ fn emit_fk_action_subprogram(
13021301
);
13031302
}
13041303

1305-
let turso_stmt = Statement::new(
1306-
built_subprogram,
1307-
connection.pager.load().clone(),
1308-
QueryMode::Normal,
1309-
);
13101304
program.emit_insn(Insn::Program {
13111305
params,
1312-
program: Arc::new(RwLock::new(turso_stmt)),
1306+
program: built_subprogram.prepared().clone(),
13131307
});
13141308

13151309
Ok(())

core/translate/trigger_exec.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
use crate::schema::{BTreeTable, Trigger};
22
use crate::sync::Arc;
3-
use crate::sync::RwLock;
43
use crate::translate::emitter::Resolver;
54
use crate::translate::expr::translate_expr;
65
use crate::translate::{translate_inner, ProgramBuilder, ProgramBuilderOpts};
76
use crate::util::normalize_ident;
87
use crate::vdbe::insn::Insn;
9-
use crate::{bail_parse_error, QueryMode, Result, Statement};
8+
use crate::{bail_parse_error, QueryMode, Result};
109
use rustc_hash::FxHashSet as HashSet;
1110
use std::num::NonZero;
1211
use turso_parser::ast::{self, Expr, TriggerEvent, TriggerTime};
@@ -649,14 +648,9 @@ fn execute_trigger_commands(
649648
);
650649
}
651650

652-
let turso_stmt = Statement::new(
653-
built_subprogram,
654-
connection.pager.load().clone(),
655-
QueryMode::Normal,
656-
);
657651
program.emit_insn(Insn::Program {
658652
params,
659-
program: Arc::new(RwLock::new(turso_stmt)),
653+
program: built_subprogram.prepared().clone(),
660654
});
661655
connection.end_trigger_compilation();
662656

core/vdbe/builder.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1495,12 +1495,12 @@ impl ProgramBuilder {
14951495
});
14961496
}
14971497

1498-
pub fn build(
1498+
pub fn build_prepared_program(
14991499
mut self,
1500-
connection: Arc<Connection>,
1500+
prepare_context: PrepareContext,
15011501
change_cnt_on: bool,
15021502
sql: &str,
1503-
) -> crate::Result<Program> {
1503+
) -> crate::Result<PreparedProgram> {
15041504
self.resolve_labels()?;
15051505

15061506
self.parameters.list.dedup();
@@ -1533,9 +1533,20 @@ impl ProgramBuilder {
15331533
is_subprogram: self.is_subprogram,
15341534
contains_trigger_subprograms,
15351535
resolve_type: self.resolve_type,
1536-
prepare_context: PrepareContext::from_connection(&connection),
1536+
prepare_context,
15371537
write_databases: self.write_databases,
15381538
};
1539+
Ok(prepared)
1540+
}
1541+
1542+
pub fn build(
1543+
self,
1544+
connection: Arc<Connection>,
1545+
change_cnt_on: bool,
1546+
sql: &str,
1547+
) -> crate::Result<Program> {
1548+
let prepare_context = PrepareContext::from_connection(&connection);
1549+
let prepared = self.build_prepared_program(prepare_context, change_cnt_on, sql)?;
15391550
Ok(Program::from_prepared(Arc::new(prepared), connection))
15401551
}
15411552
}

core/vdbe/execute.rs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ use crate::vector::{
4040
vector1bit, vector32, vector32_sparse, vector64, vector8, vector_concat, vector_distance_cos,
4141
vector_distance_dot, vector_distance_jaccard, vector_distance_l2, vector_extract, vector_slice,
4242
};
43-
use crate::CdcVersion;
4443
use crate::{
4544
error::{
4645
LimboError, SQLITE_CONSTRAINT, SQLITE_CONSTRAINT_CHECK, SQLITE_CONSTRAINT_NOTNULL,
@@ -59,8 +58,9 @@ use crate::{
5958
};
6059
use crate::{
6160
get_cursor, CaptureDataChangesInfo, CheckpointMode, Completion, Connection, DatabaseStorage,
62-
IOExt, MvCursor,
61+
IOExt, MvCursor, QueryMode,
6362
};
63+
use crate::{CdcVersion, Statement};
6464
use either::Either;
6565
use smallvec::SmallVec;
6666
use std::any::Any;
@@ -2810,6 +2810,7 @@ pub enum OpProgramState {
28102810
/// Step state tracks whether we're executing a trigger subprogram (vs FK action subprogram)
28112811
Step {
28122812
is_trigger: bool,
2813+
statement: Box<Statement>,
28132814
},
28142815
}
28152816

@@ -2819,7 +2820,7 @@ pub fn op_program(
28192820
program: &Program,
28202821
state: &mut ProgramState,
28212822
insn: &Insn,
2822-
_pager: &Arc<Pager>,
2823+
pager: &Arc<Pager>,
28232824
) -> Result<InsnFunctionStepResult> {
28242825
load_insn!(
28252826
Program {
@@ -2831,7 +2832,11 @@ pub fn op_program(
28312832
loop {
28322833
match &mut state.op_program_state {
28332834
OpProgramState::Start => {
2834-
let mut statement = subprogram.write();
2835+
let mut statement = Statement::new(
2836+
Program::from_prepared(subprogram.clone(), program.connection.clone()),
2837+
pager.clone(),
2838+
QueryMode::Normal,
2839+
);
28352840
statement.reset();
28362841

28372842
// Check if this is a trigger subprogram - if so, track execution
@@ -2867,12 +2872,17 @@ pub fn op_program(
28672872
}
28682873
}
28692874

2870-
state.op_program_state = OpProgramState::Step { is_trigger };
2875+
state.op_program_state = OpProgramState::Step {
2876+
is_trigger,
2877+
statement: Box::new(statement),
2878+
};
28712879
}
2872-
OpProgramState::Step { is_trigger } => {
2880+
OpProgramState::Step {
2881+
is_trigger,
2882+
statement,
2883+
} => {
28732884
let is_trigger = *is_trigger;
28742885
loop {
2875-
let mut statement = subprogram.write();
28762886
let res = statement.step();
28772887
match res {
28782888
Ok(step_result) => match step_result {

core/vdbe/explain.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use turso_parser::ast::SortOrder;
22

33
use crate::vdbe::{builder::CursorType, insn::RegisterOrLiteral};
44

5-
use super::{Insn, InsnReference, Program, Value};
5+
use super::{Insn, InsnReference, PreparedProgram, Value};
66
use crate::function::{Func, ScalarFunc};
77

88
pub const EXPLAIN_COLUMNS: [&str; 8] = ["addr", "opcode", "p1", "p2", "p3", "p4", "p5", "comment"];
@@ -13,7 +13,7 @@ pub const EXPLAIN_QUERY_PLAN_COLUMNS: [&str; 4] = ["id", "parent", "notused", "d
1313
pub const EXPLAIN_QUERY_PLAN_COLUMNS_TYPE: [&str; 4] = ["INTEGER", "INTEGER", "INTEGER", "TEXT"];
1414

1515
pub fn insn_to_row(
16-
program: &Program,
16+
program: &PreparedProgram,
1717
insn: &Insn,
1818
) -> (&'static str, i64, i64, i64, Value, i64, String) {
1919
let get_table_or_index_name = |cursor_id: usize| {
@@ -2084,7 +2084,7 @@ pub fn insn_to_row(
20842084
}
20852085

20862086
pub fn insn_to_row_with_comment(
2087-
program: &Program,
2087+
program: &PreparedProgram,
20882088
insn: &Insn,
20892089
manual_comment: Option<&str>,
20902090
) -> (&'static str, i64, i64, i64, Value, i64, String) {
@@ -2101,7 +2101,7 @@ pub fn insn_to_row_with_comment(
21012101
}
21022102

21032103
pub fn insn_to_str(
2104-
program: &Program,
2104+
program: &PreparedProgram,
21052105
addr: InsnReference,
21062106
insn: &Insn,
21072107
indent: String,

core/vdbe/insn.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,13 @@ pub fn to_u16(v: usize) -> u16 {
1111
}
1212

1313
use super::{execute, AggFunc, BranchOffset, CursorID, FuncCtx, InsnFunction, PageIdx};
14-
use crate::sync::RwLock;
1514
use crate::{
1615
schema::{BTreeTable, CheckConstraint, Column, Index},
1716
storage::{pager::CreateBTreeFlags, wal::CheckpointMode},
1817
translate::{collate::CollationSeq, emitter::TransactionMode},
1918
types::KeyInfo,
2019
vdbe::affinity::Affinity,
21-
Statement, Value,
20+
PreparedProgram, Value,
2221
};
2322
use strum::EnumCount;
2423
use strum_macros::{EnumDiscriminants, FromRepr, VariantArray};
@@ -615,7 +614,7 @@ pub enum Insn {
615614
/// is used by subprograms to access content in registers of the calling bytecode program."
616615
Program {
617616
params: Vec<Value>,
618-
program: Arc<RwLock<Statement>>,
617+
program: Arc<PreparedProgram>,
619618
},
620619

621620
/// Write an integer value into a register.

0 commit comments

Comments
 (0)