Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for CONSTRAINT TRIGGER syntax in PostgreSQL schema management. CONSTRAINT TRIGGERs are a special type of trigger that can be deferred until the end of a transaction and are primarily used for constraint enforcement.
- Adds new fields to the Trigger struct to track constraint trigger properties (IsConstraint, Deferrable, InitiallyDeferred)
- Updates SQL generation to handle CREATE CONSTRAINT TRIGGER syntax with proper deferrable clauses
- Modifies trigger comparison and diff handling to account for constraint trigger properties
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 |
|---|---|
| ir/ir.go | Adds constraint trigger fields to Trigger struct |
| ir/parser.go | Parses constraint trigger properties from pg_query AST |
| internal/diff/trigger.go | Updates trigger comparison and SQL generation for constraint triggers |
| internal/diff/table.go | Handles constraint trigger modifications using DROP/CREATE instead of CREATE OR REPLACE |
| ir/queries/queries.sql | Adds COALESCE handling for null column values in constraint queries |
| ir/queries/queries.sql.go | Generated Go code for the SQL query changes |
| testdata/* | Test files demonstrating constraint trigger functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // Use CREATE OR REPLACE for modified triggers | ||
| sql := generateTriggerSQLWithMode(triggerDiff.New, targetSchema) | ||
| // Constraint triggers don't support CREATE OR REPLACE, so we need to DROP and CREATE | ||
| if triggerDiff.New.IsConstraint { |
There was a problem hiding this comment.
This logic only checks if the new trigger is a constraint trigger, but should also handle the case where the old trigger was a constraint trigger but the new one isn't. Both scenarios require DROP and CREATE since constraint triggers don't support CREATE OR REPLACE.
| if triggerDiff.New.IsConstraint { | |
| if triggerDiff.New.IsConstraint || triggerDiff.Old.IsConstraint { |
| Source: triggerDiff, | ||
| CanRunInTransaction: true, | ||
| // Step 1: DROP the old trigger | ||
| dropSQL := fmt.Sprintf("DROP TRIGGER IF EXISTS %s ON %s;", triggerDiff.Old.Name, tableName) |
There was a problem hiding this comment.
For constraint triggers, the DROP statement should use 'DROP TRIGGER' without 'IF EXISTS' since we know the trigger exists (it's being modified). Additionally, constraint triggers may need special handling in the DROP statement.
| dropSQL := fmt.Sprintf("DROP TRIGGER IF EXISTS %s ON %s;", triggerDiff.Old.Name, tableName) | |
| dropSQL := fmt.Sprintf("DROP TRIGGER %s ON %s;", triggerDiff.Old.Name, tableName) |
1 participant
Fix #60