fix: correctly separate disabled triggers in \d table output#161
Open
devadathanmb wants to merge 4 commits intodbcli:mainfrom
Open
fix: correctly separate disabled triggers in \d table output#161devadathanmb wants to merge 4 commits intodbcli:mainfrom
devadathanmb wants to merge 4 commits intodbcli:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes \d table output so disabled triggers are displayed under a separate Disabled triggers: section (matching psql), instead of being lumped into the enabled Triggers: section.
Changes:
- Update trigger rendering in
\dtable details to iterate over a cached trigger result set, allowing proper per-category grouping. - Extend test DB fixtures with a table containing both enabled and disabled triggers, and add assertions covering the expected output split.
- Update changelog and adjust existing expected object/function listings to account for the new fixtures.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
pgspecial/dbcommands.py |
Fixes trigger categorization by re-iterating over fetched trigger rows per category (enabled/disabled/always/replica). |
tests/dbutils.py |
Adds trigger-related fixtures (table, trigger function, enabled + disabled trigger). |
tests/test_specials.py |
Updates listing expectations and adds a regression test verifying disabled triggers are shown under Disabled triggers:. |
changelog.rst |
Documents the bug fix in the Unreleased section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
When
\d tableis used on a table with both enabled and disabled triggers, disabled triggers should appear under a separateDisabled triggers:section — consistent with howpsqldisplays trigger information. Instead, all triggers were being lumped together underTriggers:, making it impossible to distinguish disabled triggers from enabled ones.Two bugs in
describe_one_table_details()caused this:list_triggerflag that decides whether to print a row was reset once per category, not once per row. This meant that once any row in a pass set it toTrue, all subsequent rows in that pass were printed regardless of their actual state.This change fixes both issues by materialising the cursor into a list with
fetchall()before the loop, and resetting the flag per row.I have run the full
pgcliintegration test suite locally with this patch inpgspecialto ensure that nothing breaks after this change.Closes dbcli/pgcli#1519
Screenshots
psql(expected behaviour):Before (In
pgcli):After (In
pgcli):Checklist
changelog.rst.pip install pre-commit && pre-commit install).