From 370da9fa590e73759d7ac5dc6464b59b92ec6cfa Mon Sep 17 00:00:00 2001 From: Alex Miller Date: Sun, 24 Aug 2025 13:28:42 -0700 Subject: [PATCH] ANALYZE creates sqlite_stat1 if it doesn't exist This change replaces a bail_parse_error!() when sqlite_stat1 doesn't exist with the appropriate codegen to create the table, and handle both cases of the table existing or not existing. SQLite's codegen looks like: sqlite> create table stat_test(a,b,c); sqlite> explain analyze stat_test; addr opcode p1 p2 p3 p4 p5 comment ---- ------------- ---- ---- ---- ------------- -- ------------- 0 Init 0 40 0 0 Start at 40 1 ReadCookie 0 3 2 0 2 If 3 5 0 0 3 SetCookie 0 2 4 0 4 SetCookie 0 5 1 0 5 CreateBtree 0 2 1 0 r[2]=root iDb=0 flags=1 6 OpenWrite 0 1 0 5 0 root=1 iDb=0 7 NewRowid 0 1 0 0 r[1]=rowid 8 Blob 6 3 0 0 r[3]= (len=6) 9 Insert 0 3 1 8 intkey=r[1] data=r[3] 10 Close 0 0 0 0 11 Close 0 0 0 0 12 Null 0 4 5 0 r[4..5]=NULL 13 Noop 4 0 4 0 14 OpenWrite 3 1 0 5 0 root=1 iDb=0; sqlite_master 15 SeekRowid 3 17 1 0 intkey=r[1] 16 Rowid 3 5 0 0 r[5]= rowid of 3 17 IsNull 5 26 0 0 if r[5]==NULL goto 26 18 String8 0 6 0 table 0 r[6]='table' 19 String8 0 7 0 sqlite_stat1 0 r[7]='sqlite_stat1' 20 String8 0 8 0 sqlite_stat1 0 r[8]='sqlite_stat1' 21 Copy 2 9 0 0 r[9]=r[2] 22 String8 0 10 0 CREATE TABLE sqlite_stat1(tbl,idx,stat) 0 r[10]='CREATE TABLE sqlite_stat1(tbl,idx,stat)' 23 MakeRecord 6 5 4 BBBDB 0 r[4]=mkrec(r[6..10]) 24 Delete 3 68 5 0 25 Insert 3 4 5 0 intkey=r[5] data=r[4] 26 SetCookie 0 1 2 0 27 ParseSchema 0 0 0 tbl_name='sqlite_stat1' AND type!='trigger' 0 28 OpenWrite 0 2 0 3 16 root=2 iDb=0; sqlite_stat1 29 OpenRead 5 2 0 3 0 root=2 iDb=0; stat_test 30 String8 0 18 0 stat_test 0 r[18]='stat_test'; stat_test 31 Count 5 20 0 0 r[20]=count() 32 IfNot 20 37 0 0 33 Null 0 19 0 0 r[19]=NULL 34 MakeRecord 18 3 16 BBB 0 r[16]=mkrec(r[18..20]) 35 NewRowid 0 12 0 0 r[12]=rowid 36 Insert 0 16 12 8 intkey=r[12] data=r[16] 37 LoadAnalysis 0 0 0 0 38 Expire 0 0 0 0 39 Halt 0 0 0 0 40 Transaction 0 1 1 0 1 usesStmtJournal=1 41 Goto 0 1 0 0 And now Turso's looks like: turso> create table stat_test(a,b,c); turso> explain analyze stat_test; addr opcode p1 p2 p3 p4 p5 comment ---- ----------------- ---- ---- ---- ------------- -- ------- 0 Init 0 23 0 0 Start at 23 1 Null 0 1 0 0 r[1]=NULL 2 CreateBtree 0 2 1 0 r[2]=root iDb=0 flags=1 3 OpenWrite 0 1 0 0 root=1; iDb=0 4 NewRowid 0 3 0 0 r[3]=rowid 5 String8 0 4 0 table 0 r[4]='table' 6 String8 0 5 0 sqlite_stat1 0 r[5]='sqlite_stat1' 7 String8 0 6 0 sqlite_stat1 0 r[6]='sqlite_stat1' 8 Copy 2 7 0 0 r[7]=r[2] 9 String8 0 8 0 CREATE TABLE sqlite_stat1(tbl,idx,stat) 0 r[8]='CREATE TABLE sqlite_stat1(tbl,idx,stat)' 10 MakeRecord 4 5 9 0 r[9]=mkrec(r[4..8]) 11 Insert 0 9 3 sqlite_stat1 0 intkey=r[3] data=r[9] 12 ParseSchema 0 0 0 tbl_name = 'sqlite_stat1' AND type != 'trigger' 0 tbl_name = 'sqlite_stat1' AND type != 'trigger' 13 OpenWrite 1 2 0 0 root=2; iDb=0 14 OpenRead 2 2 0 0 =stat_test, root=2, iDb=0 15 String8 0 12 0 stat_test 0 r[12]='stat_test' 16 Count 2 14 0 0 17 IfNot 14 22 0 0 if !r[14] goto 22 18 Null 0 13 0 0 r[13]=NULL 19 MakeRecord 12 3 11 0 r[11]=mkrec(r[12..14]) 20 NewRowid 1 10 0 0 r[10]=rowid 21 Insert 1 11 10 sqlite_stat1 0 intkey=r[10] data=r[11] 22 Halt 0 0 0 0 23 Goto 0 1 0 0 The notable difference in size is following the same codegen difference in CREATE TABLE, where sqlite's odd dance of adding a placeholder entry which is immediately replaced is instead done in tursodb as just inserting the correct row in the first place. Aside from lines 6-13 of sqlite's vdbe being missing, there's still the lack of LoadAnalysis, Expire, and Cookie management. --- core/translate/analyze.rs | 86 +++++++++++++++++++++++++++++++++++---- core/translate/mod.rs | 2 +- testing/analyze.test | 18 ++------ 3 files changed, 82 insertions(+), 24 deletions(-) diff --git a/core/translate/analyze.rs b/core/translate/analyze.rs index 4b72b1457..0d8f8de4e 100644 --- a/core/translate/analyze.rs +++ b/core/translate/analyze.rs @@ -1,19 +1,27 @@ +use std::sync::Arc; + use turso_parser::ast; use crate::{ bail_parse_error, - schema::Schema, + schema::{BTreeTable, Schema}, + storage::pager::CreateBTreeFlags, + translate::{ + emitter::Resolver, + schema::{emit_schema_entry, SchemaEntryType, SQLITE_TABLEID}, + }, util::normalize_ident, vdbe::{ builder::{CursorType, ProgramBuilder}, - insn::{Insn, RegisterOrLiteral::*}, + insn::{Insn, RegisterOrLiteral}, }, - Result, + Result, SymbolTable, }; pub fn translate_analyze( target_opt: Option, schema: &Schema, + syms: &SymbolTable, mut program: ProgramBuilder, ) -> Result { let Some(target) = target_opt else { @@ -34,7 +42,15 @@ pub fn translate_analyze( dest_end: None, }); + // After preparing/creating sqlite_stat1, we need to OpenWrite it, and how we acquire + // the necessary BTreeTable for cursor creation and root page for the instruction changes + // depending on which path we take. + let sqlite_stat1_btreetable: Arc; + let sqlite_stat1_source: RegisterOrLiteral<_>; + if let Some(sqlite_stat1) = schema.get_btree_table("sqlite_stat1") { + sqlite_stat1_btreetable = sqlite_stat1.clone(); + sqlite_stat1_source = RegisterOrLiteral::Literal(sqlite_stat1.root_page); // sqlite_stat1 already exists, so we need to remove the row // corresponding to the stats for the table which we're about to // ANALYZE. SQLite implements this as a full table scan over @@ -43,7 +59,7 @@ pub fn translate_analyze( let cursor_id = program.alloc_cursor_id(CursorType::BTreeTable(sqlite_stat1.clone())); program.emit_insn(Insn::OpenWrite { cursor_id, - root_page: Literal(sqlite_stat1.root_page), + root_page: RegisterOrLiteral::Literal(sqlite_stat1.root_page), db: 0, }); let after_loop = program.allocate_label(); @@ -89,7 +105,61 @@ pub fn translate_analyze( }); program.preassign_label_to_next_insn(after_loop); } else { - bail_parse_error!("ANALYZE without an existing sqlite_stat1 is not supported"); + // FIXME: Emit ReadCookie 0 3 2 + // FIXME: Emit If 3 +2 0 + // FIXME: Emit SetCookie 0 2 4 + // FIXME: Emit SetCookie 0 5 1 + + // See the large comment in schema.rs:translate_create_table about + // deviating from SQLite codegen, as the same deviation is being done + // here. + + // TODO: this code half-copies translate_create_table, because there's + // no way to get the table_root_reg back out, and it's needed for later + // codegen to open the table we just created. It's worth a future + // refactoring to remove the duplication one the rest of ANALYZE is + // implemented. + let table_root_reg = program.alloc_register(); + program.emit_insn(Insn::CreateBtree { + db: 0, + root: table_root_reg, + flags: CreateBTreeFlags::new_table(), + }); + let sql = "CREATE TABLE sqlite_stat1(tbl,idx,stat)"; + // The root_page==0 is false, but we don't rely on it, and there's no + // way to initialize it with a correct value. + sqlite_stat1_btreetable = Arc::new(BTreeTable::from_sql(sql, 0)?); + sqlite_stat1_source = RegisterOrLiteral::Register(table_root_reg); + + let table = schema.get_btree_table(SQLITE_TABLEID).unwrap(); + let sqlite_schema_cursor_id = + program.alloc_cursor_id(CursorType::BTreeTable(table.clone())); + program.emit_insn(Insn::OpenWrite { + cursor_id: sqlite_schema_cursor_id, + root_page: 1usize.into(), + db: 0, + }); + + let resolver = Resolver::new(schema, syms); + // Add the table entry to sqlite_schema + emit_schema_entry( + &mut program, + &resolver, + sqlite_schema_cursor_id, + None, + SchemaEntryType::Table, + "sqlite_stat1", + "sqlite_stat1", + table_root_reg, + Some(sql.to_string()), + )?; + //FIXME: Emit SetCookie? + let parse_schema_where_clause = + "tbl_name = 'sqlite_stat1' AND type != 'trigger'".to_string(); + program.emit_insn(Insn::ParseSchema { + db: sqlite_schema_cursor_id, + where_clause: Some(parse_schema_where_clause), + }); }; if target_schema.columns().iter().any(|c| c.primary_key) { @@ -100,13 +170,11 @@ pub fn translate_analyze( } // Count the number of rows in the target table, and insert it into sqlite_stat1. - let sqlite_stat1 = schema - .get_btree_table("sqlite_stat1") - .expect("sqlite_stat1 either pre-existed or was just created"); + let sqlite_stat1 = sqlite_stat1_btreetable; let stat_cursor = program.alloc_cursor_id(CursorType::BTreeTable(sqlite_stat1.clone())); program.emit_insn(Insn::OpenWrite { cursor_id: stat_cursor, - root_page: Literal(sqlite_stat1.root_page), + root_page: sqlite_stat1_source, db: 0, }); let target_cursor = program.alloc_cursor_id(CursorType::BTreeTable(target_btree.clone())); diff --git a/core/translate/mod.rs b/core/translate/mod.rs index 86cd60f52..7d29a8173 100644 --- a/core/translate/mod.rs +++ b/core/translate/mod.rs @@ -148,7 +148,7 @@ pub fn translate_inner( ast::Stmt::AlterTable(alter) => { translate_alter_table(alter, syms, schema, program, connection, input)? } - ast::Stmt::Analyze { name } => translate_analyze(name, schema, program)?, + ast::Stmt::Analyze { name } => translate_analyze(name, schema, syms, program)?, ast::Stmt::Attach { expr, db_name, key } => { attach::translate_attach(&expr, &db_name, &key, schema, syms, program)? } diff --git a/testing/analyze.test b/testing/analyze.test index a1761bf45..7d7f95066 100755 --- a/testing/analyze.test +++ b/testing/analyze.test @@ -5,28 +5,26 @@ source $testdir/tester.tcl # Things that do work: do_execsql_test_on_specific_db {:memory:} empty-table { - CREATE TABLE sqlite_stat1(tbl,idx,stat); CREATE TABLE temp (a integer); ANALYZE temp; SELECT * FROM sqlite_stat1; } {} do_execsql_test_on_specific_db {:memory:} one-row-table { - CREATE TABLE sqlite_stat1(tbl,idx,stat); CREATE TABLE temp (a integer); INSERT INTO temp VALUES (1); ANALYZE temp; SELECT * FROM sqlite_stat1; } {temp||1} -do_execsql_test_on_specific_db {:memory:} analyze-deletes { - CREATE TABLE sqlite_stat1(tbl,idx,stat); - INSERT INTO sqlite_stat1 VALUES ('temp', NULL, 10); +do_execsql_test_on_specific_db {:memory:} analyze-overwrites { CREATE TABLE temp (a integer); INSERT INTO temp VALUES (1); ANALYZE temp; + INSERT INTO temp VALUES (2); + ANALYZE temp; SELECT * FROM sqlite_stat1; -} {temp||1} +} {temp||2} # Things that don't work: @@ -38,25 +36,17 @@ do_execsql_test_in_memory_error analyze-one-database-fails { ANALYZE main; } {.*ANALYZE.*not supported.*} -do_execsql_test_in_memory_error analyze-without-stat-table-fails { - CREATE TABLE temp (a integer); - ANALYZE temp; -} {.*ANALYZE.*not supported.*} - do_execsql_test_in_memory_error analyze-table-with-pk-fails { - CREATE TABLE sqlite_stat1(tbl,idx,stat); CREATE TABLE temp (a integer primary key); ANALYZE temp; } {.*ANALYZE.*not supported.*} do_execsql_test_in_memory_error analyze-table-without-rowid-fails { - CREATE TABLE sqlite_stat1(tbl,idx,stat); CREATE TABLE temp (a integer primary key) WITHOUT ROWID; ANALYZE temp; } {.*ANALYZE.*not supported.*} do_execsql_test_in_memory_error analyze-index-fails { - CREATE TABLE sqlite_stat1(tbl,idx,stat); CREATE TABLE temp (a integer, b integer); CREATE INDEX temp_b ON temp (b); ANALYZE temp_b;