cockroachdb / cockroachdb/cockroach

sql,schemachanger: remove pre-v26.2 trigger back-ref orphan sweep

Open
#174,159 0 comments 0 reactions 0 assignees View on GitHub
C-enhancement T-sql-foundations
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.