Skip to content

sql: identify trigger-based table dependencies in backreferences#161910

Open
rafiss wants to merge 2 commits intocockroachdb:masterfrom
rafiss:trigger-backref
Open

sql: identify trigger-based table dependencies in backreferences#161910
rafiss wants to merge 2 commits intocockroachdb:masterfrom
rafiss:trigger-backref

Conversation

@rafiss
Copy link
Collaborator

@rafiss rafiss commented Jan 28, 2026

sql: identify trigger-based table dependencies in backreferences

Previously, when a trigger function referenced another table, the backreference in DependedOnBy was structurally identical to view dependencies. This made it impossible to distinguish trigger dependencies from view dependencies when inspecting crdb_internal.forward_dependencies.

This commit adds a trigger_id field to TableDescriptor_Reference in the protobuf schema. When creating backreferences for trigger dependencies, we now set the trigger_id to identify the originating trigger. The crdb_internal.forward_dependencies virtual table has been updated to report dependedonby_type='trigger' when the trigger_id is set, instead of 'view'.

upgrades: add migration to repair trigger backrefs

Previously, trigger dependencies in DependedOnBy had TriggerID=0, making
them indistinguishable from view dependencies. This caused issues when
dropping a trigger that shared a referenced table with another trigger,
as the drop would incorrectly remove the shared backref.

This commit adds a migration that repairs existing trigger backrefs by:

  1. Iterating through all tables with triggers in each database
  2. For each trigger's DependsOn references, finding the corresponding
    backref with TriggerID=0 and setting the correct TriggerID
  3. If multiple triggers reference the same table (which previously shared
    a single backref), creating separate backrefs for each trigger

The migration processes descriptors in batches of 100 to avoid overly
large transactions.

Resolves: #147801
Epic: CRDB-42942

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@rafiss
Copy link
Collaborator Author

rafiss commented Jan 28, 2026

@spilchen I was curious to learn more about this proposed change from the issue:

Update operationGenerator.tableHasDependencies:

  • Include self-dependencies for triggers.
  • Continue omitting self-dependencies for foreign keys (current behavior).

Was the intended change something like this?

            AND NOT ( 
              fd.descriptor_id = fd.dependedonby_id 
              AND ( 
                fd.dependedonby_type = 'fk' 
                OR ($3::BOOL = true AND fd.dependedonby_type != 'trigger')
              ) 
            ) 

in the query:

SELECT EXISTS(
SELECT fd.descriptor_name
FROM crdb_internal.forward_dependencies AS fd
WHERE fd.descriptor_id
= (
SELECT c.oid
FROM pg_catalog.pg_class AS c
JOIN pg_catalog.pg_namespace AS ns ON
ns.oid = c.relnamespace
WHERE c.relname = $1 AND ns.nspname = $2
)
AND NOT (
fd.descriptor_id = fd.dependedonby_id
AND (
fd.dependedonby_type = 'fk'
OR $3::BOOL = true
)
)
AND fd.dependedonby_type != 'sequence'

I wanted to understand why we need this change before going ahead with it

@spilchen
Copy link
Contributor

I’m not 100% sure, but I believe my thinking was around the change to crdb_internal.forward_dependencies. It will now show something for triggers where it didn't before?? And this dependency check may fire even for triggers defined on themselves (for example, a trigger on table t1 that references t1 in its trigger body). As discussed, just running a --stress against the RSW is probably the best approach here.

@rafiss rafiss force-pushed the trigger-backref branch 2 times, most recently from c4772c7 to b5aeee8 Compare January 29, 2026 05:23
Previously, when a trigger function referenced another table, the
backreference in DependedOnBy was structurally identical to view
dependencies. This made it impossible to distinguish trigger
dependencies from view dependencies when inspecting
crdb_internal.forward_dependencies.

This commit adds a trigger_id field to TableDescriptor_Reference in
the protobuf schema. When creating backreferences for trigger
dependencies, we now set the trigger_id to identify the originating
trigger. The crdb_internal.forward_dependencies virtual table has
been updated to report dependedonby_type='trigger' when the
trigger_id is set, instead of 'view'.

Resolves: cockroachdb#147801
Epic: CRDB-42942

Release note: None
Previously, trigger dependencies in DependedOnBy had TriggerID=0, making
them indistinguishable from view dependencies. This caused issues when
dropping a trigger that shared a referenced table with another trigger,
as the drop would incorrectly remove the shared backref.

This commit adds a migration that repairs existing trigger backrefs by:
1. Iterating through all tables with triggers in each database
2. For each trigger's DependsOn references, finding the corresponding
   backref with TriggerID=0 and setting the correct TriggerID
3. If multiple triggers reference the same table (which previously shared
   a single backref), creating separate backrefs for each trigger

The migration processes descriptors in batches of 100 to avoid overly
large transactions.

Epic: CRDB-42942
Release note: none
@rafiss
Copy link
Collaborator Author

rafiss commented Feb 4, 2026

It will now show something for triggers where it didn't before?? And this dependency check may fire even for triggers defined on themselves (for example, a trigger on table t1 that references t1 in its trigger body). As discussed, just running a --stress against the RSW is probably the best approach here.

these were getting tracked before, but just were categorized the same as views. i ran the RSW test under stress and it seems fine as-is.

@rafiss rafiss marked this pull request as ready for review February 4, 2026 22:08
@rafiss rafiss requested review from a team as code owners February 4, 2026 22:08
@rafiss rafiss requested a review from spilchen February 4, 2026 22:09
@github-actions
Copy link

github-actions bot commented Feb 4, 2026

Potential Bug(s) Detected

The three-stage Claude Code analysis has identified potential bug(s) in this PR that may warrant investigation.

Next Steps:
Please review the detailed findings in the workflow run.

Note: When viewing the workflow output, scroll to the bottom to find the Final Analysis Summary.

After you review the findings, please tag the issue as follows:

  • If the detected issue is real or was helpful in any way, please tag the issue with O-AI-Review-Real-Issue-Found
  • If the detected issue was not helpful in any way, please tag the issue with O-AI-Review-Not-Helpful

@github-actions github-actions bot added the o-AI-Review-Potential-Issue-Detected AI reviewer found potential issue. Never assign manually—auto-applied by GH action only. label Feb 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

o-AI-Review-Potential-Issue-Detected AI reviewer found potential issue. Never assign manually—auto-applied by GH action only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sql: identify trigger-based table dependencies in backreferences

3 participants