Julien-R44
[WIP] feat: add orchid driver
I do not implement createTableIfNotExists, is this necessary?
And I can't setup local test, for my new driver and existing drivers, could you please help me run the test?
Cool! Thanks a lot for the PR dude. I came across Orchid ORM a while back and thought it looked like Lucid with other some cool features
I do not implement createTableIfNotExists, is this necessary?
Yes, bentocache will automatically create the table if autoCreateTable is true. This allows seamless use of Bentocache without having to manually create a migration if you're lazy: https://bentocache.dev/docs/cache-drivers#databases. You can refer to the Knex or Kysely driver, it should be pretty easy to implement
And I can't setup local test, for my new driver and existing drivers, could you please help me run the test?
What's the problem? There's nothing in particular to do other than launch the docker-compose in the repo which starts a postgres. then launch the tests. But let me know if anything else is giving you trouble
Looks we have some lint issues too
@Julien-R44 lint error and peerDependencies has been fixed.
As for createTableIfNotExists, I do not find createTable related method from the #connection, Orchid ORM has a separate package rake-db for migrations and table creations, but it's very heavy to import it into runtime, so I don't want to introduce it only for createTable.
@romeerez Hi, does orchid-orm package provide some lightweight way to createTable
@bingtsingw it's very nice to see this PR!
I'm seeing this Bentocache library for the first time and I'm not sure yet what exactly is required from a db driver.
In some cases, it can be enough to just write a bit of custom SQL to create a table. But looks like in this case, it should happen dynamically.
Imagine that OrchidORM has an exported separate function createTable. But how about changing a table? Wouldn't you need much more functionality to add/drop/change/rename columns? No way there is no need to ever change a table.
So I'll look into Bentocache to see what should be changed in Orchid to support it.
We could do a dynamic import of rake-db, but if it is that heavy, then yeah we can probably do without it: it's not dramatic. You can throw an error if the createTableIfNotExists function is called with Orchid driver, saying "This Feature is not supported with OrchidORM. You must manually create your cache table" something like that and that should be okay for now
Imagine that OrchidORM has an exported separate function createTable. But how about changing a table? Wouldn't you need much more functionality to add/drop/change/rename columns? No way there is no need to ever change a table.
In fact we don't really have that need in Bentocache. Bentocache just expects a "cache" table with key,value, and expires_at. If you need to change the table, then yes, traditional migrations are recommended. See the knex driver as reference: https://github.com/Julien-R44/bentocache/blob/main/packages/bentocache/src/drivers/database/adapters/knex.ts#L64
@romeerez Thank you for your support, in Bentocache, it has an autoCreateTable option which needs to implement createTableIfNotExists, this is the Kysely version of implementation:
async createTableIfNotExists(): Promise<void> {
await this.#connection.schema
.createTable(this.#tableName)
.addColumn('key', 'varchar(255)', (col) => col.primaryKey().notNull())
.addColumn('value', 'text')
.addColumn('expires_at', 'bigint')
.ifNotExists()
.execute()
}
There is no need to change it in this situation.
@Julien-R44 Thank you for your explain, I thought it was a must. @romeerez Seems there is no need to add extra methods, I'll test if it works
@bingtsingw I have looked around, and, well, any ORM is an overkill for this case, even Knex and Kysely are overkill, since Bentocache requires only limited functionality from a driver, it should be easy enough and better for performance to do all queries (to create a table, get value, set value) with raw SQL by calling pg or postgres libs directly.
ORM has a ton of additional functionality that won't be used: relations management, various features like callbacks on create/update/delete, deletedAt, and many more. Query builder also has a ton of functionality that is redundant for this: various ways of doing joins, and wrappers for various SQL statements.
@romeerez Yes, in fact ORM is overkill for this tiny cache table, that's why I do not use Knex or Kysely and want to implement an orchid driver, cause I already use it in my project, making it a driver of bentocache is natural.
Aha, yes that makes sense.
In such case, IMO it would be better for Bentocache to have a pg lib adapter, then it could work with any ORM that depends on pg driver.
You could extract pg.Pool from the db instance and the Bentocache pg driver would use it for performing queries directly:
const db = orchidORM({ ...options }, { ...tables })
// extract pg.Pool from db instance:
const pgPool = db.$adapter.pool
const value = await pgPool.query('SELECT value FROM cache WHERE key = ?', ['key'])
@bingtsingw I have looked around, and, well, any ORM is an overkill for this case, even Knex and Kysely are overkill, since Bentocache requires only limited functionality from a driver, it should be easy enough and better for performance to do all queries (to create a table, get value, set value) with raw SQL by calling
pgorpostgreslibs directly.ORM has a ton of additional functionality that won't be used: relations management, various features like callbacks on create/update/delete,
deletedAt, and many more. Query builder also has a ton of functionality that is redundant for this: various ways of doing joins, and wrappers for various SQL statements.
Yeah, this is actually something I've hesitated about on bentocache. Do I create adapter for pg, sqlite, @libsql/client etc rather than adapters for ORMs. In the end, I preferred to use ORMs because there are almost more dialects than ORMs. So it seemed simpler to me to maintain the adapters of the most popular ORMs.
For example, just for Postgres, I would have had to make adapters for @neondatabase/serverless, @xata.io/client, pg, postgres, node-postgres etc... Where I can just add a Drizzle Adapter that will do all this work for me
I hope I didn't make a mistake :sweat_smile:
btw @romeerez, I've just had a closer look, sorry about the comparison with Lucid ORM, Orchid ORM has nothing to do with it. I got it confused with https://sutando.org/guide/getting-started.html which indeed look similar to Lucid
Orchid ORM looks really cool, well done for the work !!
Thank you!
It's indeed similar to Lucid, since Lucid is on Knex, and Orchid is syntactically inspired by Knex to be familiar for node.js devs.
Lucid's creator posted some interesting notes about reaching a good type-safety level with ORMs, and this is the exact main goal of Orchid ORM to reach.
@Julien-R44 Hi, I setup the test, seems it fails with expires_at column type.
Weird. I'm not sure why. The tests run fine in CI though: https://github.com/Julien-R44/bentocache/actions/runs/8880648142/job/24383374119?pr=17#step:7:128 So i would say, probably a configuration issue on your side
Maybe you can ignore the local execution of the knex tests. Just run the tests you added on Orchid. Or even let the CI execute them
Awesome. thanks a lot for taking the time to create this PR. I will give a final look at it and merge it before the end of the week
Some tests did not pass, I took a look at the error messages, not sure if it's related to orchid driver
We have some flakky test in the suite. It was one of them 😄 All green now
Okay that looks perfect to me. Let's merge it. Thanks again for the PR !