Add support for `CREATE TRIGGER` and `DROP TRIGGER` statement
This PR adds support for CREATE TRIGGER and DROP TRIGGER statement.
Supported dialects:
- [x] SQLite
- [x] PostgreSQL
- [x] MySQL
There are currently no tests for this (which is why it's in draft status) but it is otherwise ready so feel free to review & suggest changes.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| kysely | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Dec 18, 2023 10:02am |
Sadly, SchemaModule and it's functionality were left contextless on purpose:
- Migrations should be frozen in time. You cannot simply alter DB context between migration files and within them, and expect it to be compile-time performant over time.
- It doesn't make sense when using codegen solutions (which is the recommended way of providing types to Kysely), since these tools only see the current DB state, and they overwrite any older DB type left from previous runs.
I think this API would be useful even without making SchemaModule contextful. However, there's also temporary triggers (that are recreated on every connection to the Database) so having type-safety helps prevent critical issues in a seamless way. In my case, I am using temporary triggers which aren't part of the migration process so having type-safety there does make sense.
I do agree with you regarding "migrations should be frozen in time" but I think the correct way to do this would be to also freeze the types per schema version. It will be a bit cumbersome but it will ensure that no one accidentally breaks a migration. Of course, if someone uses codegen solutions this would be automatically handled(?). However, not everyone uses codegen solutions and not every case can be fulfilled via codegen solutions.
In my experience, having bulletproof migrations are critical in almost every SQL based application, and using types to ensure that would be extremely helpful.
Wish this could be discussed before you going all in. 😢
I needed this functionality for our own project, internally, so I am just upstreaming this. Currently, I am using these changes via patch files so if you guys have different plans, I have no problem closing this PR. I would, of course, love to have this merged after the necessary changes have been made.
The API itself might still be salvageable without type-safety and duplicating
QueryCreator(e.g. accepting expressions or query builders as arguments instead), based on usefulness.
Agreed.