add operations are forced to receive a client-side ID due to Knex + MySQL limitations

[joelalejandro]

Due to knex/knex#2682, add operations in MySQL require an id to be passed, otherwise Kurier won't return the resource ID on the result of the operation.

kurier/src/processors/knex-processor.ts

Line 163 in 28c25e3

const ids = await this.getQuery().insert(dataToInsert, primaryKeyName);

This insert + RETURNING syntax works only on engines such as PostgreSQL or Oracle.

[spersico]

I took a look into this issue (mostly because I hate the warnings in the console every time I run the tests).
I see two approaches here:

• Add a select into the end of a request, when the DB engine doesn't support returning stuff.
• Drop support for MySQL and SQLITE (no one cares about them anyway, it's not as if SQLITE is used in Android, or MySQL was used extensively in AuroraDB 😜)

The problem here is that... this is tricky grounds:
So... I know that this works for auto incremental ids in MySQL: SELECT MAX(LAST_INSERT_ID()) FROM SOME_TABLE .
But The 2 queries approach it's not reliable (it's not atomic, so it can't be trusted), and it doesn't support bulk insertions.

• So, to be sure, we would need to make the "add" operation a transaction that includes both the INSERT and the SELECT statements... and that it doesn't break all the tests. ...And that we can wrap that transaction into another transaction (transaction inception!).
• For SQLite we'll probably need another function or something to get the same results (the LAST_INSERT_ID function is AFAIK MySQL exclusive)
• Same for SQL Server... although I'm tempted to drop support for that one.
• Do we do something with UUIDs? Those should be super easy to handle, but... yeah, we need to handle those too.
• Are there other types of IDs beyond auto-incremental and UUIDs?

[joelalejandro]

@isBatak do you happen to know if using Prisma would help us to fix this problem?

[isBatak]

@joelalejandro I'm not an expert but createMany is the similar method to insert https://www.prisma.io/docs/reference/api-reference/prisma-client-reference#createmany but it is also not supported by SQLite.

createMany is not supported by SQLite.
Is it possible to detect SQLite and MySQL DB and do a slower approach with multiple inserts inside transaction.

[spersico]

So... checking back into this, I found an update that relates to the issue:
Seems like SQLite has recently added support for the returning syntax in march: Docs
And the SQLite node driver has a PR waiting to be merged, that bumps the SQLite version used in it to the latest version.

So... soon, the only engine that doesn't support returning will be MySQL.

The main issue that we face regarding this, is that any implementation would probably involve creating a transaction, that effectively creates an object, and then pulls the latest data up (something like this). Doing this outside a transaction is a no-go, because AFAIK that's the only way to ensure the atomicity of the operation.

[joelalejandro]

I like the transaction-based approach. I wonder if it would affect the ability to rollback bulk operations.

[joelalejandro]

So, SQLite 3.36 is out, and the PR mentioned by @spersico is now merged. We're waiting on a driver upgrade, but we'll also need to upgrade Knex to remove the .returning() is not supported warning for SQLite.

I've opened an issue here: knex/knex#4766

[joelalejandro]

This continues to happen with MySQL, unfortunately. I've implemented #354 to avoid having a failing response pipeline. At least client-side ID'ed insertions will work. But we'll need a last_inserted_id strategy for this.