Don't add committed rows to freshly created indexes

[kazimuth]

The FIXME here:

SpacetimeDB/crates/core/src/db/datastore/locking_tx_datastore/mut_tx.rs

Lines 365 to 371 in 3e6f91b

	// NOTE: Also add all the rows in the already committed table to the index.
	// FIXME: Is this correct? Index scan iterators (incl. the existing `Locking` versions)
	// appear to assume that a table's index refers only to rows within that table,
	// and does not handle the case where a `TxState` index refers to `CommittedState` rows.
	if let Some(committed_table) = commit_table {
	insert_index.build_from_rows(&index.columns, committed_table.scan_rows(commit_blob_store))?;
	}

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:

SpacetimeDB/crates/core/src/db/datastore/locking_tx_datastore/committed_state.rs

Lines 517 to 523 in 3e6f91b

	// Add all newly created indexes to the committed state.
	for (cols, mut index) in tx_table.indexes {
	if !commit_table.indexes.contains_key(&cols) {
	index.clear();
	commit_table.insert_index(commit_blob_store, cols, index);
	}
	}

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

cc @gefjon

[gefjon]

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.