mirror of
https://github.com/aljazceru/turso.git
synced 2025-12-17 08:34:19 +01:00
Merge 'Fix: Drop internal DBSP table when dropping materialized view' from Martin Mauch
# Fix: Clean up DBSP state table when dropping materialized views
## Problem
When dropping a materialized view, the internal DBSP state table (e.g.,
`__turso_internal_dbsp_state_v1_view_name`) and its automatic primary
key index were not being properly cleaned up. This caused two issues:
1. **Persistent schema entries**: The DBSP table and index entries
remained in `sqlite_schema` after dropping the view
2. **In-memory schema inconsistency**: The DBSP table remained in the
in-memory schema's `tables` HashMap, causing "table already exists"
errors when trying to recreate a materialized view with the same name
## Root Cause
The issue had two parts:
1. **Missing sqlite_schema cleanup**: The `translate_drop_view` function
deleted the view entry from `sqlite_schema` but didn't delete the
associated DBSP state table and index entries
2. **Missing in-memory schema cleanup**: The `remove_view` function
removed the materialized view from the in-memory schema but didn't
remove the DBSP state table and its indexes
## Solution
### Changes in `core/translate/view.rs`
- Added a second pass loop in `translate_drop_view` to scan
`sqlite_schema` and delete DBSP table and index entries
- The loop checks for entries matching the DBSP table name pattern
(`__turso_internal_dbsp_state_v{version}_{view_name}`) and the automatic
index name pattern (`sqlite_autoindex___turso_internal_dbsp_state_v{vers
ion}_{view_name}_1`)
- Registers for comparison values are allocated outside the loop for
efficiency
- Column registers are reused across loop iterations
### Changes in `core/schema.rs`
- Updated `remove_view` to also remove the DBSP state table and its
indexes from the in-memory schema's `tables` HashMap and `indexes`
collection
- This ensures consistency between the persistent schema
(`sqlite_schema`) and the in-memory schema
### Tests Added
Added two new test cases in `testing/materialized_views.test`:
1. **`matview-drop-cleans-up-dbsp-table`**: Explicitly verifies that
after dropping a materialized view:
- The view entry is removed from `sqlite_schema`
- The DBSP state table entry is removed from `sqlite_schema`
- The DBSP state index entry is removed from `sqlite_schema`
2. **`matview-recreate-after-drop`**: Verifies that a materialized view
can be successfully recreated after being dropped, which implicitly
tests that all underlying resources (including DBSP tables) are properly
cleaned up
## Testing
- All existing materialized view tests pass
- New tests specifically verify the cleanup behavior
- Manual testing confirms that materialized views can be dropped and
recreated without errors
## Related
This fix ensures that materialized views can be safely dropped and
recreated, resolving issues where the DBSP state table would persist and
cause conflicts.
Reviewed-by: Preston Thorpe <preston@turso.tech>
Closes #3928
This commit is contained in:
@@ -249,6 +249,12 @@ impl Schema {
|
||||
// Remove from tables
|
||||
self.tables.remove(&name);
|
||||
|
||||
// Remove DBSP state table and its indexes from in-memory schema
|
||||
use crate::incremental::compiler::DBSP_CIRCUIT_VERSION;
|
||||
let dbsp_table_name = format!("{DBSP_TABLE_PREFIX}{DBSP_CIRCUIT_VERSION}_{name}");
|
||||
self.tables.remove(&dbsp_table_name);
|
||||
self.remove_indices_for_table(&dbsp_table_name);
|
||||
|
||||
// Remove from materialized view tracking
|
||||
self.materialized_view_names.remove(&name);
|
||||
self.materialized_view_sql.remove(&name);
|
||||
|
||||
@@ -326,7 +326,8 @@ pub fn translate_drop_view(
|
||||
}
|
||||
|
||||
// If this is a materialized view, we need to destroy its btree as well
|
||||
if is_materialized_view {
|
||||
// and also clean up the associated DBSP state table and index
|
||||
let dbsp_table_name = if is_materialized_view {
|
||||
if let Some(table) = schema.get_table(&normalized_view_name) {
|
||||
if let Some(btree_table) = table.btree() {
|
||||
// Destroy the btree for the materialized view
|
||||
@@ -337,6 +338,38 @@ pub fn translate_drop_view(
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Construct the DBSP state table name
|
||||
use crate::incremental::compiler::DBSP_CIRCUIT_VERSION;
|
||||
Some(format!(
|
||||
"{DBSP_TABLE_PREFIX}{DBSP_CIRCUIT_VERSION}_{normalized_view_name}"
|
||||
))
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
// Destroy DBSP state table and index btrees if this is a materialized view
|
||||
if let Some(ref dbsp_table_name) = dbsp_table_name {
|
||||
// Destroy DBSP indexes first
|
||||
let dbsp_indexes: Vec<_> = schema.get_indices(dbsp_table_name).collect();
|
||||
for index in dbsp_indexes {
|
||||
program.emit_insn(Insn::Destroy {
|
||||
root: index.root_page,
|
||||
former_root_reg: 0, // No autovacuum
|
||||
is_temp: 0,
|
||||
});
|
||||
}
|
||||
|
||||
// Destroy DBSP state table btree
|
||||
if let Some(dbsp_table) = schema.get_table(dbsp_table_name) {
|
||||
if let Some(dbsp_btree_table) = dbsp_table.btree() {
|
||||
program.emit_insn(Insn::Destroy {
|
||||
root: dbsp_btree_table.root_page,
|
||||
former_root_reg: 0, // No autovacuum
|
||||
is_temp: 0,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Open cursor to sqlite_schema table
|
||||
@@ -374,7 +407,7 @@ pub fn translate_drop_view(
|
||||
});
|
||||
program.preassign_label_to_next_insn(loop_start_label);
|
||||
|
||||
// Check if this row is the view we're looking for
|
||||
// Check if this row should be deleted
|
||||
// Column 0 is type, Column 1 is name, Column 2 is tbl_name
|
||||
let col0_reg = program.alloc_register();
|
||||
let col1_reg = program.alloc_register();
|
||||
@@ -382,10 +415,10 @@ pub fn translate_drop_view(
|
||||
program.emit_column_or_rowid(sqlite_schema_cursor_id, 0, col0_reg);
|
||||
program.emit_column_or_rowid(sqlite_schema_cursor_id, 1, col1_reg);
|
||||
|
||||
// Check if type == 'view' and name == view_name
|
||||
// Check if this row matches the view, DBSP table, or DBSP index
|
||||
let skip_delete_label = program.allocate_label();
|
||||
|
||||
// Both regular and materialized views are stored as type='view' in sqlite_schema
|
||||
// Check if this is the view entry (type='view' and name=view_name)
|
||||
program.emit_insn(Insn::Ne {
|
||||
lhs: col0_reg,
|
||||
rhs: type_reg,
|
||||
@@ -393,7 +426,6 @@ pub fn translate_drop_view(
|
||||
flags: CmpInsFlags::default(),
|
||||
collation: program.curr_collation(),
|
||||
});
|
||||
|
||||
program.emit_insn(Insn::Ne {
|
||||
lhs: col1_reg,
|
||||
rhs: view_name_reg,
|
||||
@@ -401,8 +433,7 @@ pub fn translate_drop_view(
|
||||
flags: CmpInsFlags::default(),
|
||||
collation: program.curr_collation(),
|
||||
});
|
||||
|
||||
// Get the rowid and delete this row
|
||||
// Matches view - delete it
|
||||
program.emit_insn(Insn::RowId {
|
||||
cursor_id: sqlite_schema_cursor_id,
|
||||
dest: rowid_reg,
|
||||
@@ -423,6 +454,122 @@ pub fn translate_drop_view(
|
||||
|
||||
program.preassign_label_to_next_insn(end_loop_label);
|
||||
|
||||
// If this is a materialized view, delete DBSP table and index entries in a second pass
|
||||
// We do this in a separate loop to ensure we catch all entries even if they come
|
||||
// in different orders in sqlite_schema
|
||||
if let Some(ref dbsp_table_name) = dbsp_table_name {
|
||||
// Set up registers for DBSP table name and types (outside the loop for efficiency)
|
||||
let dbsp_table_name_reg_2 = program.alloc_register();
|
||||
program.emit_insn(Insn::String8 {
|
||||
dest: dbsp_table_name_reg_2,
|
||||
value: dbsp_table_name.clone(),
|
||||
});
|
||||
let table_type_reg_2 = program.alloc_register();
|
||||
program.emit_insn(Insn::String8 {
|
||||
dest: table_type_reg_2,
|
||||
value: "table".to_string(),
|
||||
});
|
||||
let index_type_reg_2 = program.alloc_register();
|
||||
program.emit_insn(Insn::String8 {
|
||||
dest: index_type_reg_2,
|
||||
value: "index".to_string(),
|
||||
});
|
||||
let dbsp_index_name_reg_2 = program.alloc_register();
|
||||
let dbsp_index_name_2 =
|
||||
format!("{PRIMARY_KEY_AUTOMATIC_INDEX_NAME_PREFIX}{dbsp_table_name}_1");
|
||||
program.emit_insn(Insn::String8 {
|
||||
dest: dbsp_index_name_reg_2,
|
||||
value: dbsp_index_name_2.clone(),
|
||||
});
|
||||
|
||||
// Allocate column registers once (outside the loop)
|
||||
let dbsp_col0_reg = program.alloc_register();
|
||||
let dbsp_col1_reg = program.alloc_register();
|
||||
|
||||
// Second pass: delete DBSP table and index entries
|
||||
let dbsp_end_loop_label = program.allocate_label();
|
||||
let dbsp_loop_start_label = program.allocate_label();
|
||||
|
||||
program.emit_insn(Insn::Rewind {
|
||||
cursor_id: sqlite_schema_cursor_id,
|
||||
pc_if_empty: dbsp_end_loop_label,
|
||||
});
|
||||
program.preassign_label_to_next_insn(dbsp_loop_start_label);
|
||||
|
||||
// Read columns for this row (reusing the same registers)
|
||||
program.emit_column_or_rowid(sqlite_schema_cursor_id, 0, dbsp_col0_reg);
|
||||
program.emit_column_or_rowid(sqlite_schema_cursor_id, 1, dbsp_col1_reg);
|
||||
|
||||
let dbsp_skip_delete_label = program.allocate_label();
|
||||
|
||||
// Check if this is the DBSP table entry (type='table' and name=dbsp_table_name)
|
||||
let check_dbsp_index_label = program.allocate_label();
|
||||
program.emit_insn(Insn::Ne {
|
||||
lhs: dbsp_col0_reg,
|
||||
rhs: table_type_reg_2,
|
||||
target_pc: check_dbsp_index_label,
|
||||
flags: CmpInsFlags::default(),
|
||||
collation: program.curr_collation(),
|
||||
});
|
||||
program.emit_insn(Insn::Ne {
|
||||
lhs: dbsp_col1_reg,
|
||||
rhs: dbsp_table_name_reg_2,
|
||||
target_pc: check_dbsp_index_label,
|
||||
flags: CmpInsFlags::default(),
|
||||
collation: program.curr_collation(),
|
||||
});
|
||||
// Matches DBSP table - delete it
|
||||
program.emit_insn(Insn::RowId {
|
||||
cursor_id: sqlite_schema_cursor_id,
|
||||
dest: rowid_reg,
|
||||
});
|
||||
program.emit_insn(Insn::Delete {
|
||||
cursor_id: sqlite_schema_cursor_id,
|
||||
table_name: "sqlite_schema".to_string(),
|
||||
is_part_of_update: false,
|
||||
});
|
||||
program.emit_insn(Insn::Goto {
|
||||
target_pc: dbsp_skip_delete_label,
|
||||
});
|
||||
|
||||
// Check if this is the DBSP index entry (type='index' and name=dbsp_index_name)
|
||||
program.preassign_label_to_next_insn(check_dbsp_index_label);
|
||||
program.emit_insn(Insn::Ne {
|
||||
lhs: dbsp_col0_reg,
|
||||
rhs: index_type_reg_2,
|
||||
target_pc: dbsp_skip_delete_label,
|
||||
flags: CmpInsFlags::default(),
|
||||
collation: program.curr_collation(),
|
||||
});
|
||||
program.emit_insn(Insn::Ne {
|
||||
lhs: dbsp_col1_reg,
|
||||
rhs: dbsp_index_name_reg_2,
|
||||
target_pc: dbsp_skip_delete_label,
|
||||
flags: CmpInsFlags::default(),
|
||||
collation: program.curr_collation(),
|
||||
});
|
||||
// Matches DBSP index - delete it
|
||||
program.emit_insn(Insn::RowId {
|
||||
cursor_id: sqlite_schema_cursor_id,
|
||||
dest: rowid_reg,
|
||||
});
|
||||
program.emit_insn(Insn::Delete {
|
||||
cursor_id: sqlite_schema_cursor_id,
|
||||
table_name: "sqlite_schema".to_string(),
|
||||
is_part_of_update: false,
|
||||
});
|
||||
|
||||
program.resolve_label(dbsp_skip_delete_label, program.offset());
|
||||
|
||||
// Move to next row
|
||||
program.emit_insn(Insn::Next {
|
||||
cursor_id: sqlite_schema_cursor_id,
|
||||
pc_if_next: dbsp_loop_start_label,
|
||||
});
|
||||
|
||||
program.preassign_label_to_next_insn(dbsp_end_loop_label);
|
||||
}
|
||||
|
||||
// Remove the view from the in-memory schema
|
||||
program.emit_insn(Insn::DropView {
|
||||
db: 0,
|
||||
|
||||
@@ -2494,3 +2494,68 @@ do_execsql_test_on_specific_db {:memory:} matview-count-distinct-global-aggregat
|
||||
3
|
||||
5
|
||||
4}
|
||||
|
||||
# Test that dropping a materialized view cleans up the DBSP state table
|
||||
do_execsql_test_on_specific_db {:memory:} matview-drop-cleans-up-dbsp-table {
|
||||
CREATE TABLE t(id INTEGER PRIMARY KEY, val INTEGER);
|
||||
INSERT INTO t VALUES (1, 10), (2, 20), (3, 30);
|
||||
|
||||
CREATE MATERIALIZED VIEW v AS
|
||||
SELECT val, COUNT(*) as cnt
|
||||
FROM t
|
||||
GROUP BY val;
|
||||
|
||||
-- Verify the view exists
|
||||
SELECT COUNT(*) FROM sqlite_schema WHERE type='view' AND name='v';
|
||||
|
||||
-- Verify the DBSP state table exists (name pattern: __turso_internal_dbsp_state_v*_v)
|
||||
SELECT COUNT(*) FROM sqlite_schema WHERE type='table' AND name LIKE '__turso_internal_dbsp_state_v%_v';
|
||||
|
||||
-- Verify the DBSP state index exists
|
||||
SELECT COUNT(*) FROM sqlite_schema WHERE type='index' AND name LIKE 'sqlite_autoindex___turso_internal_dbsp_state_v%_v_1';
|
||||
|
||||
-- Drop the materialized view
|
||||
DROP VIEW v;
|
||||
|
||||
-- Verify the view is gone
|
||||
SELECT COUNT(*) FROM sqlite_schema WHERE type='view' AND name='v';
|
||||
|
||||
-- Verify the DBSP state table is gone
|
||||
SELECT COUNT(*) FROM sqlite_schema WHERE type='table' AND name LIKE '__turso_internal_dbsp_state_v%_v';
|
||||
|
||||
-- Verify the DBSP state index is gone
|
||||
SELECT COUNT(*) FROM sqlite_schema WHERE type='index' AND name LIKE 'sqlite_autoindex___turso_internal_dbsp_state_v%_v_1';
|
||||
} {1
|
||||
1
|
||||
1
|
||||
0
|
||||
0
|
||||
0}
|
||||
|
||||
# Test that a materialized view can be recreated after dropping
|
||||
do_execsql_test_on_specific_db {:memory:} matview-recreate-after-drop {
|
||||
CREATE TABLE data(x INTEGER, y INTEGER);
|
||||
INSERT INTO data VALUES (1, 10), (2, 20), (3, 30);
|
||||
|
||||
CREATE MATERIALIZED VIEW mv AS
|
||||
SELECT x, SUM(y) as total
|
||||
FROM data
|
||||
GROUP BY x;
|
||||
|
||||
SELECT * FROM mv ORDER BY x;
|
||||
|
||||
-- Drop the view
|
||||
DROP VIEW mv;
|
||||
|
||||
-- Verify it can be recreated with a different definition
|
||||
CREATE MATERIALIZED VIEW mv AS
|
||||
SELECT x + 1 as modified_x, y * 2 as doubled_y
|
||||
FROM data
|
||||
WHERE x > 1;
|
||||
|
||||
SELECT * FROM mv ORDER BY modified_x;
|
||||
} {1|10.0
|
||||
2|20.0
|
||||
3|30.0
|
||||
3|40
|
||||
4|60}
|
||||
|
||||
Reference in New Issue
Block a user