Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion internal/diff/trigger.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,16 @@ func triggersEqual(old, new *ir.Trigger) bool {
}
}

// Compare update columns
if len(old.UpdateColumns) != len(new.UpdateColumns) {
return false
}
for i, col := range old.UpdateColumns {
if col != new.UpdateColumns[i] {
return false
}
}
Comment on lines +49 to +57
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Order-sensitive UpdateColumns comparison may produce spurious diffs

normalizeTriggerEvents sorts events into a canonical order to prevent false positives when the same set of events is listed differently in two IRs. UpdateColumns has no analogous normalization: if the same columns are listed in a different order (e.g., UPDATE OF a, b vs UPDATE OF b, a) — both semantically identical in PostgreSQL — triggersEqual returns false and an unnecessary ALTER TRIGGER is emitted.

Consider either sorting UpdateColumns in normalizeTrigger (same file as normalizeTriggerEvents), or doing a set-comparison here:

// Compare update columns (order-independent)
if len(old.UpdateColumns) != len(new.UpdateColumns) {
    return false
}
oldCols := make(map[string]struct{}, len(old.UpdateColumns))
for _, col := range old.UpdateColumns {
    oldCols[col] = struct{}{}
}
for _, col := range new.UpdateColumns {
    if _, ok := oldCols[col]; !ok {
        return false
    }
}


// Compare constraint trigger properties
if old.IsConstraint != new.IsConstraint {
return false
Expand Down Expand Up @@ -215,7 +225,11 @@ func generateTriggerSQLWithMode(trigger *ir.Trigger, targetSchema string) string
for _, orderEvent := range eventOrder {
for _, triggerEvent := range trigger.Events {
if triggerEvent == orderEvent {
events = append(events, string(triggerEvent))
if triggerEvent == ir.TriggerEventUpdate && len(trigger.UpdateColumns) > 0 {
events = append(events, "UPDATE OF "+strings.Join(trigger.UpdateColumns, ", "))
} else {
events = append(events, string(triggerEvent))
}
break
}
}
Expand Down
66 changes: 66 additions & 0 deletions ir/inspector.go
Original file line number Diff line number Diff line change
Expand Up @@ -1452,6 +1452,65 @@ func extractWhenClauseFromTriggerDef(triggerDef string) string {
return strings.TrimSpace(triggerDef[start:end])
}

// extractUpdateColumnsFromTriggerDef extracts column names from UPDATE OF col1, col2 in a trigger definition
// returned by pg_get_triggerdef(). The format is:
// "CREATE TRIGGER name BEFORE INSERT OR UPDATE OF col1, col2 ON table ..."
func extractUpdateColumnsFromTriggerDef(triggerDef string) []string {
upper := strings.ToUpper(triggerDef)
updateOfIdx := strings.Index(upper, "UPDATE OF ")
if updateOfIdx == -1 {
return nil
}

// Start after "UPDATE OF "
start := updateOfIdx + len("UPDATE OF ")

// Find " ON " which terminates the event/column list
onIdx := strings.Index(upper[start:], " ON ")
if onIdx == -1 {
return nil
}

// Extract the column list substring
colListStr := strings.TrimSpace(triggerDef[start : start+onIdx])

// Handle potential " OR " separating additional events after the columns
// e.g., "UPDATE OF col1, col2 OR DELETE ON ..."
// We need to check if there's an OR followed by another event keyword
eventKeywords := []string{"INSERT", "UPDATE", "DELETE", "TRUNCATE"}
parts := strings.Split(colListStr, " OR ")
// Only keep the first part (column list); the rest would be other events
if len(parts) > 1 {
// Check if subsequent parts are event keywords
for i := 1; i < len(parts); i++ {
trimmed := strings.TrimSpace(strings.ToUpper(parts[i]))
isEvent := false
for _, kw := range eventKeywords {
if trimmed == kw || strings.HasPrefix(trimmed, kw+" ") {
isEvent = true
break
}
}
if isEvent {
colListStr = strings.TrimSpace(parts[0])
break
}
}
Comment on lines +1481 to +1498
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OR-split uses original case, not the uppercased copy

colListStr is sliced from the original triggerDef (mixed case), but the split is done with " OR " which is an uppercase literal. Since pg_get_triggerdef() always returns SQL keywords in uppercase this works today, but the same function already computes upper for exactly this kind of case-insensitive matching. Using the raw string means any non-standard or future lowercase representation would silently skip the trim step and leave event keywords like delete in the column list.

Suggested change
parts := strings.Split(colListStr, " OR ")
// Only keep the first part (column list); the rest would be other events
if len(parts) > 1 {
// Check if subsequent parts are event keywords
for i := 1; i < len(parts); i++ {
trimmed := strings.TrimSpace(strings.ToUpper(parts[i]))
isEvent := false
for _, kw := range eventKeywords {
if trimmed == kw || strings.HasPrefix(trimmed, kw+" ") {
isEvent = true
break
}
}
if isEvent {
colListStr = strings.TrimSpace(parts[0])
break
}
}
parts := strings.Split(strings.ToUpper(colListStr), " OR ")
// Only keep the first part (column list); the rest would be other events
if len(parts) > 1 {
// Check if subsequent parts are event keywords
for i := 1; i < len(parts); i++ {
trimmed := strings.TrimSpace(parts[i])
isEvent := false
for _, kw := range eventKeywords {
if trimmed == kw || strings.HasPrefix(trimmed, kw+" ") {
isEvent = true
break
}
}
if isEvent {
// Slice colListStr at the corresponding byte offset instead
orIdx := strings.Index(strings.ToUpper(colListStr), " OR "+trimmed)
if orIdx != -1 {
colListStr = strings.TrimSpace(colListStr[:orIdx])
}
break
}
}
}

}

// Split by comma and trim each column name
rawCols := strings.Split(colListStr, ",")
var columns []string
for _, col := range rawCols {
col = strings.TrimSpace(col)
if col != "" {
Comment on lines +1468 to +1506
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extractUpdateColumnsFromTriggerDef relies on naive substring searches/splits (e.g., locating the end via " ON " and splitting on commas). This breaks for valid trigger defs where UPDATE columns are quoted identifiers containing ON or , (both allowed inside double-quoted identifiers), and it can also be confused by string literals containing UPDATE OF ... ON .... Consider extracting update-column attnums from pg_trigger.tgattr (joining pg_attribute for names) instead of parsing pg_get_triggerdef(), or implement a small tokenizer that respects quotes/literals when scanning for ON and commas.

Copilot uses AI. Check for mistakes.
columns = append(columns, col)
}
}

return columns
}

// extractFunctionCallFromTriggerDef extracts the function call (with arguments) from a trigger definition
// returned by pg_get_triggerdef(). The format is:
// "... EXECUTE FUNCTION function_name(arg1, arg2)"
Expand Down Expand Up @@ -1548,6 +1607,12 @@ func (i *Inspector) buildTriggers(ctx context.Context, schema *IR, targetSchema
condition = extractWhenClauseFromTriggerDef(triggerRow.TriggerDefinition.String)
}

// Extract UPDATE OF columns from trigger definition (only for triggers with UPDATE events)
var updateColumns []string
if triggerRow.TriggerDefinition.Valid && triggerRow.TriggerType&triggerTypeUpdate != 0 {
updateColumns = extractUpdateColumnsFromTriggerDef(triggerRow.TriggerDefinition.String)
}

// Extract transition table names
oldTable := ""
if triggerRow.OldTable.Valid {
Expand Down Expand Up @@ -1577,6 +1642,7 @@ func (i *Inspector) buildTriggers(ctx context.Context, schema *IR, targetSchema
Table: tableName,
Timing: timing,
Events: events,
UpdateColumns: updateColumns,
Level: level,
Function: functionCall,
Condition: condition,
Expand Down
5 changes: 3 additions & 2 deletions ir/ir.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,9 @@ type Trigger struct {
Table string `json:"table"`
Name string `json:"name"`
Timing TriggerTiming `json:"timing"` // BEFORE, AFTER, INSTEAD OF
Events []TriggerEvent `json:"events"` // INSERT, UPDATE, DELETE
Level TriggerLevel `json:"level"` // ROW, STATEMENT
Events []TriggerEvent `json:"events"` // INSERT, UPDATE, DELETE
UpdateColumns []string `json:"update_columns,omitempty"` // Column names for UPDATE OF
Level TriggerLevel `json:"level"` // ROW, STATEMENT
Function string `json:"function"`
Condition string `json:"condition,omitempty"` // WHEN condition
Comment string `json:"comment,omitempty"`
Expand Down
5 changes: 5 additions & 0 deletions testdata/diff/create_trigger/add_trigger/diff.sql
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ CREATE OR REPLACE TRIGGER employees_last_modified_trigger
FOR EACH ROW
EXECUTE FUNCTION update_last_modified();

CREATE OR REPLACE TRIGGER employees_salary_update_trigger
BEFORE UPDATE OF salary ON employees
FOR EACH ROW
EXECUTE FUNCTION update_last_modified();

CREATE OR REPLACE TRIGGER employees_truncate_log_trigger
AFTER TRUNCATE ON employees
FOR EACH STATEMENT
Expand Down
5 changes: 5 additions & 0 deletions testdata/diff/create_trigger/add_trigger/new.sql
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ CREATE TRIGGER employees_insert_timestamp_trigger
FOR EACH ROW
EXECUTE FUNCTION public.update_last_modified();

CREATE TRIGGER employees_salary_update_trigger
BEFORE UPDATE OF salary ON public.employees
FOR EACH ROW
EXECUTE FUNCTION public.update_last_modified();

CREATE TRIGGER employees_truncate_log_trigger
AFTER TRUNCATE ON public.employees
FOR EACH STATEMENT
Expand Down
6 changes: 6 additions & 0 deletions testdata/diff/create_trigger/add_trigger/plan.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@
"operation": "create",
"path": "public.employees.employees_last_modified_trigger"
},
{
"sql": "CREATE OR REPLACE TRIGGER employees_salary_update_trigger\n BEFORE UPDATE OF salary ON employees\n FOR EACH ROW\n EXECUTE FUNCTION update_last_modified();",
"type": "table.trigger",
"operation": "create",
"path": "public.employees.employees_salary_update_trigger"
},
{
"sql": "CREATE OR REPLACE TRIGGER employees_truncate_log_trigger\n AFTER TRUNCATE ON employees\n FOR EACH STATEMENT\n EXECUTE FUNCTION update_last_modified();",
"type": "table.trigger",
Expand Down
5 changes: 5 additions & 0 deletions testdata/diff/create_trigger/add_trigger/plan.sql
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ CREATE OR REPLACE TRIGGER employees_last_modified_trigger
FOR EACH ROW
EXECUTE FUNCTION update_last_modified();

CREATE OR REPLACE TRIGGER employees_salary_update_trigger
BEFORE UPDATE OF salary ON employees
FOR EACH ROW
EXECUTE FUNCTION update_last_modified();

CREATE OR REPLACE TRIGGER employees_truncate_log_trigger
AFTER TRUNCATE ON employees
FOR EACH STATEMENT
Expand Down
6 changes: 6 additions & 0 deletions testdata/diff/create_trigger/add_trigger/plan.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Tables:
~ employees
+ employees_insert_timestamp_trigger (trigger)
+ employees_last_modified_trigger (trigger)
+ employees_salary_update_trigger (trigger)
+ employees_truncate_log_trigger (trigger)

Views:
Expand All @@ -27,6 +28,11 @@ CREATE OR REPLACE TRIGGER employees_last_modified_trigger
FOR EACH ROW
EXECUTE FUNCTION update_last_modified();

CREATE OR REPLACE TRIGGER employees_salary_update_trigger
BEFORE UPDATE OF salary ON employees
FOR EACH ROW
EXECUTE FUNCTION update_last_modified();

CREATE OR REPLACE TRIGGER employees_truncate_log_trigger
AFTER TRUNCATE ON employees
FOR EACH STATEMENT
Expand Down
2 changes: 1 addition & 1 deletion testdata/diff/create_trigger/alter_trigger/diff.sql
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
CREATE OR REPLACE TRIGGER employees_last_modified_trigger
BEFORE INSERT OR UPDATE ON employees
BEFORE INSERT OR UPDATE OF salary ON employees
FOR EACH ROW
WHEN (((NEW.salary IS NOT NULL)))
EXECUTE FUNCTION update_last_modified();
2 changes: 1 addition & 1 deletion testdata/diff/create_trigger/alter_trigger/new.sql
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ END;
$$ LANGUAGE plpgsql;

CREATE TRIGGER employees_last_modified_trigger
BEFORE INSERT OR UPDATE ON public.employees
BEFORE INSERT OR UPDATE OF salary ON public.employees
FOR EACH ROW
WHEN (NEW.salary IS NOT NULL)
EXECUTE FUNCTION public.update_last_modified();
2 changes: 1 addition & 1 deletion testdata/diff/create_trigger/alter_trigger/plan.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
{
"steps": [
{
"sql": "CREATE OR REPLACE TRIGGER employees_last_modified_trigger\n BEFORE INSERT OR UPDATE ON employees\n FOR EACH ROW\n WHEN (((NEW.salary IS NOT NULL)))\n EXECUTE FUNCTION update_last_modified();",
"sql": "CREATE OR REPLACE TRIGGER employees_last_modified_trigger\n BEFORE INSERT OR UPDATE OF salary ON employees\n FOR EACH ROW\n WHEN (((NEW.salary IS NOT NULL)))\n EXECUTE FUNCTION update_last_modified();",
"type": "table.trigger",
"operation": "alter",
"path": "public.employees.employees_last_modified_trigger"
Expand Down
2 changes: 1 addition & 1 deletion testdata/diff/create_trigger/alter_trigger/plan.sql
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
CREATE OR REPLACE TRIGGER employees_last_modified_trigger
BEFORE INSERT OR UPDATE ON employees
BEFORE INSERT OR UPDATE OF salary ON employees
FOR EACH ROW
WHEN (((NEW.salary IS NOT NULL)))
EXECUTE FUNCTION update_last_modified();
2 changes: 1 addition & 1 deletion testdata/diff/create_trigger/alter_trigger/plan.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ DDL to be executed:
--------------------------------------------------

CREATE OR REPLACE TRIGGER employees_last_modified_trigger
BEFORE INSERT OR UPDATE ON employees
BEFORE INSERT OR UPDATE OF salary ON employees
FOR EACH ROW
WHEN (((NEW.salary IS NOT NULL)))
EXECUTE FUNCTION update_last_modified();
Loading