From cab06250179573daa81cdb407957c3c7f9c69f2b Mon Sep 17 00:00:00 2001 From: krishvishal Date: Thu, 30 Jan 2025 15:04:50 +0530 Subject: [PATCH 1/3] Fixes limbo creating an extra btree, when table has single column PRIMARy KEY. The error is due to comparing the PRIMARY KEY's name to INTEGER when in it was all in lowercase. --- core/translate/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/translate/mod.rs b/core/translate/mod.rs index 75c1f9758..8f0c5f533 100644 --- a/core/translate/mod.rs +++ b/core/translate/mod.rs @@ -258,7 +258,7 @@ fn emit_schema_entry( /// /// An automatic PRIMARY KEY index is not required if: /// - The table has no PRIMARY KEY -/// - The table has a single-column PRIMARY KEY whose typename is _exactly_ "INTEGER" e.g. not "INT". +/// - The table has a single-column PRIMARY KEY whose typename is _exactly_ "integer" e.g. not "INT". /// In this case, the PRIMARY KEY column becomes an alias for the rowid. /// /// Otherwise, an automatic PRIMARY KEY index is required. @@ -348,7 +348,7 @@ fn check_automatic_pk_index_required( let needs_auto_index = if let Some(primary_key_definition) = &primary_key_definition { match primary_key_definition { PrimaryKeyDefinitionType::Simple { typename } => { - let is_integer = typename.is_some() && typename.unwrap() == "INTEGER"; + let is_integer = typename.is_some() && typename.unwrap() == "integer"; !is_integer } PrimaryKeyDefinitionType::Composite => true, From 6f32344efb20c9e9aa85f68ecc6aa5fad2a39448 Mon Sep 17 00:00:00 2001 From: krishvishal Date: Thu, 30 Jan 2025 17:05:14 +0530 Subject: [PATCH 2/3] Make comparison of `type_name` case insensitive by converting to uppercase --- core/translate/mod.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/translate/mod.rs b/core/translate/mod.rs index 8f0c5f533..18ecbb62d 100644 --- a/core/translate/mod.rs +++ b/core/translate/mod.rs @@ -258,7 +258,7 @@ fn emit_schema_entry( /// /// An automatic PRIMARY KEY index is not required if: /// - The table has no PRIMARY KEY -/// - The table has a single-column PRIMARY KEY whose typename is _exactly_ "integer" e.g. not "INT". +/// - The table has a single-column PRIMARY KEY whose typename is _exactly_ "INTEGER" e.g. not "INT". /// In this case, the PRIMARY KEY column becomes an alias for the rowid. /// /// Otherwise, an automatic PRIMARY KEY index is required. @@ -348,7 +348,8 @@ fn check_automatic_pk_index_required( let needs_auto_index = if let Some(primary_key_definition) = &primary_key_definition { match primary_key_definition { PrimaryKeyDefinitionType::Simple { typename } => { - let is_integer = typename.is_some() && typename.unwrap() == "integer"; + let is_integer = + typename.is_some() && typename.unwrap().to_uppercase() == "INTEGER"; !is_integer } PrimaryKeyDefinitionType::Composite => true, From 8b2393fcef479b707780d6dafee4aace64e9caa5 Mon Sep 17 00:00:00 2001 From: krishvishal Date: Fri, 31 Jan 2025 08:25:54 +0530 Subject: [PATCH 3/3] Check for if a column is in descending order to add an automatic primary key index. --- core/translate/mod.rs | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/core/translate/mod.rs b/core/translate/mod.rs index 18ecbb62d..aac22515c 100644 --- a/core/translate/mod.rs +++ b/core/translate/mod.rs @@ -253,6 +253,11 @@ fn emit_schema_entry( }); } +struct PrimaryKeyColumnInfo<'a> { + name: &'a String, + is_descending: bool, +} + /// Check if an automatic PRIMARY KEY index is required for the table. /// If so, create a register for the index root page and return it. /// @@ -282,10 +287,13 @@ fn check_automatic_pk_index_required( columns: pk_cols, .. } = &constraint.constraint { - let primary_key_column_results: Vec> = pk_cols + let primary_key_column_results: Vec> = pk_cols .iter() .map(|col| match &col.expr { - ast::Expr::Id(name) => Ok(&name.0), + ast::Expr::Id(name) => Ok(PrimaryKeyColumnInfo { + name: &name.0, + is_descending: matches!(col.order, Some(ast::SortOrder::Desc)), + }), _ => Err(LimboError::ParseError( "expressions prohibited in PRIMARY KEY and UNIQUE constraints" .to_string(), @@ -297,7 +305,9 @@ fn check_automatic_pk_index_required( if let Err(e) = result { bail_parse_error!("{}", e); } - let column_name = result?; + let pk_info = result?; + + let column_name = pk_info.name; let column_def = columns.get(&ast::Name(column_name.clone())); if column_def.is_none() { bail_parse_error!("No such column: {}", column_name); @@ -314,8 +324,11 @@ fn check_automatic_pk_index_required( let column_def = column_def.unwrap(); let typename = column_def.col_type.as_ref().map(|t| t.name.as_str()); - primary_key_definition = - Some(PrimaryKeyDefinitionType::Simple { typename }); + let is_descending = pk_info.is_descending; + primary_key_definition = Some(PrimaryKeyDefinitionType::Simple { + typename, + is_descending, + }); } } } @@ -333,8 +346,10 @@ fn check_automatic_pk_index_required( bail_parse_error!("table {} has more than one primary key", tbl_name); } let typename = col_def.col_type.as_ref().map(|t| t.name.as_str()); - primary_key_definition = - Some(PrimaryKeyDefinitionType::Simple { typename }); + primary_key_definition = Some(PrimaryKeyDefinitionType::Simple { + typename, + is_descending: false, + }); } } } @@ -347,10 +362,13 @@ fn check_automatic_pk_index_required( // Check if we need an automatic index let needs_auto_index = if let Some(primary_key_definition) = &primary_key_definition { match primary_key_definition { - PrimaryKeyDefinitionType::Simple { typename } => { + PrimaryKeyDefinitionType::Simple { + typename, + is_descending, + } => { let is_integer = typename.is_some() && typename.unwrap().to_uppercase() == "INTEGER"; - !is_integer + !is_integer || *is_descending } PrimaryKeyDefinitionType::Composite => true, } @@ -521,7 +539,10 @@ fn translate_create_table( } enum PrimaryKeyDefinitionType<'a> { - Simple { typename: Option<&'a str> }, + Simple { + typename: Option<&'a str>, + is_descending: bool, + }, Composite, }