Missing features and suggestions

[B4nan]

Hi there, I've started working on oracledb support for MikroORM v7, which uses kysely for query execution. Initially I wanted to use this dialect, but I've hit a few roadblocks, so ended up inlining the code and modifying it. I would very much like to get my changes upstream so I can use this package instead, so wanted to get your opinions on those.

1. dependencies - first meta problem is that your package depends on prettier and uuid, neither of those seems important, prettier is surely a dev dependency for most of us, and I don't think you need the uuid package either (a simple bigint counter would do the same job without any deps, and it would be more performant, or you could go with randomUUID from crypto package). prettier is a bigger problem, since its a fat dependency (7.5mb) that I dont think anybody wants in production builds. Maybe the type generation should be a separate package, or at least the dependency can be made optional and accessed via dynamic imports.
2. dependency on kysely is still on v0.27, the code seems to work fine with v0.28 so likely just a dated peer dependency constraint.
3. out bindings - this is a big one for me, I need returning statements to work seamlessly. I ended up hacking it in a few places so I can pass the outbindings down the line. I've used the regular query parameters
4. savepoints - this is completely missing now, should be trivial to implement
5. isolation levels and readonly access mode, also should be trivial
6. autocommit - the way you have it now, you only allow controlling this globally, which is not very useful. I need to enable autocommit only for specific queries that are not executed inside a transaction. I ended up passing this information as part of the compiled query.
7. map of open connections, is that really needed? feels unnecessary, for the code you have, it should be enough to have a Set, and I am not sold that you need that either, having the connection instance should be enough to release it.
8. ...probably more, but those are the important parts I recall now, will update later if I remember some other changes I had to do. The driver I work on is still in early stages and doesnt pass many tests.Note that I am only using kysely as an execution layer, I dont care about query building, I got all of that implemented natively in the ORM, so I am not asking for a native returning support, I just need a way to pass the outbindings down and get them back.

The modified code is currently at https://github.com/mikro-orm/mikro-orm/blob/oracledb/packages/knex/src/dialects/oracledb/OracleDialect.ts.

I can send a PR for savepoints and isolation levels right ahead, already got those working.

[JosephTuffin]

Hi there @B4nan, sorry it's taken a while for me to get back to you, but thank you for the suggestions.

Most of these should be fairly easy to address. I've just merged in a PR for save points which also bumps the version of the Kysely dependency to v0.28.

If you have a PR for the isolation levels already that would be great. Also, if you want to look at adding support for out binds that would be helpful as it isn't something I had planned on doing since it sits outside the scope of query building.

I'll look to address the rest of the comments in the next couple of weeks.

[JosephTuffin]

@B4nan, I've made a PR to hopefully address most of your points: #92.

I've just added the out binds to the return of the execute query method. Let me know if that is enough for what you need or if there are further changes required. I'm not sure how feasible it is to implement this natively in Kysely though.