diff --git a/core/schema.rs b/core/schema.rs index 1458b968e..3d7f3b074 100644 --- a/core/schema.rs +++ b/core/schema.rs @@ -9,7 +9,7 @@ use limbo_sqlite3_parser::{ ast::{Cmd, CreateTableBody, QualifiedName, ResultColumn, Stmt}, lexer::sql::Parser, }; -use std::collections::{BTreeSet, HashMap}; +use std::collections::{BTreeSet, HashMap, HashSet}; use std::rc::Rc; use std::sync::Arc; use tracing::trace; @@ -19,20 +19,30 @@ const SCHEMA_TABLE_NAME_ALT: &str = "sqlite_master"; pub struct Schema { pub tables: HashMap>, - // table_name to list of indexes for the table + /// table_name to list of indexes for the table pub indexes: HashMap>>, + /// Used for index_experimental feature flag to track whether a table has an index. + /// This is necessary because we won't populate indexes so that we don't use them but + /// we still need to know if a table has an index to disallow any write operation that requires + /// indexes. + pub has_indexes: HashSet, } impl Schema { pub fn new() -> Self { let mut tables: HashMap> = HashMap::new(); + let mut has_indexes = HashSet::new(); let indexes: HashMap>> = HashMap::new(); #[allow(clippy::arc_with_non_send_sync)] tables.insert( SCHEMA_TABLE_NAME.to_string(), Arc::new(Table::BTree(sqlite_schema_table().into())), ); - Self { tables, indexes } + Self { + tables, + indexes, + has_indexes, + } } pub fn is_unique_idx_name(&self, name: &str) -> bool { @@ -112,6 +122,16 @@ impl Schema { .expect("Must have the index") .retain_mut(|other_idx| other_idx.name != idx.name); } + + #[cfg(not(feature = "index_experimental"))] + pub fn table_has_indexes(&self, table_name: &str) -> bool { + self.has_indexes.contains(table_name) + } + + #[cfg(not(feature = "index_experimental"))] + pub fn table_set_has_index(&mut self, table_name: &str) { + self.has_indexes.insert(table_name.to_string()); + } } #[derive(Clone, Debug)] diff --git a/core/translate/alter.rs b/core/translate/alter.rs index 62649a90e..4ea08e997 100644 --- a/core/translate/alter.rs +++ b/core/translate/alter.rs @@ -25,13 +25,15 @@ pub fn translate_alter_table( ) -> Result { let (table_name, alter_table) = alter; let ast::Name(table_name) = table_name.name; - let indexes = schema.get_indices(&table_name); - if !indexes.is_empty() && cfg!(not(feature = "index_experimental")) { - // Let's disable altering a table with indices altogether instead of checking column by - // column to be extra safe. - bail_parse_error!( - "Alter table disabled for table with indexes without index_experimental feature flag" - ); + #[cfg(not(feature = "index_experimental"))] + { + if schema.table_has_indexes(&table_name) && cfg!(not(feature = "index_experimental")) { + // Let's disable altering a table with indices altogether instead of checking column by + // column to be extra safe. + bail_parse_error!( + "Alter table disabled for table with indexes without index_experimental feature flag" + ); + } } let Some(original_btree) = schema diff --git a/core/translate/delete.rs b/core/translate/delete.rs index 3fe7af06e..c8357b47c 100644 --- a/core/translate/delete.rs +++ b/core/translate/delete.rs @@ -19,13 +19,15 @@ pub fn translate_delete( syms: &SymbolTable, mut program: ProgramBuilder, ) -> Result { - let indexes = schema.get_indices(&tbl_name.name.to_string()); - if !indexes.is_empty() && cfg!(not(feature = "index_experimental")) { - // Let's disable altering a table with indices altogether instead of checking column by - // column to be extra safe. - bail_parse_error!( - "DELETE into table disabled for table with indexes and without index_experimental feature flag" - ); + #[cfg(not(feature = "index_experimental"))] + { + if schema.table_has_indexes(&tbl_name.name.to_string()) { + // Let's disable altering a table with indices altogether instead of checking column by + // column to be extra safe. + bail_parse_error!( + "DELETE into table disabled for table with indexes and without index_experimental feature flag" + ); + } } let mut delete_plan = prepare_delete_plan( schema, diff --git a/core/translate/insert.rs b/core/translate/insert.rs index e9bcbd4d2..c81516f46 100644 --- a/core/translate/insert.rs +++ b/core/translate/insert.rs @@ -58,13 +58,15 @@ pub fn translate_insert( crate::bail_parse_error!("ON CONFLICT clause is not supported"); } - let indexes = schema.get_indices(&tbl_name.name.to_string()); - if !indexes.is_empty() && cfg!(not(feature = "index_experimental")) { - // Let's disable altering a table with indices altogether instead of checking column by - // column to be extra safe. - bail_parse_error!( - "INSERT table disabled for table with indexes and without index_experimental feature flag" - ); + #[cfg(not(feature = "index_experimental"))] + { + if schema.table_has_indexes(&tbl_name.name.to_string()) { + // Let's disable altering a table with indices altogether instead of checking column by + // column to be extra safe. + bail_parse_error!( + "INSERT table disabled for table with indexes and without index_experimental feature flag" + ); + } } let table_name = &tbl_name.name; let table = match schema.get_table(table_name.0.as_str()) { diff --git a/core/translate/schema.rs b/core/translate/schema.rs index e4d5f5948..49f792e47 100644 --- a/core/translate/schema.rs +++ b/core/translate/schema.rs @@ -613,6 +613,13 @@ pub fn translate_drop_table( schema: &Schema, mut program: ProgramBuilder, ) -> Result { + if cfg!(not(feature = "index_experimental")) + && schema.table_has_indexes(&tbl_name.name.to_string()) + { + bail_parse_error!( + "DROP Table with indexes on the table enabled only with index_experimental feature" + ); + } let opts = ProgramBuilderOpts { query_mode, num_cursors: 3, diff --git a/core/translate/update.rs b/core/translate/update.rs index 7cb94d6f3..89f0de71c 100644 --- a/core/translate/update.rs +++ b/core/translate/update.rs @@ -101,13 +101,15 @@ pub fn prepare_update_plan( bail_parse_error!("ON CONFLICT clause is not supported"); } let table_name = &body.tbl_name.name; - let indexes = schema.get_indices(&table_name.to_string()); - if !indexes.is_empty() && cfg!(not(feature = "index_experimental")) { - // Let's disable altering a table with indices altogether instead of checking column by - // column to be extra safe. - bail_parse_error!( - "INSERT table disabled for table with indexes and without index_experimental feature flag" - ); + #[cfg(not(feature = "index_experimental"))] + { + if schema.table_has_indexes(&table_name.to_string()) { + // Let's disable altering a table with indices altogether instead of checking column by + // column to be extra safe. + bail_parse_error!( + "UPDATE table disabled for table with indexes and without index_experimental feature flag" + ); + } } let table = match schema.get_table(table_name.0.as_str()) { Some(table) => table, diff --git a/core/util.rs b/core/util.rs index c5163b995..0c52eabaa 100644 --- a/core/util.rs +++ b/core/util.rs @@ -136,29 +136,34 @@ pub fn parse_schema_rows( StepResult::Busy => break, } } - #[cfg(feature = "index_experimental")] - { - for UnparsedFromSqlIndex { - table_name, - root_page, - sql, - } in from_sql_indexes - { - let table = schema.get_btree_table(&table_name).unwrap(); - let index = schema::Index::from_sql(&sql, root_page as usize, table.as_ref())?; - schema.add_index(Arc::new(index)); - } + for unparsed_sql_from_index in from_sql_indexes { + #[cfg(not(feature = "index_experimental"))] + schema.table_set_has_index(&unparsed_sql_from_index.table_name); #[cfg(feature = "index_experimental")] { - for (table_name, indices) in automatic_indices { - let table = schema.get_btree_table(&table_name).unwrap(); - let ret_index = schema::Index::automatic_from_primary_key_and_unique( - table.as_ref(), - indices, - )?; - for index in ret_index { - schema.add_index(Arc::new(index)); - } + let table = schema + .get_btree_table(&unparsed_sql_from_index.table_name) + .unwrap(); + let index = schema::Index::from_sql( + &unparsed_sql_from_index.sql, + unparsed_sql_from_index.root_page as usize, + table.as_ref(), + )?; + schema.add_index(Arc::new(index)); + } + } + for automatic_index in &automatic_indices { + #[cfg(not(feature = "index_experimental"))] + schema.table_set_has_index(&automatic_index.0); + #[cfg(feature = "index_experimental")] + { + let table = schema.get_btree_table(&automatic_index.0).unwrap(); + let ret_index = schema::Index::automatic_from_primary_key_and_unique( + table.as_ref(), + &automatic_index.1, + )?; + for index in ret_index { + schema.add_index(Arc::new(index)); } } }