diff --git a/core/schema.rs b/core/schema.rs index 93b245090..415d5f4e1 100644 --- a/core/schema.rs +++ b/core/schema.rs @@ -7,7 +7,7 @@ use limbo_sqlite3_parser::{ ast::{Cmd, CreateTableBody, QualifiedName, ResultColumn, Stmt}, lexer::sql::Parser, }; -use std::collections::HashMap; +use std::collections::{BTreeSet, HashMap}; use std::rc::Rc; use std::sync::Arc; use tracing::trace; @@ -274,6 +274,30 @@ impl PseudoTable { } } +#[derive(Debug, Eq)] +struct UniqueColumnProps { + column_name: String, + order: SortOrder, +} + +impl PartialEq for UniqueColumnProps { + fn eq(&self, other: &Self) -> bool { + self.column_name.eq(&other.column_name) + } +} + +impl PartialOrd for UniqueColumnProps { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for UniqueColumnProps { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.column_name.cmp(&other.column_name) + } +} + fn create_table( tbl_name: QualifiedName, body: CreateTableBody, @@ -285,7 +309,8 @@ fn create_table( let mut primary_key_columns = vec![]; let mut cols = vec![]; let is_strict: bool; - let mut unique_sets: Vec> = vec![]; + // BtreeSet here to preserve order of inserted keys + let mut unique_sets: Vec> = vec![]; match body { CreateTableBody::ColumnsAndConstraints { columns, @@ -320,16 +345,19 @@ fn create_table( if conflict_clause.is_some() { unimplemented!("ON CONFLICT not implemented"); } - let unique_set: Vec<_> = columns + let unique_set = columns .into_iter() .map(|column| { - let col_name = match column.expr { + let column_name = match column.expr { Expr::Id(id) => normalize_ident(&id.0), _ => { todo!("Unsupported unique expression"); } }; - (col_name, column.order.unwrap_or(SortOrder::Asc)) + UniqueColumnProps { + column_name, + order: column.order.unwrap_or(SortOrder::Asc), + } }) .collect(); unique_sets.push(unique_set); @@ -461,7 +489,18 @@ fn create_table( unique_sets: if unique_sets.is_empty() { None } else { - Some(unique_sets) + // Sort first so that dedup operation removes all duplicates + unique_sets.dedup(); + Some( + unique_sets + .into_iter() + .map(|set| { + set.into_iter() + .map(|UniqueColumnProps { column_name, order }| (column_name, order)) + .collect() + }) + .collect(), + ) }, }) } @@ -829,7 +868,8 @@ impl Index { // I wanted to just chain the iterator above but Rust type system get's messy with Iterators. // It would not allow me chain them even by using a core::iter::empty() // To circumvent this, I'm having to allocate a second Vec, and extend the other from it. - let has_primary_key_index = table.get_rowid_alias_column().is_none() && !table.primary_key_columns.is_empty(); + let has_primary_key_index = + table.get_rowid_alias_column().is_none() && !table.primary_key_columns.is_empty(); if has_primary_key_index { let (index_name, root_page) = auto_indices.next().expect( "number of auto_indices in schema should be same number of indices calculated", @@ -1444,4 +1484,29 @@ mod tests { Ok(()) } + + #[test] + fn test_automatic_index_unique_set_dedup() -> Result<()> { + let sql = r#"CREATE TABLE t1 (a, b, UNIQUE(a, b), UNIQUE(a, b));"#; + let table = BTreeTable::from_sql(sql, 0)?; + let mut index = Index::automatic_from_primary_key_and_unique( + &table, + vec![("sqlite_autoindex_t1_1".to_string(), 2)], + )?; + + assert!(index.len() == 1); + let index = index.pop().unwrap(); + + assert_eq!(index.name, "sqlite_autoindex_t1_1"); + assert_eq!(index.table_name, "t1"); + assert_eq!(index.root_page, 2); + assert!(index.unique); + assert_eq!(index.columns.len(), 2); + assert_eq!(index.columns[0].name, "a"); + assert!(matches!(index.columns[0].order, SortOrder::Asc)); + assert_eq!(index.columns[1].name, "b"); + assert!(matches!(index.columns[1].order, SortOrder::Asc)); + + Ok(()) + } } diff --git a/core/translate/optimizer/join.rs b/core/translate/optimizer/join.rs index aceb2b889..97a4ebb62 100644 --- a/core/translate/optimizer/join.rs +++ b/core/translate/optimizer/join.rs @@ -1204,6 +1204,7 @@ mod tests { primary_key: false, notnull: false, default: None, + unique: false, } } fn _create_column_of_type(name: &str, ty: Type) -> Column { @@ -1238,6 +1239,7 @@ mod tests { columns, has_rowid: true, is_strict: false, + unique_sets: None, }) } diff --git a/core/translate/schema.rs b/core/translate/schema.rs index 7ab15ee09..8fcb8abb8 100644 --- a/core/translate/schema.rs +++ b/core/translate/schema.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::fmt::Display; use std::ops::Range; use std::rc::Rc; @@ -273,6 +274,8 @@ fn check_automatic_pk_index_required( } => { let mut primary_key_definition = None; let mut unique_def_count = 0usize; + // Used to dedup named unique constraints + let mut unique_sets = vec![]; // Check table constraints for PRIMARY KEY if let Some(constraints) = constraints { @@ -325,6 +328,26 @@ fn check_automatic_pk_index_required( }); } } + } else if let ast::TableConstraint::Unique { + columns, + conflict_clause, + } = &constraint.constraint + { + if conflict_clause.is_some() { + unimplemented!("ON CONFLICT not implemented"); + } + let col_names: HashSet = columns + .iter() + .map(|column| match &column.expr { + limbo_sqlite3_parser::ast::Expr::Id(id) => { + crate::util::normalize_ident(&id.0) + } + _ => { + todo!("Unsupported unique expression"); + } + }) + .collect(); + unique_sets.push(col_names); } } } @@ -355,7 +378,8 @@ fn check_automatic_pk_index_required( bail_parse_error!("WITHOUT ROWID tables are not supported yet"); } - let mut total_indices = unique_def_count; + unique_sets.dedup(); + let mut total_indices = unique_def_count + unique_sets.len(); // Check if we need an automatic index let auto_index_pk = if let Some(primary_key_definition) = &primary_key_definition { diff --git a/vendored/sqlite3-parser/src/parser/ast/mod.rs b/vendored/sqlite3-parser/src/parser/ast/mod.rs index f2275e952..087df0534 100644 --- a/vendored/sqlite3-parser/src/parser/ast/mod.rs +++ b/vendored/sqlite3-parser/src/parser/ast/mod.rs @@ -1460,7 +1460,7 @@ bitflags::bitflags! { } /// Sort orders -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub enum SortOrder { /// `ASC` Asc,