search

xaya / libxayagame

MIT License
21 stars 19 forks source link

Do not rely on automatic primary keys for SQLite-based games #33

Open domob1812 opened 5 years ago

domob1812 commented 5 years ago

Automatic assignment of PRIMARY KEY values (e.g. through rowid) is not guaranteed to be consistent across instances, especially not when involving different SQLite versions and/or reorgs. (The documentation is not 100% clear in that respect, but it certainly does not clearly state that there is a deterministic and fixed algorithm.)

Thus, automatically assigned values should be completely avoided in our blockchain context. Thus, we should:

1) Provide a mechanism to generate unique integer keys in a consistent way. For this, we can simply use an explicit table that SQLIteGame manages, and provide users a function to generate the next ID and possibly to pre-reserve a range of IDs (e.g. for static data that is inserted as part of the initial data). This can also be cached in memory, so that we only ever sync it back to the database when a transaction is committed (rather than every time an ID is requested).

2) Encourage (or perhaps enforce) the use of WITHOUT ROWID tables. This, in turn, enforces that PRIMARY KEY values are explicit assigned at all times, so that no accidental reliance on rowid's is introduced.

Note that the session extension (which handles reorgs for us) only supports WITHOUT ROWID tables starting from SQLite version 3.17.0, so we will have to require at least that version. This excludes Debian 9 "Stretch", but includes current Ubuntu and the next Debian "Buster". I think that's an acceptable requirement.

domob1812 commented 5 years ago

It seems that the session extension has some issues when WITHOUT ROWID is combined with INSERT OR REPLACE (as in the sqlitegame_tests.cpp code if I change the table to be WITHOUT ROWID). Posted to the mailing list about that.

domob1812 commented 5 years ago

34 implemented the main part of this (first item from the original reporpt). Unfortunately, there was indeed a bug in sqlite that prevents using WITHOUT ROWID tables with the session extension. It has been fixed now, but we need to wait for that fix to be widely available in distributions before we can actually switch over to WITHOUT ROWID.

So until then, let's leave it as it is - everyone should make use of the new AutoId functions, but we cannot really enforce it.