From 7ce79ca8d6071a9fdbee867b0171aa3e18314261 Mon Sep 17 00:00:00 2001 From: Luke Policinski Date: Fri, 2 Oct 2026 15:23:07 -0400 Subject: [PATCH 1/2] bug: re-apply views and triggers together when a view changes Boot skips any file whose digest is unchanged. Recreating a view drops the triggers on it (v_pool_maps lost its INSTEAD OF triggers, so every map-pool insert failed) and, with CASCADE, the views built on it (player_perf_career takes player_performance_v), while the files defining those still matched. A change to any view file now re-applies every view and trigger file, in the same order as a fresh install. --- src/hasura/hasura.service.ts | 39 +++++++++++---- test/view-reapply.spec.ts | 93 ++++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 9 deletions(-) create mode 100644 test/view-reapply.spec.ts diff --git a/src/hasura/hasura.service.ts b/src/hasura/hasura.service.ts index 935b3177..35a60490 100644 --- a/src/hasura/hasura.service.ts +++ b/src/hasura/hasura.service.ts @@ -135,8 +135,13 @@ export class HasuraService { await this.apply(path.resolve("./hasura/enums")); await this.apply(path.resolve("./hasura/functions")); - await this.apply(path.resolve("./hasura/views")); - await this.apply(path.resolve("./hasura/triggers")); + + // Recreating a view drops the triggers on it and, with CASCADE, the views + // built on it, while the files defining those still match their digests. + const views = path.resolve("./hasura/views"); + const viewsChanged = await this.hasChanges(views); + await this.apply(views, viewsChanged); + await this.apply(path.resolve("./hasura/triggers"), viewsChanged); await this.updateSettings(); @@ -401,13 +406,13 @@ export class HasuraService { return versions; } - public async apply(filePath: string): Promise { + public async apply(filePath: string, force = false): Promise { const filePathStats = fs.statSync(filePath); if (filePathStats.isDirectory()) { const files = fs.readdirSync(filePath).sort(); for (const file of files) { - await this.apply(path.join(filePath, file)); + await this.apply(path.join(filePath, file), force); } return; } @@ -416,12 +421,9 @@ export class HasuraService { const sql = fs.readFileSync(filePath, "utf8"); const digest = this.calcSqlDigest(sql); - const setting = path.relative( - process.cwd(), - filePath.replace(".sql", ""), - ); + const setting = this.digestName(filePath); - if (digest === (await this.getSetting(setting))) { + if (!force && digest === (await this.getSetting(setting))) { return; } @@ -440,6 +442,25 @@ export class HasuraService { } } + private async hasChanges(filePath: string): Promise { + if (fs.statSync(filePath).isDirectory()) { + for (const file of fs.readdirSync(filePath).sort()) { + if (await this.hasChanges(path.join(filePath, file))) { + return true; + } + } + return false; + } + + const digest = this.calcSqlDigest(fs.readFileSync(filePath, "utf8")); + + return digest !== (await this.getSetting(this.digestName(filePath))); + } + + private digestName(filePath: string) { + return path.relative(process.cwd(), filePath.replace(".sql", "")); + } + public async getSetting(name: string) { try { const [data] = await this.postgresService.query< diff --git a/test/view-reapply.spec.ts b/test/view-reapply.spec.ts new file mode 100644 index 00000000..64e8a609 --- /dev/null +++ b/test/view-reapply.spec.ts @@ -0,0 +1,93 @@ +import { PostgresService } from "./../src/postgres/postgres.service"; +import { bootMigratedDb, SqlTestDb } from "./utils/sql-test-db"; + +// setup() skips a boot-phase file whose digest is unchanged. Recreating a view +// drops the triggers on it and, with CASCADE, the views built on it, while the +// files that define those still match their digests. +describe("re-applying a view at boot (SQL-driven)", () => { + let db: SqlTestDb; + let postgres: PostgresService; + + beforeAll(async () => { + db = await bootMigratedDb("ViewReapplyTest"); + postgres = db.postgres; + }, 600_000); + + afterAll(async () => { + await db?.stop(); + }); + + const changeView = async (file: string) => { + const [row] = await postgres.query>( + `UPDATE migration_hashes.hashes SET hash = 'changed' + WHERE name = $1 RETURNING name`, + [`hasura/views/${file}`], + ); + expect(row?.name).toBe(`hasura/views/${file}`); + }; + + const triggersOn = async (relation: string) => + ( + await postgres.query>( + `SELECT t.tgname FROM pg_trigger t + JOIN pg_class c ON c.oid = t.tgrelid + WHERE c.relname = $1 AND NOT t.tgisinternal + ORDER BY t.tgname`, + [relation], + ) + ).map(({ tgname }) => tgname); + + const viewExists = async (name: string) => { + const [{ exists }] = await postgres.query>( + "SELECT to_regclass($1) IS NOT NULL AS exists", + [`public.${name}`], + ); + return exists; + }; + + it("puts back the INSTEAD OF triggers on v_pool_maps, so a pool insert still works", async () => { + const triggers = ["td_v_pool_maps", "ti_v_pool_maps", "tu_v_pool_maps"]; + expect(await triggersOn("v_pool_maps")).toEqual(triggers); + + await changeView("v_pool_maps"); + await db.hasura.setup(); + + expect(await triggersOn("v_pool_maps")).toEqual(triggers); + + const [pool] = await postgres.query>( + "INSERT INTO map_pools (type) VALUES ('Custom') RETURNING id", + ); + const [map] = await postgres.query>( + "SELECT id FROM maps WHERE deleted_at IS NULL ORDER BY name LIMIT 1", + ); + await postgres.query( + "INSERT INTO v_pool_maps (map_pool_id, id) VALUES ($1, $2)", + [pool.id, map.id], + ); + + const [{ count }] = await postgres.query>( + "SELECT count(*)::int AS count FROM _map_pool WHERE map_pool_id = $1", + [pool.id], + ); + expect(count).toBe(1); + }); + + it("puts back the views a DROP VIEW ... CASCADE took with it", async () => { + expect(await viewExists("player_performance_v")).toBe(true); + + await changeView("v_player_perf_career"); + await db.hasura.setup(); + + expect(await viewExists("player_career_stats_v")).toBe(true); + expect(await viewExists("player_performance_v")).toBe(true); + }); + + it("re-applies nothing when no file changed", async () => { + const setSetting = jest.spyOn(db.hasura, "setSetting"); + + await db.hasura.setup(); + + expect(setSetting).not.toHaveBeenCalled(); + setSetting.mockRestore(); + }); +}); From a80b0c8922dfffb461ff4a526991229ab6ecb29a Mon Sep 17 00:00:00 2001 From: Luke Policinski Date: Fri, 2 Oct 2026 15:59:42 -0400 Subject: [PATCH 2/2] bug: forget view and INSTEAD OF trigger digests instead of forcing passes Forcing every view and trigger file wrote the view digests before the trigger pass, so a boot that died in between never restored the v_pool_maps triggers, and it re-ran all 68 trigger files (table locks, the event_match_links backfill) on any view change. A view change now deletes the digests of every view file and of the trigger files that create INSTEAD OF triggers; each file writes its digest back only once it applied, so an interrupted boot is finished by the next one and table trigger files keep their normal digest check. --- src/hasura/hasura.service.ts | 65 ++++++++++++++++++++++++++---------- test/view-reapply.spec.ts | 55 ++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 17 deletions(-) diff --git a/src/hasura/hasura.service.ts b/src/hasura/hasura.service.ts index 35a60490..e9598797 100644 --- a/src/hasura/hasura.service.ts +++ b/src/hasura/hasura.service.ts @@ -136,12 +136,24 @@ export class HasuraService { await this.apply(path.resolve("./hasura/enums")); await this.apply(path.resolve("./hasura/functions")); - // Recreating a view drops the triggers on it and, with CASCADE, the views - // built on it, while the files defining those still match their digests. const views = path.resolve("./hasura/views"); - const viewsChanged = await this.hasChanges(views); - await this.apply(views, viewsChanged); - await this.apply(path.resolve("./hasura/triggers"), viewsChanged); + const triggers = path.resolve("./hasura/triggers"); + + // Recreating a view drops the INSTEAD OF triggers on it and, with CASCADE, + // the views built on it, while the files that define those still match + // their digests. Forgetting those digests up front also leaves them for the + // next boot when this one dies part way through. + if (await this.hasChanges(views)) { + await this.forgetDigests([ + ...this.sqlFiles(views), + ...this.sqlFiles(triggers).filter((file) => + HasuraService.createsViewTriggers(fs.readFileSync(file, "utf8")), + ), + ]); + } + + await this.apply(views); + await this.apply(triggers); await this.updateSettings(); @@ -406,13 +418,13 @@ export class HasuraService { return versions; } - public async apply(filePath: string, force = false): Promise { + public async apply(filePath: string): Promise { const filePathStats = fs.statSync(filePath); if (filePathStats.isDirectory()) { const files = fs.readdirSync(filePath).sort(); for (const file of files) { - await this.apply(path.join(filePath, file), force); + await this.apply(path.join(filePath, file)); } return; } @@ -423,7 +435,7 @@ export class HasuraService { const digest = this.calcSqlDigest(sql); const setting = this.digestName(filePath); - if (!force && digest === (await this.getSetting(setting))) { + if (digest === (await this.getSetting(setting))) { return; } @@ -442,19 +454,38 @@ export class HasuraService { } } - private async hasChanges(filePath: string): Promise { - if (fs.statSync(filePath).isDirectory()) { - for (const file of fs.readdirSync(filePath).sort()) { - if (await this.hasChanges(path.join(filePath, file))) { - return true; - } + private sqlFiles(dir: string): Array { + return fs + .readdirSync(dir) + .sort() + .flatMap((file) => { + const filePath = path.join(dir, file); + return fs.statSync(filePath).isDirectory() + ? this.sqlFiles(filePath) + : [filePath]; + }); + } + + private async hasChanges(dir: string): Promise { + for (const file of this.sqlFiles(dir)) { + const digest = this.calcSqlDigest(fs.readFileSync(file, "utf8")); + if (digest !== (await this.getSetting(this.digestName(file)))) { + return true; } - return false; } + return false; + } - const digest = this.calcSqlDigest(fs.readFileSync(filePath, "utf8")); + private async forgetDigests(files: Array) { + await this.postgresService.query( + "delete from migration_hashes.hashes where name = any($1::text[])", + [files.map((file) => this.digestName(file))], + ); + } - return digest !== (await this.getSetting(this.digestName(filePath))); + // Postgres only allows INSTEAD OF triggers on views. + private static createsViewTriggers(sql: string) { + return /\bINSTEAD\s+OF\s+(INSERT|UPDATE|DELETE)\b/i.test(sql); } private digestName(filePath: string) { diff --git a/test/view-reapply.spec.ts b/test/view-reapply.spec.ts index 64e8a609..3f82081f 100644 --- a/test/view-reapply.spec.ts +++ b/test/view-reapply.spec.ts @@ -82,6 +82,61 @@ describe("re-applying a view at boot (SQL-driven)", () => { expect(await viewExists("player_performance_v")).toBe(true); }); + const failOnce = (part: string) => { + const apply = db.hasura.apply.bind(db.hasura); + const spy = jest + .spyOn(db.hasura, "apply") + .mockImplementation(async (...args: Parameters) => { + if (args[0].includes(part)) { + spy.mockRestore(); + throw new Error("boot died"); + } + return apply(...args); + }); + }; + + it("restores the v_pool_maps triggers on the next boot when a boot dies before the trigger pass", async () => { + await changeView("v_pool_maps"); + failOnce("/hasura/triggers"); + + await expect(db.hasura.setup()).rejects.toThrow("boot died"); + expect(await triggersOn("v_pool_maps")).toEqual([]); + + await db.hasura.setup(); + + expect(await triggersOn("v_pool_maps")).toEqual([ + "td_v_pool_maps", + "ti_v_pool_maps", + "tu_v_pool_maps", + ]); + }); + + it("restores a CASCADE-dropped view on the next boot when a boot dies before re-creating it", async () => { + await changeView("v_player_perf_career"); + failOnce("/hasura/views/v_player_perf_ratings.sql"); + + await expect(db.hasura.setup()).rejects.toThrow("boot died"); + expect(await viewExists("player_performance_v")).toBe(false); + + await db.hasura.setup(); + + expect(await viewExists("player_performance_v")).toBe(true); + }); + + it("leaves the table trigger files alone when a view changes", async () => { + const setSetting = jest.spyOn(db.hasura, "setSetting"); + + await changeView("v_pool_maps"); + await db.hasura.setup(); + + const triggerFiles = setSetting.mock.calls + .map(([name]) => name) + .filter((name) => name.startsWith("hasura/triggers/")); + setSetting.mockRestore(); + + expect(triggerFiles).toEqual(["hasura/triggers/v_pool_maps"]); + }); + it("re-applies nothing when no file changed", async () => { const setSetting = jest.spyOn(db.hasura, "setSetting");