Skip to content

Commit 93c0015

Browse files
fix error when dropping view in auto-migration (#3836)
# Description of Changes Fixes the following issues: 1. When dropping a view, we deleted its row from `st_view`, but didn't drop the backing table. 2. `delete_col_eq` returned a nonsensical error if the delete set was empty. # API and ABI breaking changes None # Expected complexity level and risk 0 # Testing - [x] Auto-migrate smoketests
1 parent afe169a commit 93c0015

2 files changed

Lines changed: 76 additions & 4 deletions

File tree

crates/datastore/src/locking_tx_datastore/mut_tx.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -465,9 +465,6 @@ impl MutTxId {
465465
fn delete_col_eq(&mut self, table_id: TableId, col_pos: ColId, value: &AlgebraicValue) -> Result<()> {
466466
let rows = self.iter_by_col_eq(table_id, col_pos, value)?;
467467
let ptrs_to_delete = rows.map(|row_ref| row_ref.pointer()).collect::<Vec<_>>();
468-
if ptrs_to_delete.is_empty() {
469-
return Err(TableError::IdNotFound(SystemTable::st_column, col_pos.0 as _).into());
470-
}
471468

472469
for ptr in ptrs_to_delete {
473470
// TODO(error-handling,bikeshedding): Consider correct failure semantics here.
@@ -516,6 +513,7 @@ impl MutTxId {
516513

517514
/// Drop the backing table of a view and update the system tables.
518515
pub fn drop_view(&mut self, view_id: ViewId) -> Result<()> {
516+
let st_view_row = self.lookup_st_view(view_id)?;
519517
// Drop the view's metadata
520518
self.drop_st_view(view_id)?;
521519
self.drop_st_view_param(view_id)?;
@@ -527,7 +525,7 @@ impl MutTxId {
527525
if let StViewRow {
528526
table_id: Some(table_id),
529527
..
530-
} = self.lookup_st_view(view_id)?
528+
} = st_view_row
531529
{
532530
return self.drop_table(table_id);
533531
};

smoketests/tests/views.py

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,80 @@ def test_views_auto_migration(self):
426426
""")
427427

428428

429+
class AutoMigrateDropView(Smoketest):
430+
MODULE_CODE = """
431+
use spacetimedb::ViewContext;
432+
433+
#[derive(Copy, Clone)]
434+
#[spacetimedb::table(name = player_state)]
435+
pub struct PlayerState {
436+
#[primary_key]
437+
id: u64,
438+
#[index(btree)]
439+
level: u64,
440+
}
441+
442+
#[spacetimedb::view(name = player, public)]
443+
pub fn player(ctx: &ViewContext) -> Option<PlayerState> {
444+
ctx.db.player_state().id().find(1u64)
445+
}
446+
"""
447+
448+
MODULE_CODE_DROP_VIEW = """
449+
#[derive(Copy, Clone)]
450+
#[spacetimedb::table(name = player_state)]
451+
pub struct PlayerState {
452+
#[primary_key]
453+
id: u64,
454+
#[index(btree)]
455+
level: u64,
456+
}
457+
"""
458+
459+
def test_auto_migration_drop_view(self):
460+
"""Assert that views can be dropped in an auto-migration"""
461+
462+
self.write_module_code(self.MODULE_CODE_DROP_VIEW)
463+
self.publish_module(self.database_identity, clear=False, break_clients=False)
464+
465+
466+
class AutoMigrateAddView(Smoketest):
467+
MODULE_CODE = """
468+
#[derive(Copy, Clone)]
469+
#[spacetimedb::table(name = player_state)]
470+
pub struct PlayerState {
471+
#[primary_key]
472+
id: u64,
473+
#[index(btree)]
474+
level: u64,
475+
}
476+
"""
477+
478+
MODULE_CODE_ADD_VIEW = """
479+
use spacetimedb::ViewContext;
480+
481+
#[derive(Copy, Clone)]
482+
#[spacetimedb::table(name = player_state)]
483+
pub struct PlayerState {
484+
#[primary_key]
485+
id: u64,
486+
#[index(btree)]
487+
level: u64,
488+
}
489+
490+
#[spacetimedb::view(name = player, public)]
491+
pub fn player(ctx: &ViewContext) -> Option<PlayerState> {
492+
ctx.db.player_state().id().find(1u64)
493+
}
494+
"""
495+
496+
def test_auto_migration_drop_view(self):
497+
"""Assert that views can be added in an auto-migration"""
498+
499+
self.write_module_code(self.MODULE_CODE_ADD_VIEW)
500+
self.publish_module(self.database_identity, clear=False)
501+
502+
429503
class AutoMigrateViewsTrapped(Smoketest):
430504
MODULE_CODE = """
431505
use spacetimedb::ViewContext;

0 commit comments

Comments
 (0)