Skip to content

Commit 3848843

Browse files
committed
Enable adding columns in auto migrations in schema
1 parent 83f52ca commit 3848843

File tree

7 files changed

+200
-31
lines changed

7 files changed

+200
-31
lines changed

crates/core/src/db/update.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ fn auto_migrate_database(
252252
log!(logger, "Removing-row level security `{sql_rls}`");
253253
stdb.drop_row_level_security(tx, sql_rls.clone())?;
254254
}
255+
_ => unimplemented!(),
255256
}
256257
}
257258

crates/lib/src/db/raw_def/v9.rs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use spacetimedb_primitives::*;
1414
use spacetimedb_sats::typespace::TypespaceBuilder;
1515
use spacetimedb_sats::AlgebraicType;
1616
use spacetimedb_sats::AlgebraicTypeRef;
17+
use spacetimedb_sats::AlgebraicValue;
1718
use spacetimedb_sats::ProductType;
1819
use spacetimedb_sats::ProductTypeElement;
1920
use spacetimedb_sats::SpacetimeType;
@@ -361,7 +362,25 @@ pub struct RawRowLevelSecurityDefV9 {
361362
#[sats(crate = crate)]
362363
#[cfg_attr(feature = "test", derive(PartialEq, Eq, PartialOrd, Ord))]
363364
#[non_exhaustive]
364-
pub enum RawMiscModuleExportV9 {}
365+
pub enum RawMiscModuleExportV9 {
366+
ColumnDefaultValue(RawColumnDefaultValueV9),
367+
}
368+
369+
/// Marks a field as having a particularly default
370+
#[derive(Debug, Clone, SpacetimeType)]
371+
#[sats(crate = crate)]
372+
#[cfg_attr(feature = "test", derive(PartialEq, Eq, PartialOrd, Ord))]
373+
pub struct RawColumnDefaultValueV9 {
374+
/// Must point to a `ProductType` or an error will be thrown during validation.
375+
pub table: RawIdentifier,
376+
377+
/// Must be the index of a valid column within `ty`.
378+
pub col_id: ColId,
379+
380+
/// A BSATN-encoded AlgebraicValue valid at `ty`.
381+
/// (We can't use AlgebraicValue directly because it isn't serializable!)
382+
pub value: Box<[u8]>,
383+
}
365384

366385
/// A type declaration.
367386
///
@@ -794,6 +813,20 @@ impl RawTableDefBuilder<'_> {
794813
self
795814
}
796815

816+
/// Adds a default value for the field.
817+
/// Note: this sets the value for all other tables defined on the same
818+
pub fn with_default_column_value(self, column: impl Into<ColId>, value: AlgebraicValue) -> Self {
819+
// Added to misc_exports for backwards-compatibility reasons
820+
self.module_def
821+
.misc_exports
822+
.push(RawMiscModuleExportV9::ColumnDefaultValue(RawColumnDefaultValueV9 {
823+
table: self.table.name.clone(),
824+
col_id: column.into(),
825+
value: spacetimedb_sats::bsatn::to_vec(&value).unwrap().into(),
826+
}));
827+
self
828+
}
829+
797830
/// Build the table and add it to the module, returning the `product_type_ref` of the table.
798831
pub fn finish(self) -> AlgebraicTypeRef {
799832
self.table.product_type_ref

crates/sats/src/buffer.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use core::fmt;
1010
use core::str::Utf8Error;
1111

1212
/// An error that occurred when decoding.
13-
#[derive(Debug, Clone, PartialEq, Eq)]
13+
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
1414
pub enum DecodeError {
1515
/// Not enough data was provided in the input.
1616
BufferLength {

crates/schema/src/auto_migrate.rs

Lines changed: 67 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,15 @@ pub enum AutoMigrateStep<'def> {
109109
///
110110
/// This should be done before any new indices are added.
111111
ChangeColumns(<TableDef as ModuleDefLookup>::Key<'def>),
112+
/// Add columns to a table, in a layout-INCOMPATIBLE way.
113+
///
114+
/// This is a destructive operation that requires first running a `DisconnectAllUsers`.
115+
///
116+
/// The added columns are guaranteed to be contiguous and at the end of the table. They are also
117+
/// guaranteed to have default values set.
118+
///
119+
/// This suppresses any `ChangeColumns` steps for the same table.
120+
AddColumns(<TableDef as ModuleDefLookup>::Key<'def>),
112121

113122
/// Add a table, including all indexes, constraints, and sequences.
114123
/// There will NOT be separate steps in the plan for adding indexes, constraints, and sequences.
@@ -124,6 +133,9 @@ pub enum AutoMigrateStep<'def> {
124133

125134
/// Change the access of a table.
126135
ChangeAccess(<TableDef as ModuleDefLookup>::Key<'def>),
136+
137+
/// Disconnect all users connected to the module.
138+
DisconnectAllUsers,
127139
}
128140

129141
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
@@ -137,7 +149,7 @@ pub struct ChangeColumnTypeParts {
137149
/// Something that might prevent an automatic migration.
138150
#[derive(thiserror::Error, Debug, PartialEq, Eq, PartialOrd, Ord)]
139151
pub enum AutoMigrateError {
140-
#[error("Adding a column {column} to table {table} requires a manual migration")]
152+
#[error("Adding a column {column} to table {table} requires a default value annotation")]
141153
AddColumn { table: Identifier, column: Identifier },
142154

143155
#[error("Removing a column {column} from table {table} requires a manual migration")]
@@ -386,11 +398,18 @@ fn auto_migrate_table<'def>(plan: &mut AutoMigratePlan<'def>, old: &'def TableDe
386398
})
387399
.map(|col_diff| -> Result<_> {
388400
match col_diff {
389-
Diff::Add { new } => Err(AutoMigrateError::AddColumn {
390-
table: new.table_name.clone(),
391-
column: new.name.clone(),
401+
Diff::Add { new } => {
402+
if new.default_value.is_some() {
403+
// row_type_changed, columns_added
404+
Ok(ProductMonoid(Any(false), Any(true)))
405+
} else {
406+
Err(AutoMigrateError::AddColumn {
407+
table: new.table_name.clone(),
408+
column: new.name.clone(),
409+
}
410+
.into())
411+
}
392412
}
393-
.into()),
394413
Diff::Remove { old } => Err(AutoMigrateError::RemoveColumn {
395414
table: old.table_name.clone(),
396415
column: old.name.clone(),
@@ -408,6 +427,7 @@ fn auto_migrate_table<'def>(plan: &mut AutoMigratePlan<'def>, old: &'def TableDe
408427

409428
// Note that the diff algorithm relies on `ModuleDefLookup` for `ColumnDef`,
410429
// which looks up columns by NAME, NOT position: precisely to allow this step to work!
430+
// We reject changes to
411431
let positions_ok = if old.col_id == new.col_id {
412432
Ok(())
413433
} else {
@@ -417,22 +437,34 @@ fn auto_migrate_table<'def>(plan: &mut AutoMigratePlan<'def>, old: &'def TableDe
417437
.into())
418438
};
419439

420-
(types_ok, positions_ok).combine_errors().map(|(x, _)| x)
440+
(types_ok, positions_ok)
441+
.combine_errors()
442+
.map(|(x, _)| ProductMonoid(x, Any(false)))
421443
}
422444
}
423445
})
424-
.collect_all_errors::<Any>();
446+
.collect_all_errors::<ProductMonoid<Any, Any>>();
425447

426-
let ((), Any(row_type_changed)) = (type_ok, columns_ok).combine_errors()?;
448+
let ((), ProductMonoid(Any(row_type_changed), Any(columns_added))) = (type_ok, columns_ok).combine_errors()?;
427449

428-
if row_type_changed {
450+
if columns_added {
451+
if !plan
452+
.steps
453+
.iter()
454+
.any(|step| matches!(step, AutoMigrateStep::DisconnectAllUsers))
455+
{
456+
plan.steps.push(AutoMigrateStep::DisconnectAllUsers);
457+
}
458+
plan.steps.push(AutoMigrateStep::AddColumns(key));
459+
} else if row_type_changed {
429460
plan.steps.push(AutoMigrateStep::ChangeColumns(key));
430461
}
431462

432463
Ok(())
433464
}
434465

435466
/// An "any" monoid with `false` as identity and `|` as the operator.
467+
#[derive(Default)]
436468
struct Any(bool);
437469

438470
impl FromIterator<Any> for Any {
@@ -448,6 +480,26 @@ impl BitOr for Any {
448480
}
449481
}
450482

483+
/// A monoid that allows running two `Any`s in parallel.
484+
#[derive(Default)]
485+
struct ProductMonoid<M1, M2>(M1, M2);
486+
487+
impl<M1: BitOr<Output = M1>, M2: BitOr<Output = M2>> BitOr for ProductMonoid<M1, M2> {
488+
type Output = Self;
489+
490+
fn bitor(self, rhs: Self) -> Self::Output {
491+
Self(self.0 | rhs.0, self.1 | rhs.1)
492+
}
493+
}
494+
495+
impl<M1: BitOr<Output = M1> + Default, M2: BitOr<Output = M2> + Default> FromIterator<ProductMonoid<M1, M2>>
496+
for ProductMonoid<M1, M2>
497+
{
498+
fn from_iter<T: IntoIterator<Item = ProductMonoid<M1, M2>>>(iter: T) -> Self {
499+
iter.into_iter().reduce(|p1, p2| p1 | p2).unwrap_or_default()
500+
}
501+
}
502+
451503
fn ensure_old_ty_upgradable_to_new(
452504
within: bool,
453505
old: &ColumnDef,
@@ -700,7 +752,7 @@ mod tests {
700752
use spacetimedb_data_structures::expect_error_matching;
701753
use spacetimedb_lib::{
702754
db::raw_def::{v9::btree, *},
703-
AlgebraicType, ProductType, ScheduleAt,
755+
AlgebraicType, AlgebraicValue, ProductType, ScheduleAt,
704756
};
705757
use spacetimedb_primitives::ColId;
706758
use v9::{RawModuleDefV9Builder, TableAccess};
@@ -813,11 +865,13 @@ mod tests {
813865
("id", AlgebraicType::U64),
814866
("name", AlgebraicType::String),
815867
("count", AlgebraicType::U16),
868+
("freshness", AlgebraicType::U32), // added column!
816869
]),
817870
true,
818871
)
819872
// add column sequence
820873
.with_column_sequence(0)
874+
.with_default_column_value(3, AlgebraicValue::U32(5))
821875
// change access
822876
.with_access(TableAccess::Private)
823877
.finish();
@@ -963,6 +1017,9 @@ mod tests {
9631017
steps.contains(&AutoMigrateStep::ChangeColumns(&deliveries)),
9641018
"{steps:?}"
9651019
);
1020+
1021+
assert!(steps.contains(&AutoMigrateStep::DisconnectAllUsers), "{steps:?}");
1022+
assert!(steps.contains(&AutoMigrateStep::AddColumns(&bananas)), "{steps:?}");
9661023
}
9671024

9681025
#[test]

crates/schema/src/def.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,14 @@ use spacetimedb_data_structures::error_stream::{CollectAllErrors, CombineErrors,
3232
use spacetimedb_data_structures::map::HashMap;
3333
use spacetimedb_lib::db::raw_def;
3434
use spacetimedb_lib::db::raw_def::v9::{
35-
Lifecycle, RawConstraintDataV9, RawConstraintDefV9, RawIdentifier, RawIndexAlgorithm, RawIndexDefV9,
36-
RawModuleDefV9, RawReducerDefV9, RawRowLevelSecurityDefV9, RawScheduleDefV9, RawScopedTypeNameV9, RawSequenceDefV9,
37-
RawSql, RawTableDefV9, RawTypeDefV9, RawUniqueConstraintDataV9, TableAccess, TableType,
35+
Lifecycle, RawColumnDefaultValueV9, RawConstraintDataV9, RawConstraintDefV9, RawIdentifier, RawIndexAlgorithm,
36+
RawIndexDefV9, RawMiscModuleExportV9, RawModuleDefV9, RawReducerDefV9, RawRowLevelSecurityDefV9, RawScheduleDefV9,
37+
RawScopedTypeNameV9, RawSequenceDefV9, RawSql, RawTableDefV9, RawTypeDefV9, RawUniqueConstraintDataV9, TableAccess,
38+
TableType,
3839
};
3940
use spacetimedb_lib::{ProductType, RawModuleDef};
4041
use spacetimedb_primitives::{ColId, ColList, ColOrCols, ColSet, ReducerId, TableId};
41-
use spacetimedb_sats::AlgebraicType;
42+
use spacetimedb_sats::{AlgebraicType, AlgebraicValue};
4243
use spacetimedb_sats::{AlgebraicTypeRef, Typespace};
4344

4445
pub mod deserialize;
@@ -659,6 +660,9 @@ pub struct ColumnDef {
659660

660661
/// The table this `ColumnDef` is stored in.
661662
pub table_name: Identifier,
663+
664+
/// The default value of this column, if present.
665+
pub default_value: Option<AlgebraicValue>,
662666
}
663667

664668
/// A constraint definition attached to a table.

0 commit comments

Comments
 (0)