From 9ddb0befc447a46b1fa06621a68120e1a59aa364 Mon Sep 17 00:00:00 2001 From: Piotr Jastrzebski Date: Sun, 7 Jul 2024 14:19:08 +0200 Subject: [PATCH 1/5] Add has_rowid field to BTreeTable Signed-off-by: Piotr Jastrzebski --- core/schema.rs | 3 +++ core/storage.rs | 5 ++++- core/util.rs | 3 ++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/core/schema.rs b/core/schema.rs index d55018176..1e5f73cf6 100644 --- a/core/schema.rs +++ b/core/schema.rs @@ -72,6 +72,7 @@ pub struct BTreeTable { pub root_page: usize, pub name: String, pub columns: Vec, + pub has_rowid: bool, } impl BTreeTable { @@ -198,6 +199,7 @@ fn create_table( Ok(BTreeTable { root_page, name: table_name, + has_rowid: true, columns: cols, }) } @@ -256,6 +258,7 @@ pub fn sqlite_schema_table() -> BTreeTable { BTreeTable { root_page: 1, name: "sqlite_schema".to_string(), + has_rowid: true, columns: vec![ Column { name: "type".to_string(), diff --git a/core/storage.rs b/core/storage.rs index 41e430f91..c210cb566 100644 --- a/core/storage.rs +++ b/core/storage.rs @@ -72,7 +72,10 @@ impl PageIO for FileStorage { fn get(&self, page_idx: usize, c: Rc) -> Result<()> { let size = c.buf().len(); assert!(page_idx > 0); - ensure!(size >= 1 << 9 && size <= 1 << 16 && size & (size - 1) == 0, StorageError::NotADB); + ensure!( + size >= 1 << 9 && size <= 1 << 16 && size & (size - 1) == 0, + StorageError::NotADB + ); let pos = (page_idx - 1) * size; self.file.pread(pos, c)?; Ok(()) diff --git a/core/util.rs b/core/util.rs index 07a9f4b7e..32dc322c8 100644 --- a/core/util.rs +++ b/core/util.rs @@ -3,5 +3,6 @@ pub fn normalize_ident(ident: &str) -> String { &ident[1..ident.len() - 1] } else { ident - }).to_lowercase() + }) + .to_lowercase() } From 2aa0a929558c334620a120b88d246ba103fc584e Mon Sep 17 00:00:00 2001 From: Piotr Jastrzebski Date: Sun, 7 Jul 2024 14:27:38 +0200 Subject: [PATCH 2/5] Setup has_rowid correctly for BTreeTable Signed-off-by: Piotr Jastrzebski --- core/schema.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/core/schema.rs b/core/schema.rs index 1e5f73cf6..0de192ad3 100644 --- a/core/schema.rs +++ b/core/schema.rs @@ -1,7 +1,9 @@ +use crate::util::normalize_ident; use anyhow::Result; use core::fmt; use fallible_iterator::FallibleIterator; use log::trace; +use sqlite3_parser::ast::TableOptions; use sqlite3_parser::{ ast::{Cmd, CreateTableBody, QualifiedName, ResultColumn, Stmt}, lexer::sql::Parser, @@ -9,8 +11,6 @@ use sqlite3_parser::{ use std::collections::HashMap; use std::rc::Rc; -use crate::util::normalize_ident; - pub struct Schema { pub tables: HashMap>, } @@ -153,9 +153,14 @@ fn create_table( ) -> Result { let table_name = normalize_ident(&tbl_name.name.0); trace!("Creating table {}", table_name); + let mut has_rowid = true; let mut cols = vec![]; match body { - CreateTableBody::ColumnsAndConstraints { columns, .. } => { + CreateTableBody::ColumnsAndConstraints { + columns, + constraints: _, + options, + } => { for column in columns { let name = column.col_name.0.to_string(); let ty = match column.col_type { @@ -193,13 +198,16 @@ fn create_table( primary_key, }); } + if options.contains(TableOptions::WITHOUT_ROWID) { + has_rowid = false; + } } CreateTableBody::AsSelect(_) => todo!(), }; Ok(BTreeTable { root_page, name: table_name, - has_rowid: true, + has_rowid, columns: cols, }) } From 50ecea0c86cec358b4939fe7677215549b36066d Mon Sep 17 00:00:00 2001 From: Piotr Jastrzebski Date: Sun, 7 Jul 2024 14:50:39 +0200 Subject: [PATCH 3/5] Use has_rowid in column_is_rowid_alias Signed-off-by: Piotr Jastrzebski --- core/schema.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/schema.rs b/core/schema.rs index 0de192ad3..16c2de666 100644 --- a/core/schema.rs +++ b/core/schema.rs @@ -78,7 +78,7 @@ pub struct BTreeTable { impl BTreeTable { pub fn column_is_rowid_alias(&self, col: &Column) -> bool { let composite_primary_key = self.columns.iter().filter(|col| col.primary_key).count() > 1; - col.primary_key && col.ty == Type::Integer && !composite_primary_key + col.primary_key && col.ty == Type::Integer && !composite_primary_key && self.has_rowid } pub fn get_column(&self, name: &str) -> Option<(usize, &Column)> { From 73e037afa286c44abc4d095e45bfd17e31a1e6aa Mon Sep 17 00:00:00 2001 From: Piotr Jastrzebski Date: Sun, 7 Jul 2024 14:51:22 +0200 Subject: [PATCH 4/5] Add tests for hes_rowid field Signed-off-by: Piotr Jastrzebski --- core/schema.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/core/schema.rs b/core/schema.rs index 16c2de666..ac3b49da6 100644 --- a/core/schema.rs +++ b/core/schema.rs @@ -301,6 +301,22 @@ pub fn sqlite_schema_table() -> BTreeTable { mod tests { use super::*; + #[test] + pub fn test_has_rowid_true() -> Result<()> { + let sql = r#"CREATE TABLE t1 (a INTEGER PRIMARY KEY, b TEXT);"#; + let table = BTreeTable::from_sql(sql, 0)?; + assert!(table.has_rowid, "has_rowid should be set to true"); + Ok(()) + } + + #[test] + pub fn test_has_rowid_false() -> Result<()> { + let sql = r#"CREATE TABLE t1 (a INTEGER PRIMARY KEY, b TEXT) WITHOUT ROWID;"#; + let table = BTreeTable::from_sql(sql, 0)?; + assert!(!table.has_rowid, "has_rowid should be set to false"); + Ok(()) + } + #[test] pub fn test_sqlite_schema() { let expected = r#"CREATE TABLE sqlite_schema ( From 828c4ce45929355870aaab4f9aadc88feb8a7d2f Mon Sep 17 00:00:00 2001 From: Piotr Jastrzebski Date: Sun, 7 Jul 2024 15:14:38 +0200 Subject: [PATCH 5/5] Add tests for BTreeTable::column_is_rowid_alias Signed-off-by: Piotr Jastrzebski --- core/schema.rs | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/core/schema.rs b/core/schema.rs index ac3b49da6..1d3518912 100644 --- a/core/schema.rs +++ b/core/schema.rs @@ -317,6 +317,93 @@ mod tests { Ok(()) } + #[test] + pub fn test_column_is_rowid_alias_single_text() -> Result<()> { + let sql = r#"CREATE TABLE t1 (a TEXT PRIMARY KEY, b TEXT);"#; + let table = BTreeTable::from_sql(sql, 0)?; + let column = table.get_column("a").unwrap().1; + assert!( + !table.column_is_rowid_alias(column), + "column 'a´ has type different than INTEGER so can't be a rowid alias" + ); + Ok(()) + } + + #[test] + pub fn test_column_is_rowid_alias_single_integer() -> Result<()> { + let sql = r#"CREATE TABLE t1 (a INTEGER PRIMARY KEY, b TEXT);"#; + let table = BTreeTable::from_sql(sql, 0)?; + let column = table.get_column("a").unwrap().1; + assert!( + table.column_is_rowid_alias(column), + "column 'a´ should be a rowid alias" + ); + Ok(()) + } + + #[test] + #[ignore] // We don't support separate definition of primary keys yet + pub fn test_column_is_rowid_alias_single_integer_separate_primary_key_definition() -> Result<()> + { + let sql = r#"CREATE TABLE t1 (a INTEGER, b TEXT, PRIMARY KEY(a));"#; + let table = BTreeTable::from_sql(sql, 0)?; + let column = table.get_column("a").unwrap().1; + assert!( + table.column_is_rowid_alias(column), + "column 'a´ should be a rowid alias" + ); + Ok(()) + } + + #[test] + pub fn test_column_is_rowid_alias_single_integer_separate_primary_key_definition_without_rowid( + ) -> Result<()> { + let sql = r#"CREATE TABLE t1 (a INTEGER, b TEXT, PRIMARY KEY(a)) WITHOUT ROWID;"#; + let table = BTreeTable::from_sql(sql, 0)?; + let column = table.get_column("a").unwrap().1; + assert!( + !table.column_is_rowid_alias(column), + "column 'a´ shouldn't be a rowid alias because table has no rowid" + ); + Ok(()) + } + + #[test] + pub fn test_column_is_rowid_alias_single_integer_without_rowid() -> Result<()> { + let sql = r#"CREATE TABLE t1 (a INTEGER PRIMARY KEY, b TEXT) WITHOUT ROWID;"#; + let table = BTreeTable::from_sql(sql, 0)?; + let column = table.get_column("a").unwrap().1; + assert!( + !table.column_is_rowid_alias(column), + "column 'a´ shouldn't be a rowid alias because table has no rowid" + ); + Ok(()) + } + + #[test] + pub fn test_column_is_rowid_alias_inline_composite_primary_key() -> Result<()> { + let sql = r#"CREATE TABLE t1 (a INTEGER PRIMARY KEY, b TEXT PRIMARY KEY);"#; + let table = BTreeTable::from_sql(sql, 0)?; + let column = table.get_column("a").unwrap().1; + assert!( + !table.column_is_rowid_alias(column), + "column 'a´ shouldn't be a rowid alias because table has composite primary key" + ); + Ok(()) + } + + #[test] + pub fn test_column_is_rowid_alias_separate_composite_primary_key_definition() -> Result<()> { + let sql = r#"CREATE TABLE t1 (a INTEGER, b TEXT, PRIMARY KEY(a, b));"#; + let table = BTreeTable::from_sql(sql, 0)?; + let column = table.get_column("a").unwrap().1; + assert!( + !table.column_is_rowid_alias(column), + "column 'a´ shouldn't be a rowid alias because table has composite primary key" + ); + Ok(()) + } + #[test] pub fn test_sqlite_schema() { let expected = r#"CREATE TABLE sqlite_schema (