wip sql: clean up schema [rebase before Local engine cleanup]

src/sql/engine/local.rs

@@ -140,6 +140,8 @@ impl<E: storage::Engine> super::Transaction for Transaction<E> {
  }
 
  fn delete(&self, table: &str, ids: &[Value]) -> Result<()> {
+ // Check for foreign key referenes.
+
  // TODO: try to be more clever than simply iterating over each ID.
  for id in ids {
  let table = self.must_get_table(table)?;
@@ -153,7 +155,7 @@ impl<E: storage::Engine> super::Transaction for Transaction<E> {
  while let Some(row) = scan.next().transpose()? {
  for (i, c) in &cs {
  if &row[*i] == id
- && (table.name != t.name || id != &table.get_row_key(&row)?)
+ && (table.name != t.name || id != table.get_row_key(&row)?)
  {
  return errinput!(
  "primary key {id} referenced by table {} column {c}",
@@ -199,7 +201,7 @@ impl<E: storage::Engine> super::Transaction for Transaction<E> {
  if !self.get(&table.name, &[id.clone()])?.is_empty() {
  return errinput!("primary key {id} already exists for table {}", table.name);
  }
- self.txn.set(&Key::Row((&table.name).into(), (&id).into()).encode(), row.encode())?;
+ self.txn.set(&Key::Row((&table.name).into(), id.into()).encode(), row.encode())?;
 
  // Update indexes
  for (i, column) in table.columns.iter().enumerate().filter(|(_, c)| c.index) {
@@ -276,7 +278,7 @@ impl<E: storage::Engine> super::Transaction for Transaction<E> {
  // TODO: be more clever than just iterating here.
  for (id, row) in rows {
  // If the primary key changes we do a delete and create, otherwise we replace the row
- if id != table.get_row_key(&row)? {
+ if id != *table.get_row_key(&row)? {
  self.delete(&table.name, &[id.clone()])?;
  self.insert(&table.name, vec![row])?;
  return Ok(());
@@ -331,7 +333,7 @@ impl<E: storage::Engine> Catalog for Transaction<E> {
  }
  let mut scan = self.scan(&table.name, None)?;
  while let Some(row) = scan.next().transpose()? {
- self.delete(&table.name, &[table.get_row_key(&row)?])?
+ self.delete(&table.name, &[table.get_row_key(&row)?.clone()])?
  }
  self.txn.delete(&Key::Table(table.name.into()).encode())?;
  Ok(true)

src/sql/parser/lexer.rs

@@ -484,3 +484,13 @@ impl<'a> Lexer<'a> {
  while self.next_if(|c| c.is_whitespace()).is_some() {}
  }
 }
+
+/// Returns true if the entire given string is a single valid identifier.
+pub fn is_ident(ident: &str) -> bool {
+ let mut lexer = Lexer::new(ident);
+ let token = lexer.next();
+ if lexer.next().is_some() {
+ return false; // multiple tokens, so not an identifier
+ }
+ matches!(token, Some(Ok(Token::Ident(_))))
+}

src/sql/parser/mod.rs

@@ -2,20 +2,5 @@ pub mod ast;
 mod lexer;
 mod parser;
 
-pub use lexer::{Keyword, Lexer, Token};
+pub use lexer::{is_ident, Keyword, Lexer, Token};
 pub use parser::Parser;
-
-use regex::Regex;
-
-// Formats an identifier by quoting it as appropriate.
-// TODO: move this elsewhere.
-pub(super) fn format_ident(ident: &str) -> String {
- static RE_IDENT: std::sync::OnceLock<Regex> = std::sync::OnceLock::new();
- let re_ident = RE_IDENT.get_or_init(|| Regex::new(r#"^\w[\w_]*$"#).unwrap());
-
- if re_ident.is_match(ident) && Keyword::try_from(ident.to_lowercase().as_str()).is_err() {
- ident.to_string()
- } else {
- format!("\"{}\"", ident.replace('\"', "\"\""))
- }
-}

src/sql/plan/optimizer.rs

@@ -174,14 +174,12 @@ impl Optimizer for IndexLookup {
  fn optimize(&self, node: Node) -> Result<Node> {
  node.transform(&Ok, &|n| match n {
  Node::Scan { table, alias, filter: Some(filter) } => {
- let pk = table.columns.iter().position(|c| c.primary_key).unwrap();
-
  // Convert the filter into conjunctive normal form, and try to convert each
  // sub-expression into a lookup. If a lookup is found, return a lookup node and then
  // apply the remaining conjunctions as a filter node, if any.
  let mut cnf = filter.clone().into_cnf_vec();
  for i in 0..cnf.len() {
- if let Some(keys) = cnf[i].as_lookup(pk) {
+ if let Some(keys) = cnf[i].as_lookup(table.primary_key) {
  cnf.remove(i);
  return Ok(self.wrap_cnf(Node::KeyLookup { table, alias, keys }, cnf));
  }

src/sql/plan/planner.rs

@@ -36,32 +36,35 @@ impl<'a, C: Catalog> Planner<'a, C> {
  ast::Statement::Explain(_) => panic!("unexpected explain statement"),
 
  // DDL statements (schema changes).
- ast::Statement::CreateTable { name, columns } => Plan::CreateTable {
- schema: Table::new(
- name,
- columns
- .into_iter()
- .map(|c| {
- let nullable = c.nullable.unwrap_or(!c.primary_key);
- let default = match c.default {
+ ast::Statement::CreateTable { name, columns } => {
+ let Some(primary_key) = columns.iter().position(|c| c.primary_key) else {
+ return errinput!("no primary key for table {name}");
+ };
+ if columns.iter().filter(|c| c.primary_key).count() > 1 {
+ return errinput!("multiple primary keys for table {name}");
+ }
+ let columns = columns
+ .into_iter()
+ .map(|c| {
+ let nullable = c.nullable.unwrap_or(!c.primary_key);
+ Ok(Column {
+ name: c.name,
+ datatype: c.datatype,
+ nullable,
+ default: match c.default {
  Some(expr) => Some(self.evaluate_constant(expr)?),
  None if nullable => Some(Value::Null),
  None => None,
- };
- Ok(Column {
- name: c.name,
- datatype: c.datatype,
- primary_key: c.primary_key,
- nullable,
- default,
- index: c.index && !c.primary_key,
- unique: c.unique || c.primary_key,
- references: c.references,
- })
+ },
+ unique: c.unique || c.primary_key,
+ index: (c.index || c.unique || c.references.is_some())
+ && !c.primary_key,
+ references: c.references,
  })
- .collect::<Result<_>>()?,
- )?,
- },
+ })
+ .collect::<Result<_>>()?;
+ Plan::CreateTable { schema: Table { name, primary_key, columns } }
+ }
 
  ast::Statement::DropTable { name, if_exists } => {
  Plan::DropTable { table: name, if_exists }
@@ -73,7 +76,7 @@ impl<'a, C: Catalog> Planner<'a, C> {
  let scope = &mut Scope::from_table(table.clone())?;
  Plan::Delete {
  table: table.name.clone(),
- key_index: table.get_row_key_index()?,
+ key_index: table.primary_key,
  source: Node::Scan {
  table,
  alias: None,
@@ -101,11 +104,10 @@ impl<'a, C: Catalog> Planner<'a, C> {
 
  ast::Statement::Update { table, set, r#where } => {
  let table = self.catalog.must_get_table(&table)?;
- let key_index = table.get_row_key_index()?;
  let scope = &mut Scope::from_table(table.clone())?;
  Plan::Update {
  table: table.name.clone(),
- key_index,
+ key_index: table.primary_key,
  source: Node::Scan {
  table,
  alias: None,