Skip to content

Commit

Permalink
Merge 'Auto-create index in CREATE TABLE when necessary' from Jussi S…
Browse files Browse the repository at this point in the history
…aurio

Closes #448
Adds support for:
- Automatically creating index on the PRIMARY KEY if the pk is not a
rowid alias
- Parsing the automatically created index into memory, for use in
queries
    * `testing/testing_norowidalias.db` now uses the PK indexes and some
tests were failing -- looks like taking the index into use revealed some
bugs in our codegen :) I fixed those in later commits.
Does not add support for:
- Inserting to the index during writes to the table

Closes #588
  • Loading branch information
penberg committed Jan 4, 2025
2 parents fc60e54 + 1b61749 commit b18abba
Show file tree
Hide file tree
Showing 6 changed files with 438 additions and 63 deletions.
113 changes: 113 additions & 0 deletions core/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -508,11 +508,50 @@ impl Index {
_ => todo!("Expected create index statement"),
}
}

pub fn automatic_from_primary_key(
table: &BTreeTable,
index_name: &str,
root_page: usize,
) -> Result<Index> {
if table.primary_key_column_names.is_empty() {
return Err(crate::LimboError::InternalError(
"Cannot create automatic index for table without primary key".to_string(),
));
}

let index_columns = table
.primary_key_column_names
.iter()
.map(|col_name| {
// Verify that each primary key column exists in the table
if table.get_column(col_name).is_none() {
return Err(crate::LimboError::InternalError(format!(
"Primary key column {} not found in table {}",
col_name, table.name
)));
}
Ok(IndexColumn {
name: normalize_ident(col_name),
order: Order::Ascending, // Primary key indexes are always ascending
})
})
.collect::<Result<Vec<_>>>()?;

Ok(Index {
name: normalize_ident(index_name),
table_name: table.name.clone(),
root_page,
columns: index_columns,
unique: true, // Primary key indexes are always unique
})
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::LimboError;

#[test]
pub fn test_has_rowid_true() -> Result<()> {
Expand Down Expand Up @@ -735,4 +774,78 @@ mod tests {
let actual = sqlite_schema_table().to_sql();
assert_eq!(expected, actual);
}

#[test]
fn test_automatic_index_single_column() -> Result<()> {
let sql = r#"CREATE TABLE t1 (a INTEGER PRIMARY KEY, b TEXT);"#;
let table = BTreeTable::from_sql(sql, 0)?;
let index = Index::automatic_from_primary_key(&table, "sqlite_autoindex_t1_1", 2)?;

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(), 1);
assert_eq!(index.columns[0].name, "a");
assert!(matches!(index.columns[0].order, Order::Ascending));
Ok(())
}

#[test]
fn test_automatic_index_composite_key() -> Result<()> {
let sql = r#"CREATE TABLE t1 (a INTEGER, b TEXT, PRIMARY KEY(a, b));"#;
let table = BTreeTable::from_sql(sql, 0)?;
let index = Index::automatic_from_primary_key(&table, "sqlite_autoindex_t1_1", 2)?;

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_eq!(index.columns[1].name, "b");
assert!(matches!(index.columns[0].order, Order::Ascending));
assert!(matches!(index.columns[1].order, Order::Ascending));
Ok(())
}

#[test]
fn test_automatic_index_no_primary_key() -> Result<()> {
let sql = r#"CREATE TABLE t1 (a INTEGER, b TEXT);"#;
let table = BTreeTable::from_sql(sql, 0)?;
let result = Index::automatic_from_primary_key(&table, "sqlite_autoindex_t1_1", 2);

assert!(result.is_err());
assert!(matches!(
result.unwrap_err(),
LimboError::InternalError(msg) if msg.contains("without primary key")
));
Ok(())
}

#[test]
fn test_automatic_index_nonexistent_column() -> Result<()> {
// Create a table with a primary key column that doesn't exist in the table
let table = BTreeTable {
root_page: 0,
name: "t1".to_string(),
has_rowid: true,
primary_key_column_names: vec!["nonexistent".to_string()],
columns: vec![Column {
name: "a".to_string(),
ty: Type::Integer,
primary_key: false,
is_rowid_alias: false,
}],
};

let result = Index::automatic_from_primary_key(&table, "sqlite_autoindex_t1_1", 2);

assert!(result.is_err());
assert!(matches!(
result.unwrap_err(),
LimboError::InternalError(msg) if msg.contains("not found in table")
));
Ok(())
}
}
2 changes: 1 addition & 1 deletion core/translate/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1360,8 +1360,8 @@ fn close_loop(
iter_dir,
..
} => {
let cursor_id = program.resolve_cursor_id(&table_reference.table_identifier);
program.resolve_label(loop_labels.next, program.offset());
let cursor_id = program.resolve_cursor_id(&table_reference.table_identifier);
if iter_dir
.as_ref()
.is_some_and(|dir| *dir == IterationDirection::Backwards)
Expand Down
Loading

0 comments on commit b18abba

Please sign in to comment.