Skip to content

Commit e2487bc

Browse files
glommerclaude
glommer
and
claude
committed
refactor: replace database_id magic numbers with named constants
SQLite reserves database index 0 for "main" and 1 for "temp"; attached databases start at index 2. The codebase had raw numeric comparisons like `database_id >= 2` and `db: 0` scattered across translate/ and vdbe/, which were unclear without knowing this convention. Add MAIN_DB_ID, TEMP_DB_ID, FIRST_ATTACHED_DB_ID constants and an is_attached_db() helper in core/lib.rs. Replace all magic numbers across 17 files. Remove the duplicate local constants from Resolver in emitter.rs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 8a0ae7f commit e2487bc

File tree

17 files changed

+71
-51
lines changed

17 files changed

+71
-51
lines changed

‎core/connection.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1323,7 +1323,7 @@ impl Connection {
13231323
database_id: usize,
13241324
f: impl FnOnce(&mut Schema) -> T,
13251325
) -> T {
1326-
if database_id < 2 {
1326+
if !crate::is_attached_db(database_id) {
13271327
self.with_schema_mut(f)
13281328
} else {
13291329
// For attached databases, update a connection-local copy of the schema.
@@ -1514,11 +1514,11 @@ impl Connection {
15141514

15151515
/// Access schema for a database using a closure pattern to avoid cloning
15161516
pub(crate) fn with_schema<T>(&self, database_id: usize, f: impl FnOnce(&Schema) -> T) -> T {
1517-
if database_id == 0 {
1517+
if database_id == crate::MAIN_DB_ID {
15181518
// Main database - use connection's schema which should be kept in sync
15191519
let schema = self.schema.read();
15201520
f(&schema)
1521-
} else if database_id == 1 {
1521+
} else if database_id == crate::TEMP_DB_ID {
15221522
// Temp database - uses same schema as main for now, but this will change later.
15231523
let schema = self.schema.read();
15241524
f(&schema)

‎core/lib.rs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,23 @@ pub use vdbe::{
136136
FromValueRow, PrepareContext, PreparedProgram, Program, Register,
137137
};
138138

139+
/// Database index for the main database (always 0 in SQLite).
140+
pub const MAIN_DB_ID: usize = 0;
141+
142+
/// Database index for the temp database (always 1 in SQLite).
143+
pub const TEMP_DB_ID: usize = 1;
144+
145+
/// First database index used for ATTACH-ed databases.
146+
/// SQLite reserves 0 for "main" and 1 for "temp", so attached databases
147+
/// start at index 2.
148+
pub const FIRST_ATTACHED_DB_ID: usize = 2;
149+
150+
/// Returns true if the database index refers to an attached database
151+
/// (i.e. not "main" and not "temp").
152+
pub const fn is_attached_db(database_id: usize) -> bool {
153+
database_id >= FIRST_ATTACHED_DB_ID
154+
}
155+
139156
/// Configuration for database features
140157
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
141158
pub struct DatabaseOpts {

‎core/translate/alter.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ fn emit_add_column_default_type_validation(
209209
program.emit_insn(Insn::OpenRead {
210210
cursor_id: check_cursor_id,
211211
root_page: original_btree.root_page,
212-
db: 0,
212+
db: crate::MAIN_DB_ID,
213213
});
214214

215215
let skip_check_label = program.allocate_label();
@@ -358,7 +358,7 @@ pub fn translate_alter_table(
358358
body: alter_table,
359359
} = alter;
360360
let database_id = resolver.resolve_database_id(&qualified_name)?;
361-
if database_id >= 2 {
361+
if crate::is_attached_db(database_id) {
362362
let schema_cookie = resolver.with_schema(database_id, |s| s.schema_version);
363363
program.begin_write_on_database(database_id, schema_cookie);
364364
}

‎core/translate/delete.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ pub fn translate_delete(
4242
crate::bail_parse_error!("table {} may not be modified", tbl_name);
4343
}
4444

45-
if database_id >= 2 {
45+
if crate::is_attached_db(database_id) {
4646
let schema_cookie = resolver.with_schema(database_id, |s| s.schema_version);
4747
program.begin_write_on_database(database_id, schema_cookie);
4848
}

‎core/translate/emitter.rs‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,6 @@ impl<'a> Resolver<'a> {
132132
const MAIN_DB: &'static str = "main";
133133
const TEMP_DB: &'static str = "temp";
134134

135-
const MAIN_DB_ID: usize = 0;
136-
const TEMP_DB_ID: usize = 1;
137-
138135
pub(crate) fn new(
139136
schema: &'a Schema,
140137
database_schemas: &'a RwLock<HashMap<usize, Arc<Schema>>>,
@@ -200,7 +197,7 @@ impl<'a> Resolver<'a> {
200197

201198
/// Access schema for a database using a closure pattern to avoid cloning
202199
pub(crate) fn with_schema<T>(&self, database_id: usize, f: impl FnOnce(&Schema) -> T) -> T {
203-
if database_id == Self::MAIN_DB_ID || database_id == Self::TEMP_DB_ID {
200+
if database_id == crate::MAIN_DB_ID || database_id == crate::TEMP_DB_ID {
204201
f(self.schema)
205202
} else {
206203
// Attached database: prefer the connection-local copy (which may contain
@@ -262,8 +259,8 @@ impl<'a> Resolver<'a> {
262259
/// Returns "main" for index 0, "temp" for index 1, and the alias for attached databases.
263260
pub(crate) fn get_database_name_by_index(&self, index: usize) -> Option<String> {
264261
match index {
265-
0 => Some(Self::MAIN_DB.to_string()),
266-
1 => Some(Self::TEMP_DB.to_string()),
262+
crate::MAIN_DB_ID => Some(Self::MAIN_DB.to_string()),
263+
crate::TEMP_DB_ID => Some(Self::TEMP_DB.to_string()),
267264
_ => self.attached_databases.read().get_name_by_index(index),
268265
}
269266
}
@@ -4221,7 +4218,7 @@ pub fn prepare_cdc_if_necessary(
42214218
program.emit_insn(Insn::OpenWrite {
42224219
cursor_id,
42234220
root_page: cdc_btree.root_page.into(),
4224-
db: 0, // CDC table always lives in the main database
4221+
db: crate::MAIN_DB_ID, // CDC table always lives in the main database
42254222
});
42264223
Ok(Some((cursor_id, cdc_btree)))
42274224
}

‎core/translate/fkeys.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1485,7 +1485,7 @@ fn fire_fk_cascade_delete(
14851485
ctx: &FkActionContext,
14861486
database_id: usize,
14871487
) -> Result<()> {
1488-
let db_name = if database_id > 0 {
1488+
let db_name = if database_id != crate::MAIN_DB_ID {
14891489
resolver.get_database_name_by_index(database_id)
14901490
} else {
14911491
None
@@ -1518,7 +1518,7 @@ fn fire_fk_set_null(
15181518
ctx: &FkActionContext,
15191519
database_id: usize,
15201520
) -> Result<()> {
1521-
let db_name = if database_id > 0 {
1521+
let db_name = if database_id != crate::MAIN_DB_ID {
15221522
resolver.get_database_name_by_index(database_id)
15231523
} else {
15241524
None
@@ -1544,7 +1544,7 @@ fn fire_fk_set_default(
15441544
ctx: &FkActionContext,
15451545
database_id: usize,
15461546
) -> Result<()> {
1547-
let db_name = if database_id > 0 {
1547+
let db_name = if database_id != crate::MAIN_DB_ID {
15481548
resolver.get_database_name_by_index(database_id)
15491549
} else {
15501550
None
@@ -1570,7 +1570,7 @@ fn fire_fk_cascade_update(
15701570
ctx: &FkActionContext,
15711571
database_id: usize,
15721572
) -> Result<()> {
1573-
let db_name = if database_id > 0 {
1573+
let db_name = if database_id != crate::MAIN_DB_ID {
15741574
resolver.get_database_name_by_index(database_id)
15751575
} else {
15761576
None

‎core/translate/index.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ pub fn translate_create_index(
9898
};
9999
program.extend(&opts);
100100

101-
if database_id >= 2 {
101+
if crate::is_attached_db(database_id) {
102102
let schema_cookie = resolver.with_schema(database_id, |s| s.schema_version);
103103
program.begin_write_on_database(database_id, schema_cookie);
104104
}
@@ -778,7 +778,7 @@ pub fn translate_drop_index(
778778
};
779779
program.extend(&opts);
780780

781-
if database_id >= 2 {
781+
if crate::is_attached_db(database_id) {
782782
let schema_cookie = resolver.with_schema(database_id, |s| s.schema_version);
783783
program.begin_write_on_database(database_id, schema_cookie);
784784
}
@@ -1042,7 +1042,10 @@ pub fn translate_optimize(
10421042
// Emit optimize instructions for each index method index
10431043
for idx in &indexes_to_optimize {
10441044
let cursor_id = program.alloc_cursor_index(None, idx)?;
1045-
program.emit_insn(Insn::IndexMethodOptimize { db: 0, cursor_id });
1045+
program.emit_insn(Insn::IndexMethodOptimize {
1046+
db: crate::MAIN_DB_ID,
1047+
cursor_id,
1048+
});
10461049
}
10471050

10481051
Ok(())

‎core/translate/insert.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ pub fn translate_insert(
300300

301301
let cdc_table = prepare_cdc_if_necessary(program, resolver.schema(), table.get_name())?;
302302

303-
if database_id >= 2 {
303+
if crate::is_attached_db(database_id) {
304304
let schema_cookie = resolver.with_schema(database_id, |s| s.schema_version);
305305
program.begin_write_on_database(database_id, schema_cookie);
306306
}

‎core/translate/main_loop.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ pub fn init_loop(
164164
continue;
165165
}
166166
// Ensure attached databases have a Transaction instruction for read access
167-
if table.database_id >= 2 {
167+
if crate::is_attached_db(table.database_id) {
168168
let schema_cookie = t_ctx
169169
.resolver
170170
.with_schema(table.database_id, |s| s.schema_version);
@@ -1407,7 +1407,7 @@ pub fn open_loop(
14071407
)?;
14081408
}
14091409
program.emit_insn(Insn::IndexMethodQuery {
1410-
db: 0,
1410+
db: crate::MAIN_DB_ID,
14111411
cursor_id: index_cursor_id.expect("IndexMethod requires a index cursor"),
14121412
start_reg,
14131413
count_reg: query.arguments.len() + 1,

‎core/translate/schema.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ pub fn translate_create_table(
182182
connection: &Connection,
183183
) -> Result<()> {
184184
let database_id = resolver.resolve_database_id(&tbl_name)?;
185-
if database_id >= 2 {
185+
if crate::is_attached_db(database_id) {
186186
let schema_cookie = resolver.with_schema(database_id, |s| s.schema_version);
187187
program.begin_write_on_database(database_id, schema_cookie);
188188
}
@@ -707,7 +707,7 @@ pub fn translate_create_virtual_table(
707707
program.emit_insn(Insn::OpenWrite {
708708
cursor_id: sqlite_schema_cursor_id,
709709
root_page: 1i64.into(),
710-
db: 0,
710+
db: crate::MAIN_DB_ID,
711711
});
712712

713713
let cdc_table = prepare_cdc_if_necessary(program, resolver.schema(), SQLITE_TABLEID)?;
@@ -725,7 +725,7 @@ pub fn translate_create_virtual_table(
725725
)?;
726726

727727
program.emit_insn(Insn::SetCookie {
728-
db: 0,
728+
db: crate::MAIN_DB_ID,
729729
cookie: Cookie::SchemaVersion,
730730
value: resolver.schema().schema_version as i32 + 1,
731731
p5: 0,
@@ -777,7 +777,7 @@ pub fn translate_drop_table(
777777
};
778778
program.extend(&opts);
779779

780-
if database_id >= 2 {
780+
if crate::is_attached_db(database_id) {
781781
let schema_cookie = resolver.with_schema(database_id, |s| s.schema_version);
782782
program.begin_write_on_database(database_id, schema_cookie);
783783
}
@@ -1188,7 +1188,7 @@ pub fn translate_drop_table(
11881188
program.emit_insn(Insn::OpenWrite {
11891189
cursor_id: ver_cursor_id,
11901190
root_page: version_table.root_page.into(),
1191-
db: 0,
1191+
db: crate::MAIN_DB_ID,
11921192
});
11931193

11941194
let end_ver_loop_label = program.allocate_label();

0 commit comments

Comments
 (0)