Skip to content

Conversation

@Shubham8287
Copy link
Contributor

@Shubham8287 Shubham8287 commented Jan 22, 2026

Description of Changes

Patch:

  1. crates/lib/src/db/raw_def/v10.rs - a definition as per https://github.com/clockworklabs/SpacetimeDBPrivate/issues/2412.
  2. Refactors RawModuleDefV9 validation code to dedup some of the core validation logic, No functionality should change there.
  3. Validation logic for new RawModuleDefV10.
  4. Modify crates::schema::def::ModuleDef :
    • To include raw_module_def_version
    • visibility field to existing RedcuerDef and ProcedureDef
  5. It deprecates RLS in favour of Views, If anyoine feel otherwise, let me know.

Lot of code is duplicated from RawModuleDefV9 includng tests.

API and ABI breaking changes

NA, RawModuleDefV10 is not yet exported by modules.

Expected complexity level and risk

3? close analyses of structure is important to ensure future extensibility.

Testing

For code motion in RawModuleDefV9 validation: Existing unit tests seems to cover any functionality change.
For RawModuleDefV10: V9 test has been repeated with more asserts to check on visibility

Copy link
Contributor

@gefjon gefjon left a comment

Choose a reason for hiding this comment

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

I haven't looked at your validation logic yet, but I assume it's reasonably straightforward. The raw def format itself looks great, excellent work.

/// Unlike V9 where lifecycle was a field on reducers,
/// V10 stores lifecycle-to-reducer mappings separately.
LifeCycleReducers(Vec<RawLifeCycleReducerDefV10>),
//TODO: Add section for Event tables, and Case conversion before exposing this from module
Copy link
Contributor

Choose a reason for hiding this comment

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

That's the neat part: we can expose this whenever we want, and adding new sections / variants to this enum is a non-breaking change!

Comment on lines +110 to +113
/// The name of the table.
/// Unique within a module, acts as the table's identifier.
/// Must be a valid `spacetimedb_schema::identifier::Identifier`.
pub name: RawIdentifier,
Copy link
Contributor

Choose a reason for hiding this comment

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

For added clarity with Tyler's case conversion proposal, could you change this (and other name fields) to be called source_name?

Comment on lines +148 to +149
/// Default values for columns in this table.
pub default_values: Vec<RawColumnDefaultValueV10>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think it would make more sense to store this in a separate section? Or any of the other stuff in here? I have no particular opinion, just thought I'd mention it as a possibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants