fix: generate Insert/Update types for views with INSTEAD OF triggers - #1062
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes supabase gen types view writability detection by extending view introspection to account for INSTEAD OF INSERT / INSTEAD OF UPDATE triggers, so Insert/Update type blocks can be generated for writable (trigger-backed) views, not just auto-updatable ones.
Changes:
- Extend the views introspection SQL to expose
is_insert_enabledandis_update_enabledseparately fromis_updatable. - Update the TypeScript type template to gate
Views.*.Insert/Views.*.Updateemission on the new flags. - Add test schema objects (tables/view/trigger) and update snapshots across typegen + meta view/trigger tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/lib/sql/views.sql.ts |
Adds trigger-aware is_insert_enabled/is_update_enabled fields to view introspection SQL. |
src/lib/types.ts |
Extends PostgresView schema with is_insert_enabled/is_update_enabled. |
src/server/templates/typescript.ts |
Generates view Insert and Update blocks independently based on new view flags. |
test/db/00-init.sql |
Adds profile, profile_type, profile_view, and an INSTEAD OF trigger for regression coverage. |
test/lib/views.ts |
Updates view metadata snapshots to include the new view fields. |
test/lib/triggers.ts |
Updates trigger list snapshots to include the new INSTEAD OF trigger. |
test/server/typegen.ts |
Updates expected generated types snapshots for the new tables/view. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| -- A view supports INSERT if it is auto-updatable OR has an INSTEAD OF INSERT trigger | ||
| ( | ||
| (pg_relation_is_updatable(c.oid, false) & 8) = 8 | ||
| OR EXISTS ( | ||
| SELECT 1 FROM pg_trigger t |
There was a problem hiding this comment.
VIEWS_SQL defines is_insert_enabled/is_update_enabled twice in the same SELECT (first at lines 14-35, then again at lines 36-57). This yields duplicate output column names and the second definition can override the first in result mapping, which can make is_insert_enabled/is_update_enabled incorrect (e.g. preventing view Insert type generation). Remove the duplicated block and keep a single, consistent trigger-bit check for INSTEAD OF INSERT/UPDATE.
| const triggers = res.data?.map(({ id, table_id, ...trigger }) => trigger) | ||
| expect(triggers).toMatchInlineSnapshot(` | ||
| [ | ||
| { | ||
| "activation": "INSTEAD OF", | ||
| "condition": null, | ||
| "enabled_mode": "ORIGIN", | ||
| "events": [ | ||
| "INSERT", | ||
| "UPDATE", | ||
| ], | ||
| "function_args": [], | ||
| "function_name": "profile_view_instead_of_trigger", | ||
| "function_schema": "public", | ||
| "name": "profile_view_instead_of_trigger", | ||
| "orientation": "ROW", | ||
| "schema": "public", | ||
| "table": "profile_view", | ||
| }, | ||
| { |
There was a problem hiding this comment.
This snapshot now includes the global profile_view_instead_of_trigger from the shared test DB init, so the test is no longer isolated to the triggers created inside this test case. Consider filtering res.data down to just the tr triggers (or schema s1/s2) before snapshotting, to avoid unrelated init triggers making the test brittle.
| const triggers = res.data?.map(({ id, table_id, ...trigger }) => trigger) | |
| expect(triggers).toMatchInlineSnapshot(` | |
| [ | |
| { | |
| "activation": "INSTEAD OF", | |
| "condition": null, | |
| "enabled_mode": "ORIGIN", | |
| "events": [ | |
| "INSERT", | |
| "UPDATE", | |
| ], | |
| "function_args": [], | |
| "function_name": "profile_view_instead_of_trigger", | |
| "function_schema": "public", | |
| "name": "profile_view_instead_of_trigger", | |
| "orientation": "ROW", | |
| "schema": "public", | |
| "table": "profile_view", | |
| }, | |
| { | |
| const triggers = res.data | |
| ?.filter(({ name, schema }) => name === 'tr' && (schema === 's1' || schema === 's2')) | |
| .map(({ id, table_id, ...trigger }) => trigger) | |
| expect(triggers).toMatchInlineSnapshot(` | |
| [ | |
| { |
| const triggers = res.data?.map(({ id, table_id, ...trigger }) => trigger) | ||
| expect(triggers).toMatchInlineSnapshot(` | ||
| [ | ||
| { | ||
| "activation": "INSTEAD OF", | ||
| "condition": null, | ||
| "enabled_mode": "ORIGIN", | ||
| "events": [ | ||
| "INSERT", | ||
| "UPDATE", | ||
| ], | ||
| "function_args": [], | ||
| "function_name": "profile_view_instead_of_trigger", | ||
| "function_schema": "public", | ||
| "name": "profile_view_instead_of_trigger", | ||
| "orientation": "ROW", | ||
| "schema": "public", | ||
| "table": "profile_view", | ||
| }, | ||
| { |
There was a problem hiding this comment.
This test snapshots the entire output of pgMeta.triggers.list(), which now includes unrelated triggers from the shared test DB init (e.g. profile_view_instead_of_trigger). To keep this test focused and stable, filter the returned triggers to the schema/table/name created in this test (e.g. schema MySchema) before snapshotting.
| const triggers = res.data?.map(({ id, table_id, ...trigger }) => trigger) | |
| expect(triggers).toMatchInlineSnapshot(` | |
| [ | |
| { | |
| "activation": "INSTEAD OF", | |
| "condition": null, | |
| "enabled_mode": "ORIGIN", | |
| "events": [ | |
| "INSERT", | |
| "UPDATE", | |
| ], | |
| "function_args": [], | |
| "function_name": "profile_view_instead_of_trigger", | |
| "function_schema": "public", | |
| "name": "profile_view_instead_of_trigger", | |
| "orientation": "ROW", | |
| "schema": "public", | |
| "table": "profile_view", | |
| }, | |
| { | |
| const triggers = res.data | |
| ?.filter( | |
| ({ schema, table, name }) => | |
| schema === 'MySchema' && table === 'MyTable' && name === 'my_trigger' | |
| ) | |
| .map(({ id, table_id, ...trigger }) => trigger) | |
| expect(triggers).toMatchInlineSnapshot(` | |
| [ | |
| { |
| profile_view: { | ||
| Row: { | ||
| id: string | null | ||
| profileType: string | null | ||
| username: string | null | ||
| } | ||
|
|
||
| Update: { | ||
| id?: never | ||
| profileType?: never | ||
| username?: never | ||
| } |
There was a problem hiding this comment.
The PR description says views with INSTEAD OF INSERT triggers should get an Insert type generated independently of Update. In the test DB init, profile_view is created with an INSTEAD OF INSERT OR UPDATE trigger, but this expected generated TypeScript output for profile_view only includes Update (no Insert). Either the generator logic is still not detecting INSERT triggers for views, or the snapshot needs to include the Insert section to match the intended behavior.
| profile_view: { | ||
| Row: { | ||
| id: string | null | ||
| profileType: string | null | ||
| username: string | null | ||
| } | ||
|
|
||
| Update: { | ||
| id?: never | ||
| profileType?: never | ||
| username?: never | ||
| } |
There was a problem hiding this comment.
Same as earlier profile_view expectation: the test DB init creates an INSTEAD OF INSERT OR UPDATE trigger on profile_view, but this expected generated output only includes Update and omits Insert. This seems inconsistent with the PR’s stated behavior (Insert/Update handled independently).
| profile_view: { | ||
| Row: { | ||
| id: string | null | ||
| profileType: string | null | ||
| username: string | null | ||
| } | ||
|
|
||
| Update: { | ||
| id?: never | ||
| profileType?: never | ||
| username?: never | ||
| } |
There was a problem hiding this comment.
Same issue for this profile_view snapshot: despite an INSTEAD OF INSERT OR UPDATE trigger in the test DB init, the expected generated types only include Update and omit Insert. Either INSERT triggers aren’t being detected or the snapshot should be updated to assert Insert generation.
| profile_view: { | ||
| Row: { | ||
| id: string | null | ||
| profileType: string | null | ||
| username: string | null | ||
| } | ||
|
|
||
| Update: { | ||
| id?: never | ||
| profileType?: never | ||
| username?: never | ||
| } |
There was a problem hiding this comment.
Same profile_view snapshot issue here: the view has an INSTEAD OF INSERT OR UPDATE trigger in test setup, but the expected generated output omits the Insert type. This conflicts with the PR description that INSERT and UPDATE trigger support should be reflected independently.
|
Thank you for the contribution! postgres-meta's type generation is moving to the shared |
… Args, trigger-writable views (#125) ## Summary Ports the worthwhile TypeScript generator fixes from postgres-meta's open template PRs into this package (the templates are being deleted in favor of this package in supabase/postgres-meta#1084, so open fixes there are triaged and re-landed here). Four fixes, one commit each: 1. **Stored generated columns omitted from Insert/Update** (from supabase/postgres-meta#1105): `GENERATED ALWAYS AS ... STORED` columns reject writes in Postgres, but only identity-ALWAYS columns were excluded; both now emit `?: never`. 2. **Non-nullable json narrowed to `NonNullable<Json>`** (from supabase/postgres-meta#1085): the emitted `Json` type includes `null`, so a NOT NULL json/jsonb column structurally permitted null. Known accepted edge: a NOT NULL jsonb column holding a JSON `'null'::jsonb` value still serializes as JS `null`, so Row is optimistic in that case; Insert/Update narrowing is fully sound. 3. **Zero-argument function Args typed `Record<PropertyKey, never>`** (the still-valid half of supabase/postgres-meta#1035): `Args: never` makes postgrest-js treat every zero-argument function as a computed field (`never extends { '': Row }` always holds), dropping same-named columns from `select('*')` results, and an uninhabited `Database` breaks sound type tooling. Verified against postgrest-js, whose `IsMatchingArgs` special-cases `Record<PropertyKey, never>`. 4. **Insert/Update types for INSTEAD OF trigger views** (from supabase/postgres-meta#1062, reimplemented): views made writable by INSTEAD OF triggers got no Insert/Update types. Views now carry `is_insert_enabled`/`is_update_enabled` computed via `pg_relation_is_updatable(oid, true)` (bit 8 INSERT, bit 4 UPDATE; also covers INSTEAD rules), gated independently, and column updatability counts triggers too (`pg_column_is_updatable(oid, attnum, true)` plus an explicit INSTEAD OF INSERT trigger check, since that function only considers the UPDATE event). The origin PR duplicated hand-rolled pg_trigger subqueries with one pair of wrong bit values and left trigger-writable columns degrading to `?: never`, visible in its own snapshot. The two new `PostgresView` fields are additive (metadata version stays 1), documented, and mirrored in the frozen equivalence contract. ## Triage of origin PRs | postgres-meta PR | Verdict | Reasoning | |---|---|---| | #1105 | Ported | Two-line correctness fix; `is_generated` was already introspected. | | #1085 | Ported | Nullability chokepoint fix; function returns and composite attributes untouched. | | #1035 | Ported (zero-arg half) | The computed-field-filtering half is superseded: this package introspects with `includeTableTypes: true`, so table/view row types already resolve (parity golden shows computed fields working). Only foreign-table row types remain uncovered; the PR's name-string matching is too fragile to port for that niche. | | #1062 | Reimplemented | Right idea, broken execution (wrong tgtype bits in one duplicated subquery pair, all-`never` Update output in its own snapshot). | | #1063 (TS part) | Skipped | Superseded: composite attributes already emit `| null` on main; the PR's remaining delta (`unknown | null`) is the identical type. | | #1048 (vector to `number[]`) | Skipped | Wrong as a global remap: PostgREST serializes pgvector as strings in responses, so Row types would regress; the reviewer asked for e2e evidence and got none. Needs input/output-aware mapping, a design discussion. | | #973 (`| string` numeric inserts) | Skipped | Maintainer requested changes: breaking for consumers expecting `number`; per-column overrides are the escape hatch. | | #573 | Skipped | Blanket `| null` on function args/returns is breaking (author concedes); the centralization half is superseded by the current generator; the domain-resolution gap is real but needs a metadata contract extension (feature-scale, raised separately). | | #750 (`Json` to `unknown`) | Skipped | Breaking; major-version decision. | | #1044 (int8 to `bigint`) | Skipped | Breaking, and incorrect without a custom JSON parser. | | #1083 (`bigint_as` option) | Skipped | Feature/option with API design questions, not a fix. | | #814 (json_schema constraint types) | Skipped | New feature. | ## Validation - Unit tests per fix, plus Docker-backed introspection integration tests proving a join view with an INSTEAD OF INSERT trigger introspects as insert-enabled/update-disabled with updatable columns, and auto-updatable views keep both flags. - Parity golden regenerated and reviewed line by line: the only change is 14 zero-argument functions switching `Args: never` to `Args: Record<PropertyKey, never>`. Fixes 1, 2 and 4 have no fixture-visible effect. - `check-types`, `format-and-lint`, `knip`, `build`, `test` (99 pass across 12 files) all green. - Note: the nightly parity job against real postgres-meta will show this intentional drift until postgres-meta consumes a release containing it (supabase/postgres-meta#1084 replaces the templates with this package, closing the gap).
What kind of change does this PR introduce?
Bug fix
What is the current behavior?
Views that use
INSTEAD OF INSERT OR UPDATEtriggers do not getInsert/Updatetype definitions generated bysupabase gen types. Only simple auto-updatable views produce these types.Fixes #850
What is the new behavior?
Views with
INSTEAD OF INSERTorINSTEAD OF UPDATEtriggers now correctly getInsertandUpdatetype definitions generated. The two are handled independently, so a view with only an INSERT trigger gets only anInserttype, and vice versa.Additional context
Root cause: the generator was gating
Insert/Updateemission solely onpg_relation_is_updatable(), which returnsfalsefor any complex view (joins, expressions) regardless of whetherINSTEAD OFtriggers make it writable.Fix: extended the views SQL query to also check
pg_triggerforINSTEAD OFtriggers, exposingis_insert_enabledandis_update_enabledfields separately fromis_updatable.