Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for PostgreSQL trigger transition tables by implementing the REFERENCING OLD TABLE and NEW TABLE clauses. This allows triggers to access all modified rows as table references rather than individual row references.
- Adds parsing and storage of transition table references (OLD TABLE AS and NEW TABLE AS) in trigger definitions
- Updates database queries to retrieve transition table names from the PostgreSQL system catalog
- Generates proper SQL DDL with REFERENCING clauses for triggers that use transition tables
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| testdata/diff/create_trigger/add_trigger_old_table/* | Test data files demonstrating trigger creation with OLD TABLE reference |
| ir/queries/queries.sql.go | Updates generated query code to include old_table and new_table fields |
| ir/queries/queries.sql | Modified SQL query to extract transition table names from pg_trigger catalog |
| ir/parser.go | Added parsing logic for REFERENCING OLD/NEW TABLE clauses in CREATE TRIGGER statements |
| ir/ir.go | Added OldTable and NewTable fields to the Trigger struct |
| ir/inspector.go | Enhanced trigger building to set transition table names from catalog data |
| internal/diff/trigger.go | Updated trigger comparison and SQL generation to handle REFERENCING clauses |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| ColumnName sql.NullString `db:"column_name" json:"column_name"` | ||
| OrdinalPosition sql.NullInt32 `db:"ordinal_position" json:"ordinal_position"` |
There was a problem hiding this comment.
The change from string/int16 to sql.NullString/sql.NullInt32 for ColumnName and OrdinalPosition appears unrelated to the trigger feature and should be in a separate commit or PR for better change isolation.
| ColumnName sql.NullString `db:"column_name" json:"column_name"` | |
| OrdinalPosition sql.NullInt32 `db:"ordinal_position" json:"ordinal_position"` | |
| ColumnName string `db:"column_name" json:"column_name"` | |
| OrdinalPosition int16 `db:"ordinal_position" json:"ordinal_position"` |
| if triggerRow.OldTable != "" { | ||
| trigger.OldTable = triggerRow.OldTable | ||
| } | ||
| if triggerRow.NewTable != "" { |
There was a problem hiding this comment.
The transition table assignment logic overwrites parsed values unconditionally. This could cause issues when the catalog data is empty but the parser correctly extracted values from DDL. Consider only setting these values when they're currently empty in the trigger object.
| if triggerRow.OldTable != "" { | |
| trigger.OldTable = triggerRow.OldTable | |
| } | |
| if triggerRow.NewTable != "" { | |
| if triggerRow.OldTable != "" && trigger.OldTable == "" { | |
| trigger.OldTable = triggerRow.OldTable | |
| } | |
| if triggerRow.NewTable != "" && trigger.NewTable == "" { |
Fix #59