SpacetimeDB icon indicating copy to clipboard operation
SpacetimeDB copied to clipboard

Don't add committed rows to freshly created indexes

Open kazimuth opened this issue 1 year ago • 1 comments

The FIXME here:

https://github.com/clockworklabs/SpacetimeDB/blob/3e6f91be8969b2f5ceb3beed60434de3393252c4/crates/core/src/db/datastore/locking_tx_datastore/mut_tx.rs#L365-L371

is a problem.

This came up while trying to implement #1523. 1523 is just a convenience, so this isn't a huge deal that it's blocked. However, this is going to be a much bigger problem for implementing spacetime migrate.

The issue is that, when we create an index during the transaction, we populate the index with rows in the committed state, as well as rows in the transaction state. This makes it impossible to read the row corresponding to an index entry in general! The row could be in one of two places, the committed state, or the transaction state. But this bit is not stored anywhere.

This is fixed once the transaction is committed -- the transaction state is merged into the committed state and the index is repopulated:

https://github.com/clockworklabs/SpacetimeDB/blob/3e6f91be8969b2f5ceb3beed60434de3393252c4/crates/core/src/db/datastore/locking_tx_datastore/committed_state.rs#L517-L523

It's only an issue when performing queries during a transaction that added an index. This will mainly come up during migrations.

cc @gefjon

kazimuth avatar Jul 17 '24 16:07 kazimuth

But this bit is not stored anywhere.

That bit is, in fact, stored: it's the SquashedOffset in the RowPointer. The issue is that many code paths assume that all RowPointers within an index in a table are pointers into that table, an assumption which usually holds but is broken in the specific edge case of creating an index and using it within the same transaction. It's worsened because many of the code paths which make this assumption have access to the insert table and its indexes, but not to the committed table. We could alternately thread the committed table through as an extra argument to all accesses to the insert table and its indexes, but that sounds like a PITA.

gefjon avatar Jul 17 '24 17:07 gefjon