Moving object defs to SATS

[mamcx]

Description of Changes

Split the move of DB definitions from #439 in a smaller change set.

This is part of the larger #267. The move into SATS is because cyclic dependencies between db <-> sats <-> vm <-> lib <-> bindings, where we have the need to communicate information across the layers, in concret with the large PR, about the definitions of database objects. For example, we need this in the query layer where TableDef hold our metadata and both vm <-> core need it.

API and ABI

• This is a breaking change to the module ABI
• This is a breaking change to the module API
• This is a breaking change to the ClientAPI
• This is a breaking change to the SDK API

If the API is breaking, please state below what will break

Some bindings internal types (and elsewhere) are moved to sats

Expected complexity level and risk

2

[kim]

I suppose the motivation for moving all those types into sats is some cyclic dependency issue, but I'm not able to see where it arises.

Arguably, it's quite strange to expand the scope of sats in this way. Maybe there is a better way to organize the crates, or maybe the solution is not so obvious. I would at least request to explain clearly in the PR description what the problem is and how it is solved. This way, git blame should give some context.

I'd like @Centril to weigh in as well if we should try to find a nicer layout now, or merge this and revisit later.

[mamcx]

I suppose the motivation for moving all those types into sats is some cyclic dependency issue, but I'm not able to see where it arises.

Yes. I updated the pr with a comment. I think trying to put it in a different place is not the best use of energy now, this set of PR is already late, and we have also an incoming refactor for the datastore and in special query engine where we could hit deps problems (I already getting this almost each time I rebase).

When we have the functionality in set we could revisit how to reduce/change the location of things, especially when we have a firm grasp on these incoming refactors.

In concrete, the query engine will need more metadata about the DB, and also the bindings-* crates where is likely AFAIK that we need to move ast objects there, including metadata. Even if is not the case, I don't see how foresee this changes now.

[Centril]

Looks good; I made some minor changes during my review:

1. ColumnIndexAttribute was moved into spacetimedb_primitives which crates bindings-* can use so now we don't need #[path] hackery.
2. ProductValue takes usize again in the various field methods. I reverted your change here because product values need not be a row and so their fields need not be columns. Perhaps with more neutral language we could use a newtype here in the future.
3. Some minor refactoring opportunities I saw in your diff.

[kulakowski]

We'd discussed the spelling of "BTree" and "Hash" vs "btree" and "hash". I thought we had agreed on the latter, but it looks like the former made it in.

[Centril]

We'd discussed the spelling of "BTree" and "Hash" vs "btree" and "hash". I thought we had agreed on the latter, but it looks like the former made it in.

Ah crap; I thought I had that part removed but must have been a bad rebase on that bit.
The spelling should now be irrelevant as the impl should be unused and removable.

[Centril]

Removed in #559.