From 1999eac8912183a51c09eac6b1db8f033f3d99c8 Mon Sep 17 00:00:00 2001 From: Zaid Humayun Date: Thu, 22 May 2025 23:53:48 +0530 Subject: [PATCH] Fixes test Testing: can drop kv_store vtable Earlier this test broke because the code to translate the drop table was not checking to see if a table was not a virtual table. SQLite does this with a macro called IsVirtualTable. Here, I check with the existing methods on the BTree struct --- core/translate/schema.rs | 340 ++++++++++++++++++++------------------- 1 file changed, 171 insertions(+), 169 deletions(-) diff --git a/core/translate/schema.rs b/core/translate/schema.rs index 98c0cd70f..b66405ae7 100644 --- a/core/translate/schema.rs +++ b/core/translate/schema.rs @@ -767,183 +767,185 @@ pub fn translate_drop_table( let r7 = program.alloc_register(); program.emit_null(r6, Some(r7)); - // 3. Open an ephemeral table, and read over entries from the schema table for entries of the table whose root page was moved in the destroy operation + if table.btree().is_some() { + // 4. Open an ephemeral table, and read over entries from the schema table for entries of the table whose root page was moved in the destroy operation - // cursor id 1 - let sqlite_schema_cursor_id_1 = program.alloc_cursor_id( - Some(SQLITE_TABLEID.to_owned()), - CursorType::BTreeTable(schema_table.clone()), - ); - let simple_table_rc = Rc::new(BTreeTable { - root_page: 0, // Not relevant for ephemeral table definition - name: "ephemeral_scratch".to_string(), - has_rowid: true, - primary_key_columns: vec![], - columns: vec![Column { - name: Some("rowid".to_string()), - ty: Type::Integer, - ty_str: "INTEGER".to_string(), - primary_key: false, - is_rowid_alias: false, - notnull: false, - default: None, - unique: false, - collation: None, - }], - is_strict: false, - unique_sets: None, - }); - // cursor id 2 - let ephemeral_cursor_id = program.alloc_cursor_id( - Some("scratch_table".to_string()), - CursorType::BTreeTable(simple_table_rc), - ); - program.emit_insn(Insn::OpenEphemeral { - cursor_id: ephemeral_cursor_id, - is_table: true, - }); - let if_not_label = program.allocate_label(); - program.emit_insn(Insn::IfNot { - reg: r2_reg, - target_pc: if_not_label, - jump_if_null: true, // jump anyway - }); - program.emit_insn(Insn::OpenRead { - cursor_id: sqlite_schema_cursor_id_1, - root_page: 1usize.into(), - }); + // cursor id 1 + let sqlite_schema_cursor_id_1 = program.alloc_cursor_id( + Some(SQLITE_TABLEID.to_owned()), + CursorType::BTreeTable(schema_table.clone()), + ); + let simple_table_rc = Rc::new(BTreeTable { + root_page: 0, // Not relevant for ephemeral table definition + name: "ephemeral_scratch".to_string(), + has_rowid: true, + primary_key_columns: vec![], + columns: vec![Column { + name: Some("rowid".to_string()), + ty: Type::Integer, + ty_str: "INTEGER".to_string(), + primary_key: false, + is_rowid_alias: false, + notnull: false, + default: None, + unique: false, + collation: None, + }], + is_strict: false, + unique_sets: None, + }); + // cursor id 2 + let ephemeral_cursor_id = program.alloc_cursor_id( + Some("scratch_table".to_string()), + CursorType::BTreeTable(simple_table_rc), + ); + program.emit_insn(Insn::OpenEphemeral { + cursor_id: ephemeral_cursor_id, + is_table: true, + }); + let if_not_label = program.allocate_label(); + program.emit_insn(Insn::IfNot { + reg: r2_reg, + target_pc: if_not_label, + jump_if_null: true, // jump anyway + }); + program.emit_insn(Insn::OpenRead { + cursor_id: sqlite_schema_cursor_id_1, + root_page: 1usize.into(), + }); - let r8 = program.alloc_register(); - let r9 = program.alloc_register(); - let r10 = program.alloc_register(); - let r11 = program.alloc_register(); - let r12 = program.alloc_register(); - let r13 = program.alloc_register(); - let _r14 = program.alloc_register(); // Unsure why this register is allocated but putting it in here to make comparison with SQLite easier - let r15 = program.alloc_register(); + let r8 = program.alloc_register(); + let r9 = program.alloc_register(); + let r10 = program.alloc_register(); + let r11 = program.alloc_register(); + let r12 = program.alloc_register(); + let r13 = program.alloc_register(); + let _r14 = program.alloc_register(); // Unsure why this register is allocated but putting it in here to make comparison with SQLite easier + let r15 = program.alloc_register(); - // Loop to copy over row id's from the schema table for rows that have the same root page as the one that was moved - let end_metadata_label = program.allocate_label(); - let metadata_loop = program.allocate_label(); - program.emit_insn(Insn::Rewind { - cursor_id: sqlite_schema_cursor_id_1, - pc_if_empty: end_metadata_label, - }); - program.preassign_label_to_next_insn(metadata_loop); - // start loop on schema table - program.emit_insn(Insn::Column { - cursor_id: sqlite_schema_cursor_id_1, - column: 3, - dest: r13, - }); - let next_label = program.allocate_label(); - program.emit_insn(Insn::Ne { - lhs: r13, - rhs: r2_reg, - target_pc: next_label, - flags: CmpInsFlags::default(), - collation: program.curr_collation(), - }); - program.emit_insn(Insn::RowId { - cursor_id: sqlite_schema_cursor_id_1, - dest: r7, - }); - program.emit_insn(Insn::Insert { - cursor: ephemeral_cursor_id, - key_reg: r7, - record_reg: r6, - flag: 0, - table_name: "scratch_table".to_string(), - }); + // Loop to copy over row id's from the schema table for rows that have the same root page as the one that was moved + let end_metadata_label = program.allocate_label(); + let metadata_loop = program.allocate_label(); + program.emit_insn(Insn::Rewind { + cursor_id: sqlite_schema_cursor_id_1, + pc_if_empty: end_metadata_label, + }); + program.preassign_label_to_next_insn(metadata_loop); + // start loop on schema table + program.emit_insn(Insn::Column { + cursor_id: sqlite_schema_cursor_id_1, + column: 3, + dest: r13, + }); + let next_label = program.allocate_label(); + program.emit_insn(Insn::Ne { + lhs: r13, + rhs: r2_reg, + target_pc: next_label, + flags: CmpInsFlags::default(), + collation: program.curr_collation(), + }); + program.emit_insn(Insn::RowId { + cursor_id: sqlite_schema_cursor_id_1, + dest: r7, + }); + program.emit_insn(Insn::Insert { + cursor: ephemeral_cursor_id, + key_reg: r7, + record_reg: r6, + flag: 0, + table_name: "scratch_table".to_string(), + }); - program.resolve_label(next_label, program.offset()); - program.emit_insn(Insn::Next { - cursor_id: sqlite_schema_cursor_id_1, - pc_if_next: metadata_loop, - }); - program.preassign_label_to_next_insn(end_metadata_label); - // End loop to copy over row id's from the schema table for rows that have the same root page as the one that was moved + program.resolve_label(next_label, program.offset()); + program.emit_insn(Insn::Next { + cursor_id: sqlite_schema_cursor_id_1, + pc_if_next: metadata_loop, + }); + program.preassign_label_to_next_insn(end_metadata_label); + // End loop to copy over row id's from the schema table for rows that have the same root page as the one that was moved - program.resolve_label(if_not_label, program.offset()); + program.resolve_label(if_not_label, program.offset()); - // 4. Open a write cursor to the schema table and re-insert the records placed in the ephemeral table but insert the correct root page now - program.emit_insn(Insn::OpenWrite { - cursor_id: sqlite_schema_cursor_id_1, - root_page: 1usize.into(), - name: SQLITE_TABLEID.to_string(), - }); + // 5. Open a write cursor to the schema table and re-insert the records placed in the ephemeral table but insert the correct root page now + program.emit_insn(Insn::OpenWrite { + cursor_id: sqlite_schema_cursor_id_1, + root_page: 1usize.into(), + name: SQLITE_TABLEID.to_string(), + }); - // Loop to copy over row id's from the ephemeral table and then re-insert into the schema table with the correct root page - let end_metadata_label = program.allocate_label(); - let metadata_loop = program.allocate_label(); - program.emit_insn(Insn::Rewind { - cursor_id: ephemeral_cursor_id, - pc_if_empty: end_metadata_label, - }); - program.preassign_label_to_next_insn(metadata_loop); - // start loop on schema table - program.emit_insn(Insn::RowId { - cursor_id: ephemeral_cursor_id, - dest: r7, - }); - let next_label = program.allocate_label(); - program.emit_insn(Insn::NotExists { - cursor: sqlite_schema_cursor_id_1, - rowid_reg: r7, - target_pc: next_label, - }); - program.emit_insn(Insn::Column { - cursor_id: sqlite_schema_cursor_id_1, - column: 0, - dest: r8, - }); - program.emit_insn(Insn::Column { - cursor_id: sqlite_schema_cursor_id_1, - column: 1, - dest: r9, - }); - program.emit_insn(Insn::Column { - cursor_id: sqlite_schema_cursor_id_1, - column: 2, - dest: r10, - }); - let root_page = table - .get_root_page() - .try_into() - .expect("Failed to cast the root page to an i64"); - program.emit_insn(Insn::Integer { - value: root_page, - dest: r11, - }); - program.emit_insn(Insn::Column { - cursor_id: sqlite_schema_cursor_id_1, - column: 4, - dest: r12, - }); - program.emit_insn(Insn::MakeRecord { - start_reg: r8, - count: 5, - dest_reg: r15, - index_name: None, - }); - program.emit_insn(Insn::Delete { - cursor_id: sqlite_schema_cursor_id_1, - }); - program.emit_insn(Insn::Insert { - cursor: sqlite_schema_cursor_id_1, - key_reg: r7, - record_reg: r15, - flag: 0, - table_name: SQLITE_TABLEID.to_string(), - }); + // Loop to copy over row id's from the ephemeral table and then re-insert into the schema table with the correct root page + let end_metadata_label = program.allocate_label(); + let metadata_loop = program.allocate_label(); + program.emit_insn(Insn::Rewind { + cursor_id: ephemeral_cursor_id, + pc_if_empty: end_metadata_label, + }); + program.preassign_label_to_next_insn(metadata_loop); + // start loop on schema table + program.emit_insn(Insn::RowId { + cursor_id: ephemeral_cursor_id, + dest: r7, + }); + let next_label = program.allocate_label(); + program.emit_insn(Insn::NotExists { + cursor: sqlite_schema_cursor_id_1, + rowid_reg: r7, + target_pc: next_label, + }); + program.emit_insn(Insn::Column { + cursor_id: sqlite_schema_cursor_id_1, + column: 0, + dest: r8, + }); + program.emit_insn(Insn::Column { + cursor_id: sqlite_schema_cursor_id_1, + column: 1, + dest: r9, + }); + program.emit_insn(Insn::Column { + cursor_id: sqlite_schema_cursor_id_1, + column: 2, + dest: r10, + }); + let root_page = table + .get_root_page() + .try_into() + .expect("Failed to cast the root page to an i64"); + program.emit_insn(Insn::Integer { + value: root_page, + dest: r11, + }); + program.emit_insn(Insn::Column { + cursor_id: sqlite_schema_cursor_id_1, + column: 4, + dest: r12, + }); + program.emit_insn(Insn::MakeRecord { + start_reg: r8, + count: 5, + dest_reg: r15, + index_name: None, + }); + program.emit_insn(Insn::Delete { + cursor_id: sqlite_schema_cursor_id_1, + }); + program.emit_insn(Insn::Insert { + cursor: sqlite_schema_cursor_id_1, + key_reg: r7, + record_reg: r15, + flag: 0, + table_name: SQLITE_TABLEID.to_string(), + }); - program.resolve_label(next_label, program.offset()); - program.emit_insn(Insn::Next { - cursor_id: ephemeral_cursor_id, - pc_if_next: metadata_loop, - }); - program.preassign_label_to_next_insn(end_metadata_label); - // End loop to copy over row id's from the ephemeral table and then re-insert into the schema table with the correct root page + program.resolve_label(next_label, program.offset()); + program.emit_insn(Insn::Next { + cursor_id: ephemeral_cursor_id, + pc_if_next: metadata_loop, + }); + program.preassign_label_to_next_insn(end_metadata_label); + // End loop to copy over row id's from the ephemeral table and then re-insert into the schema table with the correct root page + } // Drop the in-memory structures for the table program.emit_insn(Insn::DropTable {