Skip to content

Commit c6a6112

Browse files
committed
sql: Fix trigger dependency cleanup in legacy schema changer
In the legacy schema changer for DROP TABLE, there was special logic to clean up dependencies created by triggers. However, a bug in this logic caused the dependent table’s descriptor to remain unchanged, leaving behind a dangling reference. This commit ensures that the dependent table’s descriptor is correctly updated. Epic: none Release note (bug fix): Fixed a bug where dropping a table with a trigger using the legacy schema changer could leave an orphaned reference in the descriptor. This occurred when two tables were dependent on each other via a trigger, and the table containing the trigger was dropped.
1 parent fa562ef commit c6a6112

File tree

2 files changed

+64
-3
lines changed

2 files changed

+64
-3
lines changed

pkg/ccl/logictestccl/testdata/logic_test/triggers

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4018,4 +4018,54 @@ DROP TRIGGER foo ON xy;
40184018
statement ok
40194019
CREATE OR REPLACE FUNCTION f() RETURNS TRIGGER LANGUAGE PLpgSQL AS $$ BEGIN RETURN NEW; END $$;
40204020

4021+
# ==============================================================================
4022+
# Test that descriptor backreferences are cleaned up
4023+
# ==============================================================================
4024+
4025+
subtest ensure_back_ref_cleanup
4026+
4027+
statement ok
4028+
create table listings_balance (c1 int);
4029+
4030+
# Create a function that references listings_balance
4031+
statement ok
4032+
create or replace function update_listing_balance()
4033+
returns TRIGGER language PLpgSQL AS
4034+
$$
4035+
BEGIN
4036+
INSERT INTO listings_balance VALUES (1);
4037+
RETURN NEW;
4038+
END
4039+
$$;
4040+
4041+
statement ok
4042+
CREATE TABLE transaction_entries (c1 int);
4043+
4044+
# Create a trigger that references listings_balance through the trigger function.
4045+
statement ok
4046+
create trigger tr AFTER INSERT OR UPDATE ON transaction_entries FOR EACH ROW execute function update_listing_balance();
4047+
4048+
# Force the legacy schema change for the drop table. We will save the original
4049+
# DSC setting so that it be reset properly.
4050+
let $use_decl_sc
4051+
SHOW use_declarative_schema_changer
4052+
4053+
statement ok
4054+
SET use_declarative_schema_changer = off;
4055+
4056+
# This should cleanup the dependency in listings_balance.
4057+
statement ok
4058+
DROP TABLE transaction_entries;
4059+
4060+
# Reinstate original DSC setting
4061+
statement ok
4062+
SET use_declarative_schema_changer = $use_decl_sc;
4063+
4064+
statement ok
4065+
DROP FUNCTION update_listing_balance ;
4066+
4067+
# Sanity to verify the dependency no longer exists.
4068+
statement ok
4069+
drop table listings_balance cascade;
4070+
40214071
subtest end

pkg/sql/drop_table.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ func (p *planner) dropTableImpl(
278278
for i := range tableDesc.Triggers {
279279
trigger := &tableDesc.Triggers[i]
280280
for _, id := range trigger.DependsOn {
281-
if err := p.removeTriggerBackReference(ctx, tableDesc, id); err != nil {
281+
if err := p.removeTriggerBackReference(ctx, tableDesc, id, trigger.Name); err != nil {
282282
return droppedViews, err
283283
}
284284
}
@@ -672,7 +672,7 @@ func removeFKBackReferenceFromTable(
672672
// removeTriggerBackReference removes the trigger back reference for the
673673
// referenced table with the given ID.
674674
func (p *planner) removeTriggerBackReference(
675-
ctx context.Context, tableDesc *tabledesc.Mutable, refID descpb.ID,
675+
ctx context.Context, tableDesc *tabledesc.Mutable, refID descpb.ID, triggerName string,
676676
) error {
677677
var refTableDesc *tabledesc.Mutable
678678
// We don't want to lookup/edit a second copy of the same table.
@@ -690,7 +690,18 @@ func (p *planner) removeTriggerBackReference(
690690
return nil
691691
}
692692
refTableDesc.DependedOnBy = removeMatchingReferences(refTableDesc.DependedOnBy, tableDesc.ID)
693-
return nil
693+
694+
name, err := p.getQualifiedTableName(ctx, tableDesc)
695+
if err != nil {
696+
return err
697+
}
698+
refName, err := p.getQualifiedTableName(ctx, refTableDesc)
699+
if err != nil {
700+
return err
701+
}
702+
jobDesc := fmt.Sprintf("updating table %q after removing trigger %q from table %q",
703+
refName.FQString(), triggerName, name.FQString())
704+
return p.writeSchemaChange(ctx, refTableDesc, descpb.InvalidMutationID, jobDesc)
694705
}
695706

696707
// removeMatchingReferences removes all refs from the provided slice that

0 commit comments

Comments
 (0)