From d64733c0b922c177aef086c31e963cd92dcdf9f6 Mon Sep 17 00:00:00 2001 From: Bennett Clement Date: Sun, 14 Jul 2024 11:30:27 +0800 Subject: [PATCH] Improve explain comments - Resolve cursor ID to table name and get column name from index - Since we change the type of BranchOffset to i64, add assertions in Program.step() function - opcode generation compatibility with sqlite: change register number to start from 1 - Improve Column,Rowid comment, Add DecrJumpZero comment, Fix Integer comment - Fix typos in code comments --- core/schema.rs | 20 ++++++++ core/translate.rs | 31 ++++--------- core/vdbe.rs | 116 ++++++++++++++++++++++++++++++++-------------- 3 files changed, 110 insertions(+), 57 deletions(-) diff --git a/core/schema.rs b/core/schema.rs index 67705c615..0c8e68264 100644 --- a/core/schema.rs +++ b/core/schema.rs @@ -51,6 +51,26 @@ impl Table { } } + pub fn get_name(&self) -> &str { + match self { + Table::BTree(table) => &table.name, + Table::Pseudo(table) => &table.columns[0].name, + } + } + + pub fn column_index_to_name(&self, index: usize) -> Option<&str> { + match self { + Table::BTree(table) => match table.columns.get(index) { + Some(column) => Some(&column.name), + None => None, + }, + Table::Pseudo(table) => match table.columns.get(index) { + Some(column) => Some(&column.name), + None => None, + }, + } + } + pub fn get_column(&self, name: &str) -> Option<(usize, &Column)> { match self { Table::BTree(table) => table.get_column(name), diff --git a/core/translate.rs b/core/translate.rs index f4c69efb9..23905e7dd 100644 --- a/core/translate.rs +++ b/core/translate.rs @@ -1,4 +1,5 @@ use std::cell::RefCell; +use std::collections::{HashMap, HashSet}; use std::rc::Rc; use crate::function::{AggFunc, Func, SingleRowFunc}; @@ -30,7 +31,6 @@ struct Select { } struct LoopInfo { - table: Table, rewind_offset: BranchOffset, rewind_label: BranchOffset, open_cursor: usize, @@ -339,7 +339,7 @@ fn translate_tables_end(program: &mut ProgramBuilder, select: &Select) { } fn translate_table_open_cursor(program: &mut ProgramBuilder, table: &Table) -> LoopInfo { - let cursor_id = program.alloc_cursor_id(); + let cursor_id = program.alloc_cursor_id(table.clone()); let root_page = match table { Table::BTree(btree) => btree.root_page, Table::Pseudo(_) => todo!(), @@ -350,7 +350,6 @@ fn translate_table_open_cursor(program: &mut ProgramBuilder, table: &Table) -> L }); program.emit_insn(Insn::OpenReadAwait); LoopInfo { - table: table.clone(), open_cursor: cursor_id, rewind_offset: 0, rewind_label: 0, @@ -412,7 +411,7 @@ fn translate_column( let mut target_register = target_register; for join in &select.src_tables { let table = &join.table; - translate_table_star(table, program, &select, target_register); + translate_table_star(table, program, target_register); target_register += table.columns().len(); } } @@ -421,18 +420,8 @@ fn translate_column( Ok(()) } -fn translate_table_star( - table: &Table, - program: &mut ProgramBuilder, - select: &Select, - target_register: usize, -) { - let table_cursor = select - .loops - .iter() - .find(|v| v.table == *table) - .unwrap() - .open_cursor; +fn translate_table_star(table: &Table, program: &mut ProgramBuilder, target_register: usize) { + let table_cursor = program.resolve_cursor_id(table); for (i, col) in table.columns().iter().enumerate() { let col_target_register = target_register + i; if table.column_is_rowid_alias(col) { @@ -630,7 +619,7 @@ fn translate_expr( ast::Expr::FunctionCallStar { .. } => todo!(), ast::Expr::Id(ident) => { // let (idx, col) = table.unwrap().get_column(&ident.0).unwrap(); - let (idx, col, cursor_id) = resolve_ident_table(&ident.0, select)?; + let (idx, col, cursor_id) = resolve_ident_table(program, &ident.0, select)?; if col.primary_key { program.emit_insn(Insn::RowId { cursor_id, @@ -699,6 +688,7 @@ fn translate_expr( } fn resolve_ident_table<'a>( + program: &ProgramBuilder, ident: &String, select: &'a Select, ) -> Result<(usize, &'a Column, usize)> { @@ -711,12 +701,7 @@ fn resolve_ident_table<'a>( .find(|(_, col)| col.name == *ident); if res.is_some() { let (idx, col) = res.unwrap(); - let cursor_id = select - .loops - .iter() - .find(|l| l.table == join.table) - .unwrap() - .open_cursor; + let cursor_id = program.resolve_cursor_id(&join.table); return Ok((idx, col, cursor_id)); } } diff --git a/core/vdbe.rs b/core/vdbe.rs index 7f65c3d41..99f9c3ffa 100644 --- a/core/vdbe.rs +++ b/core/vdbe.rs @@ -1,6 +1,7 @@ use crate::btree::BTreeCursor; use crate::function::AggFunc; use crate::pager::Pager; +use crate::schema::Table; use crate::types::{AggContext, Cursor, CursorResult, OwnedRecord, OwnedValue, Record}; use anyhow::Result; @@ -224,21 +225,24 @@ pub struct ProgramBuilder { next_free_label: BranchOffset, next_free_cursor_id: usize, insns: Vec, - // Each lable has a list of InsnRefereces that must + // Each label has a list of InsnReferences that must // be resolved. Lists are indexed by: label.abs() - 1 unresolved_labels: Vec>, next_insn_label: Option, + // Cursors that are referenced by the program. Indexed by CursorID. + cursor_ref: Vec, } impl ProgramBuilder { pub fn new() -> Self { Self { - next_free_register: 0, + next_free_register: 1, next_free_label: 0, next_free_cursor_id: 0, insns: Vec::new(), unresolved_labels: Vec::new(), next_insn_label: None, + cursor_ref: Vec::new(), } } @@ -258,9 +262,14 @@ impl ProgramBuilder { self.next_free_register } - pub fn alloc_cursor_id(&mut self) -> usize { + pub fn alloc_cursor_id(&mut self, table: Table) -> usize { let cursor = self.next_free_cursor_id; self.next_free_cursor_id += 1; + if self.cursor_ref.iter().find(|t| **t == table).is_some() { + todo!("duplicate table is unhandled. see how resolve_ident_table() calls resolve_cursor_id") + } + self.cursor_ref.push(table); + assert!(self.cursor_ref.len() == self.next_free_cursor_id); cursor } @@ -415,10 +424,16 @@ impl ProgramBuilder { label_references.clear(); } + // translate table to cursor id + pub fn resolve_cursor_id(&self, table: &Table) -> CursorID { + self.cursor_ref.iter().position(|t| t == table).unwrap() + } + pub fn build(self) -> Program { Program { max_registers: self.next_free_register, insns: self.insns, + cursor_ref: self.cursor_ref, } } } @@ -460,6 +475,7 @@ impl ProgramState { pub struct Program { pub max_registers: usize, pub insns: Vec, + pub cursor_ref: Vec
, } impl Program { @@ -471,7 +487,12 @@ impl Program { let mut prev_insn: Option<&Insn> = None; for (addr, insn) in self.insns.iter().enumerate() { indent_count = get_indent_count(indent_count, insn, prev_insn); - print_insn(addr as BranchOffset, insn, indent.repeat(indent_count)); + print_insn( + &self, + addr as InsnReference, + insn, + indent.repeat(indent_count), + ); prev_insn = Some(insn); } } @@ -483,10 +504,11 @@ impl Program { ) -> Result> { loop { let insn = &self.insns[state.pc as usize]; - trace_insn(state.pc, insn); + trace_insn(self, state.pc as InsnReference, insn); let mut cursors = state.cursors.borrow_mut(); match insn { Insn::Init { target_pc } => { + assert!(*target_pc >= 0); state.pc = *target_pc; } Insn::Null { dest } => { @@ -494,6 +516,7 @@ impl Program { state.pc += 1; } Insn::NotNull { reg, target_pc } => { + assert!(*target_pc >= 0); let reg = *reg; let target_pc = *target_pc; match &state.registers[reg] { @@ -510,6 +533,7 @@ impl Program { rhs, target_pc, } => { + assert!(*target_pc >= 0); let lhs = *lhs; let rhs = *rhs; let target_pc = *target_pc; @@ -545,6 +569,7 @@ impl Program { rhs, target_pc, } => { + assert!(*target_pc >= 0); let lhs = *lhs; let rhs = *rhs; let target_pc = *target_pc; @@ -580,6 +605,7 @@ impl Program { rhs, target_pc, } => { + assert!(*target_pc >= 0); let lhs = *lhs; let rhs = *rhs; let target_pc = *target_pc; @@ -608,6 +634,7 @@ impl Program { rhs, target_pc, } => { + assert!(*target_pc >= 0); let lhs = *lhs; let rhs = *rhs; let target_pc = *target_pc; @@ -636,6 +663,7 @@ impl Program { rhs, target_pc, } => { + assert!(*target_pc >= 0); let lhs = *lhs; let rhs = *rhs; let target_pc = *target_pc; @@ -664,6 +692,7 @@ impl Program { rhs, target_pc, } => { + assert!(*target_pc >= 0); let lhs = *lhs; let rhs = *rhs; let target_pc = *target_pc; @@ -688,6 +717,7 @@ impl Program { } } Insn::IfNot { reg, target_pc } => { + assert!(*target_pc >= 0); let reg = *reg; let target_pc = *target_pc; match &state.registers[reg] { @@ -785,6 +815,7 @@ impl Program { cursor_id, pc_if_next, } => { + assert!(*pc_if_next >= 0); let cursor = cursors.get_mut(cursor_id).unwrap(); cursor.wait_for_completion()?; if !cursor.is_empty() { @@ -800,6 +831,7 @@ impl Program { state.pc += 1; } Insn::Goto { target_pc } => { + assert!(*target_pc >= 0); state.pc = *target_pc; } Insn::Integer { value, dest } => { @@ -830,18 +862,21 @@ impl Program { } state.pc += 1; } - Insn::DecrJumpZero { reg, target_pc } => match state.registers[*reg] { - OwnedValue::Integer(n) => { - let n = n - 1; - if n == 0 { - state.pc = *target_pc; - } else { - state.registers[*reg] = OwnedValue::Integer(n); - state.pc += 1; + Insn::DecrJumpZero { reg, target_pc } => { + assert!(*target_pc >= 0); + match state.registers[*reg] { + OwnedValue::Integer(n) => { + let n = n - 1; + if n == 0 { + state.pc = *target_pc; + } else { + state.registers[*reg] = OwnedValue::Integer(n); + state.pc += 1; + } } + _ => unreachable!("DecrJumpZero on non-integer register"), } - _ => unreachable!("DecrJumpZero on non-integer register"), - }, + } Insn::AggStep { acc_reg, col, @@ -1090,6 +1125,7 @@ impl Program { cursor_id, pc_if_next, } => { + assert!(*pc_if_next >= 0); let cursor = cursors.get_mut(cursor_id).unwrap(); match cursor.next()? { CursorResult::Ok(_) => {} @@ -1125,19 +1161,19 @@ fn make_owned_record(registers: &[OwnedValue], start_reg: &usize, count: &usize) OwnedRecord::new(values) } -fn trace_insn(addr: BranchOffset, insn: &Insn) { +fn trace_insn(program: &Program, addr: InsnReference, insn: &Insn) { if !log::log_enabled!(log::Level::Trace) { return; } - log::trace!("{}", insn_to_str(addr, insn, String::new())); + log::trace!("{}", insn_to_str(program, addr, insn, String::new())); } -fn print_insn(addr: BranchOffset, insn: &Insn, indent: String) { - let s = insn_to_str(addr, insn, indent); +fn print_insn(program: &Program, addr: InsnReference, insn: &Insn, indent: String) { + let s = insn_to_str(program, addr, insn, indent); println!("{}", s); } -fn insn_to_str(addr: BranchOffset, insn: &Insn, indent: String) -> String { +fn insn_to_str(program: &Program, addr: InsnReference, insn: &Insn, indent: String) -> String { let (opcode, p1, p2, p3, p4, p5, comment): (&str, i32, i32, i32, OwnedValue, u16, String) = match insn { Insn::Init { target_pc } => ( @@ -1313,15 +1349,23 @@ fn insn_to_str(addr: BranchOffset, insn: &Insn, indent: String) -> String { cursor_id, column, dest, - } => ( - "Column", - *cursor_id as i32, - *column as i32, - *dest as i32, - OwnedValue::Text(Rc::new("".to_string())), - 0, - format!("r[{}]= cursor {} column {}", dest, cursor_id, column), - ), + } => { + let table = &program.cursor_ref[*cursor_id]; + ( + "Column", + *cursor_id as i32, + *column as i32, + *dest as i32, + OwnedValue::Text(Rc::new("".to_string())), + 0, + format!( + "r[{}]={}.{}", + dest, + table.get_name(), + table.column_index_to_name(*column).unwrap() + ), + ) + } Insn::MakeRecord { start_reg, count, @@ -1342,7 +1386,7 @@ fn insn_to_str(addr: BranchOffset, insn: &Insn, indent: String) -> String { 0, OwnedValue::Text(Rc::new("".to_string())), 0, - format!("output=r[{}..{}]", start_reg, start_reg + count), + format!("output=r[{}..{}]", start_reg, start_reg + count - 1), ), Insn::NextAsync { cursor_id } => ( "NextAsync", @@ -1394,12 +1438,12 @@ fn insn_to_str(addr: BranchOffset, insn: &Insn, indent: String) -> String { ), Insn::Integer { value, dest } => ( "Integer", - *dest as i32, *value as i32, + *dest as i32, 0, OwnedValue::Text(Rc::new("".to_string())), 0, - "".to_string(), + format!("r[{}]={}", dest, value), ), Insn::Real { value, dest } => ( "Real", @@ -1435,7 +1479,11 @@ fn insn_to_str(addr: BranchOffset, insn: &Insn, indent: String) -> String { 0, OwnedValue::Text(Rc::new("".to_string())), 0, - "".to_string(), + format!( + "r[{}]={}.rowid", + dest, + &program.cursor_ref[*cursor_id].get_name() + ), ), Insn::DecrJumpZero { reg, target_pc } => ( "DecrJumpZero", @@ -1444,7 +1492,7 @@ fn insn_to_str(addr: BranchOffset, insn: &Insn, indent: String) -> String { 0, OwnedValue::Text(Rc::new("".to_string())), 0, - "".to_string(), + format!("if (--r[{}]==0) goto {}", reg, target_pc), ), Insn::AggStep { func,