rtldg
commented
1 year ago Oooh wow, probably haven't heard about this happening yet because timer users usually don't use sqlite. (and because some of the migrations weren't sqlite compatible for a while so updating the timer version wasn't really practical with sqlite dbs)
I definitely wouldn't be opposed to just swapping out that REPLACE INTO query with the sqlite upsert. This needs sqlite version >= 3.24.0 while the version in my Sourcemod 1.10 binaries seem to be 3.26.0... so that should be fine.
You do say imo the better option is to add foreign keys back though. I'm not not well versed in SQL though so I probably won't be adding foreign keys in myself right away. If you want to jump on it and make a pr too that would be good.
Any thoughts on me making a quick commit to update that query for now and then the foreign key thing comes later?
jedso
I'm working on a PR for normalizing stage times and in the process enabled foreign key constraints for SQLite with
PRAGMA foreign_keys = ON(it's off by default). But in doing so it exposed a pretty hilarious bug for SQLite databases: https://github.com/shavitush/bhoptimer/blob/87c2b1e4368b7553ff5495a681dd566c54c57732/addons/sourcemod/scripting/shavit-core.sp#L2716-L2721 This query which runs OnClientAuthorized REPLACEs the whole user row with an implicit DELETE and INSERT. This is a problem because since v2.5.5a (bb74e14bcfc3285f71757b2fd674cb6754f7f041), auth in the playertimes table has referenced auth in the users table as a FK with ON UPDATE CASCADE ON DELETE CASCADE referential actions.So basically, with foreign key constraints enabled in SQLite, all playertimes are deleted for the player that joins the server after the user table is updated... not good.
This bug is obviously fixable by just doing another query (not using REPLACE), but this bug doesn't happen on databases created after v3.0.8. For SQLite, it's because the FK constraint was removed from the CREATE TABLE statement (https://github.com/shavitush/bhoptimer/commit/7ca863f04b9319e832c61d000021aa7e7933a356) and for MySQL, because of this commit (60c614df9d95095e1529785a3fc40d97f80d5c13) which added the #if 0 directive around the ADD CONSTRAINT query: https://github.com/shavitush/bhoptimer/blob/87c2b1e4368b7553ff5495a681dd566c54c57732/addons/sourcemod/scripting/include/shavit/sql-create-tables-and-migrations.sp#L554-L559
So databases created >= v3.1.0 don't have this auth FK in playertimes, but there was no migration for dbs created <= v3.0.8 to remove the existing auth FK creating an arbitrary schema difference depending on which version of the plugin was used to create the db.
TL;DR: there should be a migration that either adds the auth FK back to playertimes for dbs created >= v3.1.0 (imo the better option), or removes it if it exists (<= v3.0.8).