mirror of
https://github.com/aljazceru/turso.git
synced 2026-01-03 00:14:21 +01:00
Merge 'translate: couple fixes from testing with Gorm' from Preston Thorpe
Ongoing tests for [turso-go](https://github.com/tursodatabase/turso-go) have unearthed a couple more issues closes #3187 ### Number 1: We were getting something like: ```sql sqlite_autoindex_`databases`_2 ``` when creating autoindex for table in Gorm (gorm is notorious for backticks everywhere), because of not normalizing the column name when creating autoindex. ### Number 2: When creating table with `PRIMARY KEY AUTOINCREMENT`, we were still creating the index, but it wasn't properly handled in `populate_indices`, because we are doing the following: ```rust if column.primary_key && unique_set.is_primary_key { if column.is_rowid_alias { // rowid alias, no index needed continue; // continues, but doesn't consume it.. } ``` So if we created such an index entry for the AUTOINCREMENT... we would trip this: ```rust assert!(automatic_indexes.is_empty(), "all automatic indexes parsed from sqlite_schema should have been consumed, but {} remain", automatic_indexes.len()); ``` Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com> Closes #3186
This commit is contained in:
@@ -1,4 +1,3 @@
|
||||
use std::ops::Range;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::ast;
|
||||
@@ -105,16 +104,15 @@ pub fn translate_create_table(
|
||||
// https://github.com/sqlite/sqlite/blob/95f6df5b8d55e67d1e34d2bff217305a2f21b1fb/src/build.c#L2856-L2871
|
||||
// https://github.com/sqlite/sqlite/blob/95f6df5b8d55e67d1e34d2bff217305a2f21b1fb/src/build.c#L1334C5-L1336C65
|
||||
|
||||
let index_regs =
|
||||
check_automatic_pk_index_required(&body, &mut program, tbl_name.name.as_str())?;
|
||||
let index_regs = collect_autoindexes(&body, &mut program, &normalized_tbl_name)?;
|
||||
if let Some(index_regs) = index_regs.as_ref() {
|
||||
if !schema.indexes_enabled() {
|
||||
bail_parse_error!("Constraints UNIQUE and PRIMARY KEY (unless INTEGER PRIMARY KEY) on table are not supported without indexes");
|
||||
}
|
||||
for index_reg in index_regs.clone() {
|
||||
for index_reg in index_regs.iter() {
|
||||
program.emit_insn(Insn::CreateBtree {
|
||||
db: 0,
|
||||
root: index_reg,
|
||||
root: *index_reg,
|
||||
flags: CreateBTreeFlags::new_index(),
|
||||
});
|
||||
}
|
||||
@@ -147,9 +145,8 @@ pub fn translate_create_table(
|
||||
if let Some(index_regs) = index_regs {
|
||||
for (idx, index_reg) in index_regs.into_iter().enumerate() {
|
||||
let index_name = format!(
|
||||
"{}{}_{}",
|
||||
PRIMARY_KEY_AUTOMATIC_INDEX_NAME_PREFIX,
|
||||
tbl_name.name.as_str(),
|
||||
"{PRIMARY_KEY_AUTOMATIC_INDEX_NAME_PREFIX}{}_{}",
|
||||
normalized_tbl_name,
|
||||
idx + 1
|
||||
);
|
||||
emit_schema_entry(
|
||||
@@ -296,17 +293,42 @@ pub fn emit_schema_entry(
|
||||
/// In this case, the PRIMARY KEY column becomes an alias for the rowid.
|
||||
///
|
||||
/// Otherwise, an automatic PRIMARY KEY index is required.
|
||||
fn check_automatic_pk_index_required(
|
||||
fn collect_autoindexes(
|
||||
body: &ast::CreateTableBody,
|
||||
program: &mut ProgramBuilder,
|
||||
tbl_name: &str,
|
||||
) -> Result<Option<Range<usize>>> {
|
||||
let table = create_table(tbl_name, body, 0)?; // abusing create_table() to avoid code duplication; we don't care about the root page here
|
||||
if table.unique_sets.is_empty() {
|
||||
return Ok(None);
|
||||
) -> Result<Option<Vec<usize>>> {
|
||||
let table = create_table(tbl_name, body, 0)?;
|
||||
|
||||
let mut regs: Vec<usize> = Vec::new();
|
||||
|
||||
// include UNIQUE singles, include PK single only if not rowid alias
|
||||
for us in table.unique_sets.iter().filter(|us| us.columns.len() == 1) {
|
||||
let (col_name, _sort) = us.columns.first().unwrap();
|
||||
let Some((_pos, col)) = table.get_column(col_name) else {
|
||||
bail_parse_error!("Column {col_name} not found in table {}", table.name);
|
||||
};
|
||||
|
||||
let needs_index = if us.is_primary_key {
|
||||
!(col.primary_key && col.is_rowid_alias)
|
||||
} else {
|
||||
// UNIQUE single needs an index
|
||||
true
|
||||
};
|
||||
|
||||
if needs_index {
|
||||
regs.push(program.alloc_register());
|
||||
}
|
||||
}
|
||||
|
||||
for _us in table.unique_sets.iter().filter(|us| us.columns.len() > 1) {
|
||||
regs.push(program.alloc_register());
|
||||
}
|
||||
if regs.is_empty() {
|
||||
Ok(None)
|
||||
} else {
|
||||
Ok(Some(regs))
|
||||
}
|
||||
let start_reg = program.alloc_registers(table.unique_sets.len());
|
||||
Ok(Some(start_reg..start_reg + table.unique_sets.len()))
|
||||
}
|
||||
|
||||
fn create_table_body_to_str(tbl_name: &ast::QualifiedName, body: &ast::CreateTableBody) -> String {
|
||||
|
||||
@@ -556,7 +556,7 @@ fn rewrite_upsert_expr_in_place(
|
||||
}
|
||||
// Unqualified column id -> CURRENT
|
||||
Expr::Id(Name::Ident(name)) | Expr::Id(Name::Quoted(name)) => {
|
||||
if let Some(reg) = col_reg(name.as_str()) {
|
||||
if let Some(reg) = col_reg(&normalize_ident(name.as_str())) {
|
||||
*expr = Expr::Register(reg);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1068,7 +1068,9 @@ pub fn parse_pragma_bool(expr: &Expr) -> Result<bool> {
|
||||
pub fn extract_column_name_from_expr(expr: impl AsRef<ast::Expr>) -> Option<String> {
|
||||
match expr.as_ref() {
|
||||
ast::Expr::Id(name) => Some(name.as_str().to_string()),
|
||||
ast::Expr::Qualified(_, name) => Some(name.as_str().to_string()),
|
||||
ast::Expr::DoublyQualified(_, _, name) | ast::Expr::Qualified(_, name) => {
|
||||
Some(normalize_ident(name.as_str()))
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -15,6 +15,11 @@ do_execsql_test_on_specific_db {:memory:} create_table_unique_contained_in_prima
|
||||
CREATE TABLE t4 (a,b, primary key(a,b), unique(a));
|
||||
} {}
|
||||
|
||||
# https://github.com/tursodatabase/turso/issues/3187
|
||||
do_execsql_test_on_specific_db {:memory:} create_table_unique_constraint_and_autoinc_backticks {
|
||||
CREATE TABLE `databases` (`id` integer PRIMARY KEY AUTOINCREMENT,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime,`hostname` text NOT NULL,`namespace` text,`address` text,`primary_address` text,`local` numeric,`allowed_ips` text, CONSTRAINT `uni_databases_hostname` UNIQUE (`hostname`));
|
||||
} {}
|
||||
|
||||
# https://github.com/tursodatabase/turso/issues/2686
|
||||
do_execsql_test_on_specific_db {:memory:} create_table_rowid_unique_regression_test {
|
||||
create table u(x integer unique primary key);
|
||||
|
||||
Reference in New Issue
Block a user