cockroachdb / cockroachdb/cockroach
sql,schemachanger: remove pre-v26.2 trigger back-ref orphan sweep
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
`UpdateTriggerBackReferencesInRelations` (`pkg/sql/schemachanger/scexec/scmutationexec/references.go`) contains code that removes orphaned trigger back-references left behind by pre-v26.2 `DROP TRIGGER` (see #173259). This issue is to track removing that code.
**Background**
Before v26.2, trigger back-references carried no `TriggerID` (i.e. `TriggerID == 0`). Dropping one of several triggers could therefore leave an untagged back-reference behind which is the issue described in #173259.
The sweep is deliberately narrow: it only removes a `TriggerID == 0` back-ref during a drop when no trigger is still present on the source table references the target relation (`!sourceHasLiveTriggerRef`), and it excludes sequences.
We can't easily replace this sweep with a upgrade repair because **a trigger can create multiple back-references to the same table**. This is indistinguishable from an actual orphaned trigger back-ref.
Example on 26.1:
```sql
CREATE DATABASE db1;
USE db1;
CREATE TABLE t1 (a INT, b INT, INDEX idx_a (a), INDEX idx_b (b));
CREATE TABLE t2 (id INT PRIMARY KEY);
CREATE FUNCTION f() RETURNS TRIGGER AS $$
DECLARE x INT; y INT;
BEGIN
SELECT a INTO x FROM t1@idx_a WHERE a = 1;
SELECT b INTO y FROM t1@idx_b WHERE b = 2;
RETURN NEW;
END;
$$ LANGUAGE PLpgSQL;
CREATE TRIGGER trg AFTER INSERT ON t2 FOR EACH ROW EXECUTE FUNCTION f();
SELECT descriptor_id, dependedonby_id, dependedonby_type, dependedonby_index_id, dependedonby_details
FROM crdb_internal.forward_dependencies
WHERE descriptor_id = 't1'::regclass::int
ORDER BY dependedonby_details;
descriptor_id | dependedonby_id | dependedonby_type | dependedonby_index_id | dependedonby_details
----------------+-----------------+-------------------+-----------------------+-----------------------
114 | 115 | view | 2 | Columns: [1]
114 | 115 | view | 3 | Columns: [2]
(2 rows)
```
```sql
CREATE DATABASE db2;
USE db2;
CREATE TABLE t1 (a INT, b INT, INDEX idx_a (a), INDEX idx_b (b));
CREATE TABLE t2 (id INT PRIMARY KEY);
CREATE FUNCTION fn_a() RETURNS TRIGGER LANGUAGE PLpgSQL AS $$
BEGIN SELECT a FROM t1@idx_a WHERE a = 1; RETURN NEW; END; $$;
CREATE FUNCTION fn_b() RETURNS TRIGGER LANGUAGE PLpgSQL AS $$
BEGIN SELECT b FROM t1@idx_b WHERE b = 2; RETURN NEW; END; $$;
CREATE TRIGGER tr_a AFTER UPDATE ON t2 FOR EACH ROW EXECUTE FUNCTION fn_a();
CREATE TRIGGER tr_b AFTER INSERT ON t2 FOR EACH ROW EXECUTE FUNCTION fn_b();
-- drop one of the triggers to create an orphan
DROP TRIGGER tr_a ON t2;
SELECT descriptor_id, dependedonby_id, dependedonby_type, dependedonby_index_id, dependedonby_details
FROM crdb_internal.forward_dependencies
WHERE descriptor_id = 't1'::regclass::int
ORDER BY dependedonby_details;
descriptor_id | dependedonby_id | dependedonby_type | dependedonby_index_id | dependedonby_details
----------------+-----------------+-------------------+-----------------------+-----------------------
119 | 120 | view | 2 | Columns: [1]
119 | 120 | view | 3 | Columns: [2]
```
After upgrade to 26.2, exactly one of the references will be tagged with a TriggerID. In either case, `DROP TRIGGER` would fail without the sweep because the reference with `TriggerID == 0` wasn't cleaned up.
There's no easy way of telling how many back-references one trigger created. Therefore, we decided to keep the sweep in `UpdateTriggerBackReferencesInRelations`.
**When this can be removed**
There's no release that guarantees no `TriggerID == 0` entries remain because it can only be rewritten by this sweep, or descriptor surgery, and we're not running a bulk cleanup. It's best to keep it for a few releases. Removing it early means customers may hit errors dropping triggers on affected relations, and that can be fixed with a descriptor surgery.
**Notes**
Restore behaves differently from upgrade.`maybeRepairTriggerBackrefs` leaves no references with `TriggerID == 0`. This means restoring from pre 26.2 would keep exactly one reference with `TriggerID != 0`. Restoring from 26.2+ would tag at most one reference with a `TriggerID`. This behavior is inconsistent and could be improved.
Jira issue: CRDB-67460
Epic CRDB-65516
Contributor guide
Research direction
Start in pkg/sql/schemachanger/scexec/scmutationexec/references.go at UpdateTriggerBackReferencesInRelations and inspect the pre-v26.2 TriggerID == 0 orphan sweep. Review the surrounding trigger back-reference handling and the issue's release-safety conditions; done means the obsolete sweep is removed without causing affected DROP TRIGGER operations to fail.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, sql
- Domain
- databases
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100