Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release/v0.12.0 beta hotfix3 #1868

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/client-api-messages/src/websocket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ pub fn should_compress(len: usize) -> bool {
/// TODO(perf): measure!
const COMPRESS_THRESHOLD: usize = 1024;

len <= COMPRESS_THRESHOLD
len >= COMPRESS_THRESHOLD
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
len >= COMPRESS_THRESHOLD
len > COMPRESS_THRESHOLD

(to match master)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Centril please suggest this change in the PR (#1867) this is only a release branch.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, but that PR does not need to merge into master though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah fair.. and this release has already "gone out" insofar as we rolled it to the places that needed it critically.

}

pub fn brotli_compress(bytes: &[u8], out: &mut Vec<u8>) {
Expand Down
8 changes: 4 additions & 4 deletions crates/core/src/db/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,16 @@ pub fn update_database(
let existing_tables = stdb.get_all_tables_mut(tx)?;

// TODO: consider using `ErrorStream` here.
let old_def = plan.old_def();
let old_module_def = plan.old_def();
for table in existing_tables
.iter()
.filter(|table| table.table_type != StTableType::System)
{
let old_def = old_def
let old_def = old_module_def
.table(&table.table_name[..])
.ok_or_else(|| anyhow::anyhow!("table {} not found in old_def", table.table_name))?;
.ok_or_else(|| anyhow::anyhow!("table {} not found in old_module_def", table.table_name))?;

table.check_compatible(old_def)?;
table.check_compatible(old_module_def, old_def)?;
}

match plan {
Expand Down
27 changes: 14 additions & 13 deletions crates/schema/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub trait Schema: Sized {
fn from_module_def(module_def: &ModuleDef, def: &Self::Def, parent_id: Self::ParentId, id: Self::Id) -> Self;

/// Check that a schema entity is compatible with a definition.
fn check_compatible(&self, def: &Self::Def) -> Result<(), anyhow::Error>;
fn check_compatible(&self, module_def: &ModuleDef, def: &Self::Def) -> Result<(), anyhow::Error>;
}

/// A data structure representing the schema of a database table.
Expand Down Expand Up @@ -602,7 +602,7 @@ impl Schema for TableSchema {
)
}

fn check_compatible(&self, def: &Self::Def) -> Result<(), anyhow::Error> {
fn check_compatible(&self, module_def: &ModuleDef, def: &Self::Def) -> Result<(), anyhow::Error> {
ensure_eq!(&self.table_name[..], &def.name[..], "Table name mismatch");
ensure_eq!(self.primary_key, def.primary_key, "Primary key mismatch");
let def_table_access: StAccess = (def.table_access).into();
Expand All @@ -615,7 +615,7 @@ impl Schema for TableSchema {
.columns
.get(col.col_pos.0 as usize)
.ok_or_else(|| anyhow::anyhow!("Column {} not found in definition", col.col_pos.0))?;
col.check_compatible(col_def)?;
col.check_compatible(module_def, col_def)?;
}
ensure_eq!(self.columns.len(), def.columns.len(), "Column count mismatch");

Expand All @@ -624,7 +624,7 @@ impl Schema for TableSchema {
.indexes
.get(&index.index_name[..])
.ok_or_else(|| anyhow::anyhow!("Index {} not found in definition", index.index_id.0))?;
index.check_compatible(index_def)?;
index.check_compatible(module_def, index_def)?;
}
ensure_eq!(self.indexes.len(), def.indexes.len(), "Index count mismatch");

Expand All @@ -633,7 +633,7 @@ impl Schema for TableSchema {
.constraints
.get(&constraint.constraint_name[..])
.ok_or_else(|| anyhow::anyhow!("Constraint {} not found in definition", constraint.constraint_id.0))?;
constraint.check_compatible(constraint_def)?;
constraint.check_compatible(module_def, constraint_def)?;
}
ensure_eq!(
self.constraints.len(),
Expand All @@ -646,7 +646,7 @@ impl Schema for TableSchema {
.sequences
.get(&sequence.sequence_name[..])
.ok_or_else(|| anyhow::anyhow!("Sequence {} not found in definition", sequence.sequence_id.0))?;
sequence.check_compatible(sequence_def)?;
sequence.check_compatible(module_def, sequence_def)?;
}
ensure_eq!(self.sequences.len(), def.sequences.len(), "Sequence count mismatch");

Expand All @@ -655,7 +655,7 @@ impl Schema for TableSchema {
.schedule
.as_ref()
.ok_or_else(|| anyhow::anyhow!("Schedule not found in definition"))?;
schedule.check_compatible(schedule_def)?;
schedule.check_compatible(module_def, schedule_def)?;
}
ensure_eq!(
self.schedule.is_some(),
Expand Down Expand Up @@ -754,9 +754,10 @@ impl Schema for ColumnSchema {
}
}

fn check_compatible(&self, def: &Self::Def) -> Result<(), anyhow::Error> {
fn check_compatible(&self, module_def: &ModuleDef, def: &Self::Def) -> Result<(), anyhow::Error> {
ensure_eq!(&self.col_name[..], &def.name[..], "Column name mismatch");
ensure_eq!(self.col_type, def.ty, "Column type mismatch");
let resolved_def_ty = WithTypespace::new(module_def.typespace(), &def.ty).resolve_refs()?;
ensure_eq!(self.col_type, resolved_def_ty, "Column type mismatch");
ensure_eq!(self.col_pos, def.col_id, "Columnh ID mismatch");
Ok(())
}
Expand Down Expand Up @@ -843,7 +844,7 @@ impl Schema for SequenceSchema {
}
}

fn check_compatible(&self, def: &Self::Def) -> Result<(), anyhow::Error> {
fn check_compatible(&self, _module_def: &ModuleDef, def: &Self::Def) -> Result<(), anyhow::Error> {
ensure_eq!(&self.sequence_name[..], &def.name[..], "Sequence name mismatch");
ensure_eq!(self.col_pos, def.column, "Sequence column mismatch");
ensure_eq!(self.increment, def.increment, "Sequence increment mismatch");
Expand Down Expand Up @@ -895,7 +896,7 @@ impl Schema for ScheduleSchema {
}
}

fn check_compatible(&self, def: &Self::Def) -> Result<(), anyhow::Error> {
fn check_compatible(&self, _module_def: &ModuleDef, def: &Self::Def) -> Result<(), anyhow::Error> {
ensure_eq!(&self.schedule_name[..], &def.name[..], "Schedule name mismatch");
ensure_eq!(
&self.reducer_name[..],
Expand Down Expand Up @@ -939,7 +940,7 @@ impl Schema for IndexSchema {
}
}

fn check_compatible(&self, def: &Self::Def) -> Result<(), anyhow::Error> {
fn check_compatible(&self, _module_def: &ModuleDef, def: &Self::Def) -> Result<(), anyhow::Error> {
ensure_eq!(&self.index_name[..], &def.name[..], "Index name mismatch");
ensure_eq!(&self.index_algorithm, &def.algorithm, "Index algorithm mismatch");
Ok(())
Expand Down Expand Up @@ -1002,7 +1003,7 @@ impl Schema for ConstraintSchema {
}
}

fn check_compatible(&self, def: &Self::Def) -> Result<(), anyhow::Error> {
fn check_compatible(&self, _module_def: &ModuleDef, def: &Self::Def) -> Result<(), anyhow::Error> {
ensure_eq!(&self.constraint_name[..], &def.name[..], "Constraint name mismatch");
ensure_eq!(&self.data, &def.data, "Constraint data mismatch");
Ok(())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
import logging


class AddTablePseudomigration(Smoketest):
class AddTableAutoMigration(Smoketest):
MODULE_CODE = """
use spacetimedb::{println, ReducerContext, Table};
use spacetimedb::{println, ReducerContext, Table, SpacetimeType};

#[spacetimedb::table(name = person)]
pub struct Person {
Expand All @@ -23,6 +23,19 @@ class AddTablePseudomigration(Smoketest):
println!("{}: {}", prefix, person.name);
}
}

#[spacetimedb::table(name = point_mass)]
pub struct PointMass {
mass: f64,
/// This used to cause an error when check_compatible did not resolve types in a `ModuleDef`.
position: Vector2,
}

#[derive(SpacetimeType, Clone, Copy)]
pub struct Vector2 {
x: f64,
y: f64,
}
"""

MODULE_CODE_UPDATED = (
Expand All @@ -47,7 +60,7 @@ class AddTablePseudomigration(Smoketest):
"""
)

def test_add_table_pseudomigration(self):
def test_add_table_auto_migration(self):
"""This tests uploading a module with a schema change that should not require clearing the database."""

logging.info("Initial publish complete")
Expand Down
Loading