[feat] Add support for other db clients

[marvin-wtt]

About

Add support for other DB client drivers.

• The visitor query needed some modification, as some drivers (like SQLite) don't support queries to increment JSON values. Also, the insertion of a new visit must occur in the transaction too to avoid concurrent writes.

See ##761

Breaking Change

• When migrating, all cooldowns are lost.

Tasks

• Convert raw queries
• Replace special column types
• Do not use returning in queries

[marvin-wtt]

@poeti8 It seems like the migrations got messed up somewhere in the past. Some migrations are modified at a later stage or appied multiple times. I'd adress this in another PR to not blow this one up.

[poeti8]

@marvin-wtt Hmmm I see. Thanks.

[marvin-wtt]

@poeti8 PR should be ready for review now

[marvin-wtt]

@poeti8 I think it would be best to add some tests to verify that it actually works for different DB types, mainly mysql, mariadb, pg, and sqlite3. I offer to do it. Does vitest work for you, or do you prefer jest?

[poeti8]

Thank you, I'll run this to test it soon.

Does vitest work for you, or do you prefer jest?

vitest please.

[marvin-wtt]

Thank you, I'll run this to test it soon.

I think it's easier if you just check out the migrations PR directly. This way you can just run npm run migrate and everything is set up.

[poeti8]

Hey, any updates on this? Is it done other than my suggestions?

[poeti8]

I tested the branch manually and everything worked fine. I created a testsuit but adding all tests will take some time. I'm currently on vacation. Maybe it's better to push it in a separate PR.

Sweet, thanks. Yes, we can leave the tests for another pull request. I'll go ahead by testing and merging this one soon.

The migration branch addresses the issues regarding the migrations but I think we disagree on how it should be run.

I'll take a close look again. We'll figure something out.

[marvin-wtt]

Returning is not supported by mysql. By default the inserted id is returned. For pg, returning id should result in the same

[poeti8]

Returning is not supported by mysql. By default the inserted id is returned. For pg, returning id should result in the same

Yes I noticed. The mistake however was that knex doesn't return id as value, it returns it within the object: [{ id }]. So I updated the code to access the id correctly via createdRows[0].id.

[marvin-wtt]

I think your fix breaks MYSQL support. Can you confirm? Maybe we need to add a helper function.

return typeof res[0] === 'string' ? res[0] : res[0].id;

[poeti8]

I think your fix breaks MYSQL support. Can you confirm? Maybe we need to add a helper function.

You're right I should make sure. I'll write a helper function if that is the case. Although, I assume knex would be consistent with what it returns, otherwise people would end up handling all sort of edge cases manually.

[poeti8]

This looks good. I'll test everything manually again with the three databases and merge it after.

Really good job. Thanks!

[poeti8]

Found an issue: Dates in SQLite are saved like 2024-09-25 12:56:52. This would break any place we check or parse the dates since it's not valid for new Date(). Need to write a helper function to handle these. Maybe can only use this helper function if the client is one of SQLite's to avoid using any RegExp that would make things slower.

[marvin-wtt]

I'll take a look. If it's not possible to store it in zulu time, at least we can add a knex typecast so no helper function is needed