Skip to content

Conversation

@cloutiertyler
Copy link
Contributor

Description of Changes

Cherry-picks the core event table implementation (fa83d6d40, 07d57a552) and adds:

  1. Bootstrap fix: Register st_event_table in bootstrap_system_tables() — it was missing from the cherry-picked commit, causing event table creation to fail at runtime when inserting into ST_EVENT_TABLE_ID.

  2. 4 datastore unit tests covering event table behaviors:

    • Insert+delete in the same tx cancels out (no TxData entry, no committed state)
    • Update yields only the final row in TxData (old row replaced, not accumulated)
    • Bare insert records in TxData but not in committed state
    • replay_insert is a no-op for event tables (commitlog replay doesn't rebuild state)
  3. Migration validation: ChangeTableEventFlag error in auto_migrate.rs prevents changing a table's event flag (event→non-event or vice versa) via auto-migration. Includes tests for both directions and a positive test that identical flags are accepted.

  4. Test fixture updates: Bootstrap test expectations updated for the new st_event_table system table (table row, column, index ID 23, constraint ID 19). Enabled spacetimedb-schema/test feature in datastore dev-deps to fix pre-existing test compilation issue with for_test methods.

API and ABI breaking changes

None. The ChangeTableEventFlag error variant is added to the internal AutoMigrateError enum but this is not a public API.

Expected complexity level and risk

1 — Straightforward additions. The bootstrap fix is a one-liner that was simply missing from the cherry-pick. The tests exercise existing behavior without changing it. The migration guard follows the exact same pattern as the existing ChangeTableType check.

Testing

  • cargo test -p spacetimedb-datastore — 77 tests pass (73 existing + 4 new)
  • cargo test -p spacetimedb-schema --features test --lib — 98 tests pass (96 existing + 2 new)
  • cargo check --workspace --exclude view-client — compiles clean (view-client has a pre-existing issue with CanBeLookupTable from the cherry-pick)
  • Reviewer: confirm that the 4 event table test names/assertions match the behaviors described in review feedback

Each validated schema type (TableDef, IndexDef, ReducerDef, etc.) now
has its own From impl for the corresponding V10 raw type, mirroring
the existing V9 conversions. The main From<ModuleDef> for RawModuleDefV10
is simplified to use .into() calls.
- Register st_event_table in bootstrap_system_tables (was missing)
- Add 4 datastore unit tests for event table behavior:
  - insert+delete cancellation produces no TxData
  - update yields only the final row in TxData
  - bare insert records in TxData but not committed state
  - replay_insert is a no-op for event tables
- Add ChangeTableEventFlag migration error preventing event flag
  transitions (event->non-event and vice versa)
- Update bootstrap test expectations for new system table
- Enable spacetimedb-schema test feature in datastore dev-deps
spacetimedb-lib = { path = "../lib", features = ["proptest"] }
spacetimedb-sats = { path = "../sats", features = ["proptest"] }
spacetimedb-commitlog = { path = "../commitlog", features = ["test"] }
spacetimedb-schema = { path = "../schema", features = ["test"] }
Copy link
Contributor Author

@cloutiertyler cloutiertyler Feb 10, 2026

Choose a reason for hiding this comment

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

[dev-dependencies]
spacetimedb-schema = { path = "../schema", features = ["test"] }

This enables the test feature of spacetimedb-schema when running datastore tests. The spacetimedb-schema crate is already a regular dependency of the datastore crate — my change just adds it as a dev-dependency
with the test feature flag enabled.

The test feature gates for_test() constructors on types like TableName, Identifier, and ColumnSchema. The datastore tests use these constructors extensively (e.g., TableName::for_test("Foo") in TableRow
conversions), but the feature wasn't being enabled in test builds — so the tests were already broken on master before our changes.

In short: the datastore already depends on schema. I'm just enabling the test feature so the existing test helpers compile.

I am not sure why claude is insistent on this. Could someone with knowledge verify that this is true and necessary?

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.

1 participant