Skip to content

Commit a00104c

Browse files
authored
copy triggers in sqlite table recreation (#39)
### Description of change Copies triggers when recreating a table in SQLite. Previously, any DSL migration operations that resulted in recreating the table would drop the triggers, because they weren't being properly copied over. Fixes https://linear.app/n8n/issue/CAT-1709. Verified in n8n. ### Pull-Request Checklist <!-- Please make sure to review and check all of the following. If an item is not applicable, you can add "N/A" to the end. --> - [ ] Code is up-to-date with the `master` branch - [ ] `npm run format` to apply prettier formatting - [ ] `npm run test` passes with this change - [ ] This pull request links relevant issues as `Fixes #0000` - [ ] There are new or updated unit tests validating the change - [ ] Documentation has been updated to reflect this change - [ ] The new commits follow conventions explained in [CONTRIBUTING.md](https://github.com/typeorm/typeorm/blob/master/CONTRIBUTING.md) <!-- 🎉 Thank you for contributing and making TypeORM even better! -->
1 parent 4d02c94 commit a00104c

File tree

2 files changed

+67
-0
lines changed

2 files changed

+67
-0
lines changed

src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2005,6 +2005,14 @@ export abstract class AbstractSqliteQueryRunner
20052005
const upQueries: Query[] = []
20062006
const downQueries: Query[] = []
20072007

2008+
// query triggers from old table before recreation
2009+
const [, tableNameOldInitial] = this.splitTablePath(oldTable.name)
2010+
const triggerQuery = `SELECT name, sql FROM sqlite_master WHERE type = 'trigger' AND tbl_name = ?`
2011+
const triggers: { name: string; sql: string }[] = await this.query(
2012+
triggerQuery,
2013+
[tableNameOldInitial],
2014+
)
2015+
20082016
// drop old table indices
20092017
oldTable.indices.forEach((index) => {
20102018
upQueries.push(this.dropIndexSql(index))
@@ -2112,6 +2120,14 @@ export abstract class AbstractSqliteQueryRunner
21122120
downQueries.push(this.dropIndexSql(index))
21132121
})
21142122

2123+
// recreate table triggers
2124+
triggers.forEach((trigger) => {
2125+
// upQueries: recreate triggers on the new table (forward migration)
2126+
upQueries.push(new Query(trigger.sql))
2127+
// downQueries: recreate triggers on the old table (rollback)
2128+
downQueries.push(new Query(trigger.sql))
2129+
})
2130+
21152131
// update generated columns in "typeorm_metadata" table
21162132
// Step 1: clear data for removed generated columns
21172133
oldTable.columns

test/functional/query-runner/drop-column.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,57 @@ describe("query runner > drop column", () => {
111111
))
112112
})
113113

114+
it("should preserve triggers when recreating table in sqlite", () =>
115+
Promise.all(
116+
connections
117+
// Only run this test for sqlite-based connections
118+
.filter(
119+
(connection) =>
120+
connection.options.type === "sqlite" ||
121+
connection.options.type === "sqlite-pooled",
122+
)
123+
.map(async (connection) => {
124+
const queryRunner = connection.createQueryRunner()
125+
126+
// Get the post table
127+
let table = await queryRunner.getTable("post")
128+
expect(table).to.exist
129+
130+
// Create a trigger on the table
131+
const triggerName = "test_insert_trigger"
132+
const triggerSql = `CREATE TRIGGER ${triggerName} AFTER INSERT ON "${table!.name}" BEGIN SELECT 1; END`
133+
await queryRunner.query(triggerSql)
134+
135+
// Verify trigger exists before column drop
136+
const triggersBefore: { name: string; sql: string }[] =
137+
await queryRunner.query(
138+
`SELECT name, sql FROM sqlite_master WHERE type = 'trigger' AND tbl_name = ?`,
139+
[table!.name],
140+
)
141+
expect(triggersBefore).to.have.length(1)
142+
expect(triggersBefore[0].name).to.equal(triggerName)
143+
144+
// Drop a column, which triggers recreateTable in SQLite
145+
await queryRunner.dropColumn(table!, "version")
146+
147+
// Verify trigger still exists after table recreation
148+
table = await queryRunner.getTable("post")
149+
const triggersAfter: { name: string; sql: string }[] =
150+
await queryRunner.query(
151+
`SELECT name, sql FROM sqlite_master WHERE type = 'trigger' AND tbl_name = ?`,
152+
[table!.name],
153+
)
154+
expect(triggersAfter).to.have.length(1)
155+
expect(triggersAfter[0].name).to.equal(triggerName)
156+
expect(triggersAfter[0].sql).to.equal(triggerSql)
157+
158+
// Clean up
159+
await queryRunner.query(`DROP TRIGGER ${triggerName}`)
160+
await queryRunner.executeMemoryDownSql()
161+
await queryRunner.release()
162+
}),
163+
))
164+
114165
it("should safely handle SQL injection in hasEnumType", () =>
115166
Promise.all(
116167
connections

0 commit comments

Comments
 (0)