From afc55b27f095d4d62182169917053c5374e3c879 Mon Sep 17 00:00:00 2001 From: "Levy A." Date: Mon, 30 Jun 2025 13:29:22 -0300 Subject: [PATCH 1/4] refactor: remove unnecessary column definitions for PseudoTable the only information that matters in the amount of column --- core/schema.rs | 35 ++++++------------------------- core/translate/group_by.rs | 19 +---------------- core/translate/order_by.rs | 42 +++----------------------------------- core/vdbe/explain.rs | 5 +---- 4 files changed, 11 insertions(+), 90 deletions(-) diff --git a/core/schema.rs b/core/schema.rs index 1a5f21fa3..df6f8ea4f 100644 --- a/core/schema.rs +++ b/core/schema.rs @@ -163,7 +163,7 @@ impl Table { pub fn get_column_at(&self, index: usize) -> Option<&Column> { match self { Self::BTree(table) => table.columns.get(index), - Self::Pseudo(table) => table.columns.get(index), + Self::Pseudo(table) => unimplemented!(), Self::Virtual(table) => table.columns.get(index), Self::FromClauseSubquery(from_clause_subquery) => { from_clause_subquery.columns.get(index) @@ -174,7 +174,7 @@ impl Table { pub fn columns(&self) -> &Vec { match self { Self::BTree(table) => &table.columns, - Self::Pseudo(table) => &table.columns, + Self::Pseudo(table) => unimplemented!(), Self::Virtual(table) => &table.columns, Self::FromClauseSubquery(from_clause_subquery) => &from_clause_subquery.columns, } @@ -293,39 +293,16 @@ impl BTreeTable { #[derive(Debug, Default)] pub struct PseudoTable { - pub columns: Vec, + pub column_count: usize, } impl PseudoTable { pub fn new() -> Self { - Self { columns: vec![] } + Self { column_count: 0 } } - pub fn new_with_columns(columns: Vec) -> Self { - Self { columns } - } - - pub fn add_column(&mut self, name: &str, ty: Type, primary_key: bool) { - self.columns.push(Column { - name: Some(normalize_ident(name)), - ty, - ty_str: ty.to_string().to_uppercase(), - primary_key, - is_rowid_alias: false, - notnull: false, - default: None, - unique: false, - collation: None, - }); - } - pub fn get_column(&self, name: &str) -> Option<(usize, &Column)> { - let name = normalize_ident(name); - for (i, column) in self.columns.iter().enumerate() { - if column.name.as_ref().map_or(false, |n| *n == name) { - return Some((i, column)); - } - } - None + pub fn new_with_columns(columns: impl AsRef<[Column]>) -> Self { + Self { column_count: columns.as_ref().len() } } } diff --git a/core/translate/group_by.rs b/core/translate/group_by.rs index 4411e7454..511ce5fee 100644 --- a/core/translate/group_by.rs +++ b/core/translate/group_by.rs @@ -304,27 +304,10 @@ pub fn group_by_create_pseudo_table( program: &mut ProgramBuilder, sorter_column_count: usize, ) -> usize { - // Create pseudo-columns for the pseudo-table - // (these are placeholders as we only care about structure, not semantics) - let ty = crate::schema::Type::Null; - let pseudo_columns = (0..sorter_column_count) - .map(|_| Column { - name: None, - primary_key: false, - ty, - ty_str: ty.to_string().to_uppercase(), - is_rowid_alias: false, - notnull: false, - default: None, - unique: false, - collation: None, - }) - .collect::>(); - // Create a pseudo-table to read one row at a time from the sorter // This allows us to use standard table access operations on the sorted data let pseudo_table = Rc::new(PseudoTable { - columns: pseudo_columns, + column_count: sorter_column_count, }); program.alloc_cursor_id(CursorType::Pseudo(pseudo_table.clone())) diff --git a/core/translate/order_by.rs b/core/translate/order_by.rs index c844d1a71..70df8c772 100644 --- a/core/translate/order_by.rs +++ b/core/translate/order_by.rs @@ -87,44 +87,8 @@ pub fn emit_order_by( let sort_loop_start_label = program.allocate_label(); let sort_loop_next_label = program.allocate_label(); let sort_loop_end_label = program.allocate_label(); - let mut pseudo_columns = vec![]; - for _ in 0..order_by.len() { - let ty = crate::schema::Type::Null; - pseudo_columns.push(Column { - // Names don't matter. We are tracking which result column is in which position in the ORDER BY clause in m.result_column_indexes_in_orderby_sorter. - name: None, - primary_key: false, - ty, - ty_str: ty.to_string().to_uppercase(), - is_rowid_alias: false, - notnull: false, - default: None, - unique: false, - collation: None, - }); - } - for i in 0..result_columns.len() { - // If any result columns are not in the ORDER BY sorter, it's because they are equal to a sort key and were already added to the pseudo columns above. - if let Some(ref v) = t_ctx.result_columns_to_skip_in_orderby_sorter { - if v.contains(&i) { - continue; - } - } - let ty = crate::schema::Type::Null; - pseudo_columns.push(Column { - name: None, - primary_key: false, - ty, - ty_str: ty.to_string().to_uppercase(), - is_rowid_alias: false, - notnull: false, - default: None, - unique: false, - collation: None, - }); - } - let num_columns_in_sorter = order_by.len() + result_columns.len() + let sorter_column_count = order_by.len() + result_columns.len() - t_ctx .result_columns_to_skip_in_orderby_sorter .as_ref() @@ -132,7 +96,7 @@ pub fn emit_order_by( .unwrap_or(0); let pseudo_table = Rc::new(PseudoTable { - columns: pseudo_columns, + column_count: sorter_column_count, }); let pseudo_cursor = program.alloc_cursor_id(CursorType::Pseudo(pseudo_table.clone())); let SortMetadata { @@ -143,7 +107,7 @@ pub fn emit_order_by( program.emit_insn(Insn::OpenPseudo { cursor_id: pseudo_cursor, content_reg: reg_sorter_data, - num_fields: num_columns_in_sorter, + num_fields: sorter_column_count, }); program.emit_insn(Insn::SorterSort { diff --git a/core/vdbe/explain.rs b/core/vdbe/explain.rs index cedc08387..8fe97dc89 100644 --- a/core/vdbe/explain.rs +++ b/core/vdbe/explain.rs @@ -539,10 +539,7 @@ pub fn insn_to_str( let name = &index.columns.get(*column).unwrap().name; Some(name) } - CursorType::Pseudo(pseudo_table) => { - let name = pseudo_table.columns.get(*column).unwrap().name.as_ref(); - name - } + CursorType::Pseudo(pseudo_table) => None, CursorType::Sorter => None, CursorType::VirtualTable(v) => v.columns.get(*column).unwrap().name.as_ref(), }; From 3907b387b36260c3372c78d785c7ed4e5acba85e Mon Sep 17 00:00:00 2001 From: "Levy A." Date: Mon, 30 Jun 2025 13:36:01 -0300 Subject: [PATCH 2/4] cargo fix --- core/translate/group_by.rs | 2 +- core/translate/order_by.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/translate/group_by.rs b/core/translate/group_by.rs index 511ce5fee..cfaf5e2e3 100644 --- a/core/translate/group_by.rs +++ b/core/translate/group_by.rs @@ -6,7 +6,7 @@ use crate::translate::expr::{walk_expr, WalkControl}; use crate::translate::plan::ResultSetColumn; use crate::{ function::AggFunc, - schema::{Column, PseudoTable}, + schema::PseudoTable, translate::collate::CollationSeq, util::exprs_are_equivalent, vdbe::{ diff --git a/core/translate/order_by.rs b/core/translate/order_by.rs index 70df8c772..d3b7a57e7 100644 --- a/core/translate/order_by.rs +++ b/core/translate/order_by.rs @@ -3,7 +3,7 @@ use std::rc::Rc; use turso_sqlite3_parser::ast::{self, SortOrder}; use crate::{ - schema::{Column, PseudoTable}, + schema::PseudoTable, translate::collate::CollationSeq, util::exprs_are_equivalent, vdbe::{ From ffd6844b5b6b122f8a01038318fd956d0a1504bc Mon Sep 17 00:00:00 2001 From: "Levy A." Date: Mon, 30 Jun 2025 13:50:26 -0300 Subject: [PATCH 3/4] refactor: remove `PseudoTable` from `Table` the only reason for `PseudoTable` to exist, is to provide column information for `PseudoCursor` creation. this should not be part of the schema. --- core/schema.rs | 17 +++++------------ core/translate/display.rs | 2 +- core/translate/expr.rs | 1 - core/translate/group_by.rs | 10 +++------- core/translate/index.rs | 7 ++++--- core/translate/main_loop.rs | 2 -- core/translate/order_by.rs | 9 +++------ core/translate/plan.rs | 1 - core/translate/schema.rs | 1 - core/vdbe/builder.rs | 4 ++-- core/vdbe/explain.rs | 2 +- 11 files changed, 19 insertions(+), 37 deletions(-) diff --git a/core/schema.rs b/core/schema.rs index df6f8ea4f..b64b72296 100644 --- a/core/schema.rs +++ b/core/schema.rs @@ -136,7 +136,6 @@ impl Schema { #[derive(Clone, Debug)] pub enum Table { BTree(Rc), - Pseudo(Rc), Virtual(Rc), FromClauseSubquery(FromClauseSubquery), } @@ -145,7 +144,6 @@ impl Table { pub fn get_root_page(&self) -> usize { match self { Table::BTree(table) => table.root_page, - Table::Pseudo(_) => unimplemented!(), Table::Virtual(_) => unimplemented!(), Table::FromClauseSubquery(_) => unimplemented!(), } @@ -154,7 +152,6 @@ impl Table { pub fn get_name(&self) -> &str { match self { Self::BTree(table) => &table.name, - Self::Pseudo(_) => "", Self::Virtual(table) => &table.name, Self::FromClauseSubquery(from_clause_subquery) => &from_clause_subquery.name, } @@ -163,7 +160,6 @@ impl Table { pub fn get_column_at(&self, index: usize) -> Option<&Column> { match self { Self::BTree(table) => table.columns.get(index), - Self::Pseudo(table) => unimplemented!(), Self::Virtual(table) => table.columns.get(index), Self::FromClauseSubquery(from_clause_subquery) => { from_clause_subquery.columns.get(index) @@ -174,7 +170,6 @@ impl Table { pub fn columns(&self) -> &Vec { match self { Self::BTree(table) => &table.columns, - Self::Pseudo(table) => unimplemented!(), Self::Virtual(table) => &table.columns, Self::FromClauseSubquery(from_clause_subquery) => &from_clause_subquery.columns, } @@ -183,7 +178,6 @@ impl Table { pub fn btree(&self) -> Option> { match self { Self::BTree(table) => Some(table.clone()), - Self::Pseudo(_) => None, Self::Virtual(_) => None, Self::FromClauseSubquery(_) => None, } @@ -201,7 +195,6 @@ impl PartialEq for Table { fn eq(&self, other: &Self) -> bool { match (self, other) { (Self::BTree(a), Self::BTree(b)) => Rc::ptr_eq(a, b), - (Self::Pseudo(a), Self::Pseudo(b)) => Rc::ptr_eq(a, b), (Self::Virtual(a), Self::Virtual(b)) => Rc::ptr_eq(a, b), _ => false, } @@ -291,12 +284,12 @@ impl BTreeTable { } } -#[derive(Debug, Default)] -pub struct PseudoTable { +#[derive(Debug, Default, Clone, Copy)] +pub struct PseudoCursorType { pub column_count: usize, } -impl PseudoTable { +impl PseudoCursorType { pub fn new() -> Self { Self { column_count: 0 } } @@ -557,8 +550,8 @@ fn create_table( }) } -pub fn _build_pseudo_table(columns: &[ResultColumn]) -> PseudoTable { - let table = PseudoTable::new(); +pub fn _build_pseudo_table(columns: &[ResultColumn]) -> PseudoCursorType { + let table = PseudoCursorType::new(); for column in columns { match column { ResultColumn::Expr(expr, _as_name) => { diff --git a/core/translate/display.rs b/core/translate/display.rs index cbe5b0455..1c8d2d8e4 100644 --- a/core/translate/display.rs +++ b/core/translate/display.rs @@ -321,7 +321,7 @@ impl ToSqlString for JoinedTable { ) -> String { let table_or_subquery = match &self.table { - Table::BTree(..) | Table::Pseudo(..) | Table::Virtual(..) => { + Table::BTree(..) | Table::Virtual(..) => { self.table.get_name().to_string() } Table::FromClauseSubquery(from_clause_subquery) => { diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 1268bdf39..b64a14b64 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -1930,7 +1930,6 @@ pub fn translate_expr( }); Ok(target_register) } - Table::Pseudo(_) => panic!("Column access on pseudo table"), } } ast::Expr::RowId { diff --git a/core/translate/group_by.rs b/core/translate/group_by.rs index cfaf5e2e3..351f4b6fb 100644 --- a/core/translate/group_by.rs +++ b/core/translate/group_by.rs @@ -1,12 +1,10 @@ -use std::rc::Rc; - use turso_sqlite3_parser::ast; use crate::translate::expr::{walk_expr, WalkControl}; use crate::translate::plan::ResultSetColumn; use crate::{ function::AggFunc, - schema::PseudoTable, + schema::PseudoCursorType, translate::collate::CollationSeq, util::exprs_are_equivalent, vdbe::{ @@ -306,11 +304,9 @@ pub fn group_by_create_pseudo_table( ) -> usize { // Create a pseudo-table to read one row at a time from the sorter // This allows us to use standard table access operations on the sorted data - let pseudo_table = Rc::new(PseudoTable { + program.alloc_cursor_id(CursorType::Pseudo(PseudoCursorType { column_count: sorter_column_count, - }); - - program.alloc_cursor_id(CursorType::Pseudo(pseudo_table.clone())) + })) } /// In case sorting is needed for GROUP BY, sorts the rows in the GROUP BY sorter diff --git a/core/translate/index.rs b/core/translate/index.rs index b729cc46e..73496c6a5 100644 --- a/core/translate/index.rs +++ b/core/translate/index.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use crate::vdbe::insn::CmpInsFlags; use crate::{ - schema::{BTreeTable, Column, Index, IndexColumn, PseudoTable, Schema}, + schema::{BTreeTable, Column, Index, IndexColumn, PseudoCursorType, Schema}, storage::pager::CreateBTreeFlags, util::normalize_ident, vdbe::{ @@ -83,8 +83,9 @@ pub fn translate_create_index( let btree_cursor_id = program.alloc_cursor_id(CursorType::BTreeIndex(idx.clone())); let table_cursor_id = program.alloc_cursor_id(CursorType::BTreeTable(tbl.clone())); let sorter_cursor_id = program.alloc_cursor_id(CursorType::Sorter); - let pseudo_table = PseudoTable::new_with_columns(tbl.columns.clone()); - let pseudo_cursor_id = program.alloc_cursor_id(CursorType::Pseudo(pseudo_table.into())); + let pseudo_cursor_id = program.alloc_cursor_id(CursorType::Pseudo(PseudoCursorType { + column_count: tbl.columns.len(), + })); // Create a new B-Tree and store the root page index in a register let root_page_reg = program.alloc_register(); diff --git a/core/translate/main_loop.rs b/core/translate/main_loop.rs index d715a4f69..88c18c054 100644 --- a/core/translate/main_loop.rs +++ b/core/translate/main_loop.rs @@ -563,7 +563,6 @@ pub fn open_loop( end_offset: loop_end, }); } - Table::Pseudo(_) => panic!("Pseudo tables should not loop"), } if let Some(table_cursor_id) = table_cursor_id { @@ -1042,7 +1041,6 @@ pub fn close_loop( target_pc: loop_labels.loop_start, }); } - other => unreachable!("Unsupported table reference type: {:?}", other), } program.preassign_label_to_next_insn(loop_labels.loop_end); } diff --git a/core/translate/order_by.rs b/core/translate/order_by.rs index d3b7a57e7..3fb9ec21a 100644 --- a/core/translate/order_by.rs +++ b/core/translate/order_by.rs @@ -1,9 +1,7 @@ -use std::rc::Rc; - use turso_sqlite3_parser::ast::{self, SortOrder}; use crate::{ - schema::PseudoTable, + schema::PseudoCursorType, translate::collate::CollationSeq, util::exprs_are_equivalent, vdbe::{ @@ -95,10 +93,9 @@ pub fn emit_order_by( .map(|v| v.len()) .unwrap_or(0); - let pseudo_table = Rc::new(PseudoTable { + let pseudo_cursor = program.alloc_cursor_id(CursorType::Pseudo(PseudoCursorType { column_count: sorter_column_count, - }); - let pseudo_cursor = program.alloc_cursor_id(CursorType::Pseudo(pseudo_table.clone())); + })); let SortMetadata { sort_cursor, reg_sorter_data, diff --git a/core/translate/plan.rs b/core/translate/plan.rs index 9d8c102ae..462f38442 100644 --- a/core/translate/plan.rs +++ b/core/translate/plan.rs @@ -989,7 +989,6 @@ impl JoinedTable { let index_cursor_id = None; Ok((table_cursor_id, index_cursor_id)) } - Table::Pseudo(_) => Ok((None, None)), Table::FromClauseSubquery(..) => Ok((None, None)), } } diff --git a/core/translate/schema.rs b/core/translate/schema.rs index 6b875c935..20ee64bc2 100644 --- a/core/translate/schema.rs +++ b/core/translate/schema.rs @@ -749,7 +749,6 @@ pub fn translate_drop_table( db: 0, // TODO change this for multiple databases }); } - Table::Pseudo(..) => unimplemented!(), Table::FromClauseSubquery(..) => panic!("FromClauseSubquery can't be dropped"), }; diff --git a/core/vdbe/builder.rs b/core/vdbe/builder.rs index 4654ae329..278728174 100644 --- a/core/vdbe/builder.rs +++ b/core/vdbe/builder.rs @@ -6,7 +6,7 @@ use turso_sqlite3_parser::ast::{self, TableInternalId}; use crate::{ numeric::Numeric, parameters::Parameters, - schema::{BTreeTable, Index, PseudoTable, Table}, + schema::{BTreeTable, Index, PseudoCursorType, Table}, translate::{ collate::CollationSeq, emitter::TransactionMode, @@ -116,7 +116,7 @@ pub struct ProgramBuilder { pub enum CursorType { BTreeTable(Rc), BTreeIndex(Arc), - Pseudo(Rc), + Pseudo(PseudoCursorType), Sorter, VirtualTable(Rc), } diff --git a/core/vdbe/explain.rs b/core/vdbe/explain.rs index 8fe97dc89..933b82ee2 100644 --- a/core/vdbe/explain.rs +++ b/core/vdbe/explain.rs @@ -539,7 +539,7 @@ pub fn insn_to_str( let name = &index.columns.get(*column).unwrap().name; Some(name) } - CursorType::Pseudo(pseudo_table) => None, + CursorType::Pseudo(_) => None, CursorType::Sorter => None, CursorType::VirtualTable(v) => v.columns.get(*column).unwrap().name.as_ref(), }; From 25927d91d88edcf25b2240864b5d2af24e14c929 Mon Sep 17 00:00:00 2001 From: "Levy A." Date: Mon, 30 Jun 2025 14:37:51 -0300 Subject: [PATCH 4/4] cargo fmt --- core/schema.rs | 4 +++- core/translate/display.rs | 4 +--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/schema.rs b/core/schema.rs index b64b72296..6c11ac077 100644 --- a/core/schema.rs +++ b/core/schema.rs @@ -295,7 +295,9 @@ impl PseudoCursorType { } pub fn new_with_columns(columns: impl AsRef<[Column]>) -> Self { - Self { column_count: columns.as_ref().len() } + Self { + column_count: columns.as_ref().len(), + } } } diff --git a/core/translate/display.rs b/core/translate/display.rs index 1c8d2d8e4..229559f63 100644 --- a/core/translate/display.rs +++ b/core/translate/display.rs @@ -321,9 +321,7 @@ impl ToSqlString for JoinedTable { ) -> String { let table_or_subquery = match &self.table { - Table::BTree(..) | Table::Virtual(..) => { - self.table.get_name().to_string() - } + Table::BTree(..) | Table::Virtual(..) => self.table.get_name().to_string(), Table::FromClauseSubquery(from_clause_subquery) => { // Could possibly merge the contexts together here format!(